On Thu, 2024-07-11 at 05:52 -0700, Noah Misch wrote:
> > I could try to refactor it into two statements and execute them
> > separately, or I could try to rewrite the statement to use a fully-
> > qualified destination table before execution. Thoughts?
>
> Those sound fine. Also fine: just adding a comment on why creation
> namespace
> considerations led to not doing it there.
Attached. 0002 separates the CREATE MATERIALIZED VIEW ... WITH DATA
into (effectively):
CREATE MATERIALIZED VIEW ... WITH NO DATA;
REFRESH MATERIALIZED VIEW ...;
Using refresh also achieves the stated goal more directly: to (mostly)
ensure that a subsequent REFRESH will succeed.
Note: the creation itself no longer executes in a security-restricted
context, but I don't think that's a problem. The only reason it's using
the security restricted context is so the following REFRESH will
succeed, right?
Regards,
Jeff Davis
From cf1722bf0716897f42ce89282f361af4be56b60b Mon Sep 17 00:00:00 2001
From: Jeff Davis <j...@j-davis.com>
Date: Tue, 9 Jul 2024 11:12:46 -0700
Subject: [PATCH v2 1/2] Add missing RestrictSearchPath() calls.
Reported-by: Noah Misch
Backpatch-through: 17
Discussion: https://postgr.es/m/20240630222344.db.nmi...@google.com
---
src/backend/commands/indexcmds.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index 2caab88aa58..c5a56c75f69 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -1230,6 +1230,7 @@ DefineIndex(Oid tableId,
*/
AtEOXact_GUC(false, root_save_nestlevel);
root_save_nestlevel = NewGUCNestLevel();
+ RestrictSearchPath();
/* Add any requested comment */
if (stmt->idxcomment != NULL)
@@ -2027,6 +2028,7 @@ ComputeIndexAttrs(IndexInfo *indexInfo,
{
SetUserIdAndSecContext(save_userid, save_sec_context);
*ddl_save_nestlevel = NewGUCNestLevel();
+ RestrictSearchPath();
}
}
@@ -2074,6 +2076,7 @@ ComputeIndexAttrs(IndexInfo *indexInfo,
{
SetUserIdAndSecContext(save_userid, save_sec_context);
*ddl_save_nestlevel = NewGUCNestLevel();
+ RestrictSearchPath();
}
/*
@@ -2104,6 +2107,7 @@ ComputeIndexAttrs(IndexInfo *indexInfo,
{
SetUserIdAndSecContext(save_userid, save_sec_context);
*ddl_save_nestlevel = NewGUCNestLevel();
+ RestrictSearchPath();
}
/*
--
2.34.1
From 00323c08d07f1fb53e29e89e44b9393d442c15b1 Mon Sep 17 00:00:00 2001
From: Jeff Davis <j...@j-davis.com>
Date: Fri, 12 Jul 2024 14:23:17 -0700
Subject: [PATCH v2 2/2] For materialized views, use REFRESH to load data
during creation.
Previously, CREATE MATERIALIZED VIEW ... WITH DATA populated the MV
during creation. Instead, use REFRESH, which locks down
security-restricted operations and restricts the
search_path. Otherwise, it's possible to create MVs that cannot be
refreshed.
Reported-by: Noah Misch
Backpatch-through: 17
Discussion: https://postgr.es/m/20240630222344.db.nmi...@google.com
---
src/backend/commands/createas.c | 35 +++++++++++++------------
src/test/regress/expected/namespace.out | 4 +--
2 files changed, 20 insertions(+), 19 deletions(-)
diff --git a/src/backend/commands/createas.c b/src/backend/commands/createas.c
index 62050f4dc59..166ade5faa7 100644
--- a/src/backend/commands/createas.c
+++ b/src/backend/commands/createas.c
@@ -225,10 +225,8 @@ ExecCreateTableAs(ParseState *pstate, CreateTableAsStmt *stmt,
Query *query = castNode(Query, stmt->query);
IntoClause *into = stmt->into;
bool is_matview = (into->viewQuery != NULL);
+ bool do_refresh = false;
DestReceiver *dest;
- Oid save_userid = InvalidOid;
- int save_sec_context = 0;
- int save_nestlevel = 0;
ObjectAddress address;
List *rewritten;
PlannedStmt *plan;
@@ -263,18 +261,13 @@ ExecCreateTableAs(ParseState *pstate, CreateTableAsStmt *stmt,
Assert(query->commandType == CMD_SELECT);
/*
- * For materialized views, lock down security-restricted operations and
- * arrange to make GUC variable changes local to this command. This is
- * not necessary for security, but this keeps the behavior similar to
- * REFRESH MATERIALIZED VIEW. Otherwise, one could create a materialized
- * view not possible to refresh.
+ * For materialized views, always skip data during table creation, and use
+ * REFRESH instead.
*/
if (is_matview)
{
- GetUserIdAndSecContext(&save_userid, &save_sec_context);
- SetUserIdAndSecContext(save_userid,
- save_sec_context | SECURITY_RESTRICTED_OPERATION);
- save_nestlevel = NewGUCNestLevel();
+ do_refresh = !into->skipData;
+ into->skipData = true;
}
if (into->skipData)
@@ -346,13 +339,21 @@ ExecCreateTableAs(ParseState *pstate, CreateTableAsStmt *stmt,
PopActiveSnapshot();
}
- if (is_matview)
+ /*
+ * For materialized views, use REFRESH, which locks down
+ * security-restricted operations and restricts the search_path.
+ * Otherwise, one could create a materialized view not possible to
+ * refresh.
+ */
+ if (do_refresh)
{
- /* Roll back any GUC changes */
- AtEOXact_GUC(false, save_nestlevel);
+ RefreshMatViewStmt *refresh = makeNode(RefreshMatViewStmt);
- /* Restore userid and security context */
- SetUserIdAndSecContext(save_userid, save_sec_context);
+ refresh->relation = into->rel;
+ ExecRefreshMatView(refresh, pstate->p_sourcetext, NULL, qc);
+
+ if (qc)
+ qc->commandTag = CMDTAG_SELECT;
}
return address;
diff --git a/src/test/regress/expected/namespace.out b/src/test/regress/expected/namespace.out
index 7d36e9cc0c4..dbbda72d395 100644
--- a/src/test/regress/expected/namespace.out
+++ b/src/test/regress/expected/namespace.out
@@ -129,8 +129,8 @@ $$;
CREATE TABLE test_maint(i INT);
INSERT INTO test_maint VALUES (1), (2);
CREATE MATERIALIZED VIEW test_maint_mv AS SELECT fn(i) FROM test_maint;
-NOTICE: current search_path: test_maint_search_path
-NOTICE: current search_path: test_maint_search_path
+NOTICE: current search_path: pg_catalog, pg_temp
+NOTICE: current search_path: pg_catalog, pg_temp
-- the following commands should see search_path as pg_catalog, pg_temp
CREATE INDEX test_maint_idx ON test_maint_search_path.test_maint (fn(i));
NOTICE: current search_path: pg_catalog, pg_temp
--
2.34.1
