Re: Inconsistency with EXPLAIN ANALYZE CREATE MATERIALIZED VIEW

On Wed, 7 Aug 2024 02:13:04 +0900
Yugo Nagata <nagata(at)sraoss(dot)co(dot)jp> wrote:

> On Thu, 01 Aug 2024 13:34:51 -0700
> Jeff Davis <pgsql(at)j-davis(dot)com> wrote:
>
> > On Fri, 2024-08-02 at 00:13 +0500, Kirill Reshke wrote:
> > > On Thu, 1 Aug 2024 at 23:27, Jeff Davis <pgsql(at)j-davis(dot)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.

I'm sorry. After sending the mail, I found the patch didn't work.
If we call RestrictSearchPath() before creating a relation, it tries
to create the relation in pg_catalog and it causes an error.

Perhaps, we would need to create the rel before RestrictSearchPath() by calling
create_ctas_nodata or something like this...

>
> 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 <nagata(at)sraoss(dot)co(dot)jp>

--
Yugo Nagata <nagata(at)sraoss(dot)co(dot)jp>