On Fri, Jul 12, 2024 at 02:50:52PM -0700, Jeff Davis wrote:
> 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?
Right, that's the only reason.
> @@ -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;
> }
Since refresh->relation is a RangeVar, this departs from the standard against
repeated name lookups, from CVE-2014-0062 (commit 5f17304).
