Re: Inconsistency with EXPLAIN ANALYZE CREATE MATERIALIZED VIEW - Mailing list pgsql-hackers

From Yugo Nagata
Subject Re: Inconsistency with EXPLAIN ANALYZE CREATE MATERIALIZED VIEW
Date
Msg-id 20240807030614.c89c79e05e34cd38a70d3795@sraoss.co.jp
Whole thread Raw
In response to Re: Inconsistency with EXPLAIN ANALYZE CREATE MATERIALIZED VIEW  (Yugo Nagata <nagata@sraoss.co.jp>)
Responses Re: Inconsistency with EXPLAIN ANALYZE CREATE MATERIALIZED VIEW
List pgsql-hackers
On Wed, 7 Aug 2024 02:13:04 +0900
Yugo Nagata <nagata@sraoss.co.jp> wrote:

> On Thu, 01 Aug 2024 13:34:51 -0700
> Jeff Davis <pgsql@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 <pgsql@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.

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@sraoss.co.jp>


-- 
Yugo Nagata <nagata@sraoss.co.jp>



pgsql-hackers by date:

Previous
From: Robert Haas
Date:
Subject: Re: Vectored IO in XLogWrite()
Next
From: Jeff Davis
Date:
Subject: Re: Inconsistency with EXPLAIN ANALYZE CREATE MATERIALIZED VIEW