On Thu, 01 Aug 2024 13:34:51 -0700
Jeff Davis <pg...@j-davis.com> wrote:
> On Fri, 2024-08-02 at 00:13 +0500, Kirill Reshke wrote:
> > On Thu, 1 Aug 2024 at 23:27, Jeff Davis <pg...@j-davis.com> wrote:
> > >
> > > EXPLAIN ANALYZE CREATE MATERIALIZED VIEW doesn't go through
> > > ExecCreateTableAs(), but does use CreateIntoRelDestReceiver().
> >
> > EXPLAIN ANALYZE and regular query goes through create_ctas_internal
> > (WITH NO DATA case too). Maybe we can simply move
> > SetUserIdAndSecContext call in this function?
>
> We need to set up the SECURITY_RESTRICTED_OPERATION before planning, in
> case the planner executes some functions.
>
> I believe we need to do some more refactoring to make this work. In
> version 17, I already refactored so that CREATE MATERIALIZED VIEW and
> REFRESH MATERIALIZED VIEW share the code. We can do something similar
> to extend that to EXPLAIN ... CREATE MATERIALIZED VIEW.
I think the most simple way to fix this is to set up the userid and the
the SECURITY_RESTRICTED_OPERATION, and call RestrictSearchPath()
before pg_plan_query in ExplainOneQuery(), as in the attached patch.
However, this is similar to the old code of ExecCreateTableAs() before
refactored. If we want to reuse REFRESH even in EXPLAIN as similar as the
current CREATE MATERIALIZED code, I think we would have to refactor
RefereshMatViewByOid to receive instrument_option and eflags as arguments
and call it in ExplainOnePlan, for example.
Do you think that is more preferred than setting up
SECURITY_RESTRICTED_OPERATION in ExplainOneQuery?
> As for the August release, the code freeze is on Saturday. Perhaps it
> can be done by then, but is there a reason we should rush it? This
> affects all supported versions, so we've lived with it for a while, and
> I don't see a security problem here. I wouldn't expect it to be a
> common use case, either.
I agree that we don't have to rush it since it is not a security bug
but just it could make a materialized view that cannot be refreshed.
Regards,
Yugo Nagata
--
Yugo Nagata <nag...@sraoss.co.jp>
diff --git a/src/backend/commands/explain.c b/src/backend/commands/explain.c
index 5771aabf40..f8beb6f6a6 100644
--- a/src/backend/commands/explain.c
+++ b/src/backend/commands/explain.c
@@ -22,6 +22,7 @@
#include "jit/jit.h"
#include "libpq/pqformat.h"
#include "libpq/protocol.h"
+#include "miscadmin.h"
#include "nodes/extensible.h"
#include "nodes/makefuncs.h"
#include "nodes/nodeFuncs.h"
@@ -467,6 +468,10 @@ standard_ExplainOneQuery(Query *query, int cursorOptions,
MemoryContext planner_ctx = NULL;
MemoryContext saved_ctx = NULL;
+ Oid save_userid = InvalidOid;
+ int save_sec_context = 0;
+ int save_nestlevel = 0;
+
if (es->memory)
{
/*
@@ -487,6 +492,20 @@ standard_ExplainOneQuery(Query *query, int cursorOptions,
bufusage_start = pgBufferUsage;
INSTR_TIME_SET_CURRENT(planstart);
+ /*
+ * For CREATE MATERIALIZED VIEW command, switch to the owner's userid, so
+ * that any functions are run as that user. Also lock down security-restricted
+ * operations and arrange to make GUC variable changes local to this command.
+ */
+ if (into && into->viewQuery)
+ {
+ GetUserIdAndSecContext(&save_userid, &save_sec_context);
+ SetUserIdAndSecContext(save_userid,
+ save_sec_context | SECURITY_RESTRICTED_OPERATION);
+ save_nestlevel = NewGUCNestLevel();
+ RestrictSearchPath();
+ }
+
/* plan the query */
plan = pg_plan_query(query, queryString, cursorOptions, params);
@@ -510,6 +529,16 @@ standard_ExplainOneQuery(Query *query, int cursorOptions,
ExplainOnePlan(plan, into, es, queryString, params, queryEnv,
&planduration, (es->buffers ? &bufusage : NULL),
es->memory ? &mem_counters : NULL);
+
+ /* CREATE MATERIALIZED VIEW command */
+ if (into && into->viewQuery)
+ {
+ /* Roll back any GUC changes */
+ AtEOXact_GUC(false, save_nestlevel);
+
+ /* Restore userid and security context */
+ SetUserIdAndSecContext(save_userid, save_sec_context);
+ }
}
/*
