Re: MAINTAIN privilege -- what do we need to un-revert it?

From: Yugo Nagata <nagata(at)sraoss(dot)co(dot)jp>
To: Yugo NAGATA <nagata(at)sraoss(dot)co(dot)jp>
Cc: Jeff Davis <pgsql(at)j-davis(dot)com>, Noah Misch <noah(at)leadboat(dot)com>, Michael Paquier <michael(at)paquier(dot)xyz>, Nathan Bossart <nathandbossart(at)gmail(dot)com>, pgsql-hackers(at)postgresql(dot)org, Joe Conway <mail(at)joeconway(dot)com>, Robert Haas <robertmhaas(at)gmail(dot)com>
Subject: Re: MAINTAIN privilege -- what do we need to un-revert it?
Date: 2024-08-05 07:05:02
Message-ID: 20240805160502.d2a4975802a832b1e04afb80@sraoss.co.jp
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Fri, 2 Aug 2024 16:13:01 +0900
Yugo NAGATA <nagata(at)sraoss(dot)co(dot)jp> wrote:

> On Thu, 01 Aug 2024 11:31:53 -0700
> Jeff Davis <pgsql(at)j-davis(dot)com> wrote:
>
> > On Wed, 2024-07-31 at 18:20 +0900, Yugo NAGATA wrote:
> > > I agree that it might not be important, but I think adding the flag
> > > would be
> > > also helpful for improving code-readability because it clarify the
> > > function
> > > is used in the two cases. I attached patch for this fix (patch 0003).
> >
> > Committed with one minor modification: I moved the boolean flag to be
> > near the other booleans rather than at the end. Thank you.
> >
> > > Sure. I fixed the patch to remove 'param' from both functions. (patch
> > > 0002)
> >
> > Committed, thank you.
>
> Thank you for committing them.
> Should not they be backported to REL_17_STABLE?
>
> >
> > > I also add the small refactoring around ExecCreateTableAs(). (patch
> > > 0001)
> > >
> > > - Remove matview-related codes from intorel_startup.
> > >   Materialized views are no longer handled in this function.
> > >
> > > - RefreshMatViewByOid is moved to just after create_ctas_nodata
> > >   call to improve code readability.
> > >
> >
> > I'm not sure the changes in intorel_startup() are correct. I tried
> > adding an Assert(into->viewQuery == NULL), and it fails because there's
> > another path I did not consider: "EXPLAIN ANALYZE CREATE MATERIALIZED
> > VIEW ...", which does not go through ExecCreateTableAs() but does go
> > through CreateIntoRelDestReceiver().
> >
> > See:
> >
> > https://postgr.es/m/20444c382e6cb5e21e93c94d679d0198b0dba4dd.camel@j-davis.com
> >
> > Should we refactor a bit and try to make EXPLAIN use the same code
> > paths?
>
> I overlooked that CreateIntoRelDestReceiver() is used from EXPLAIN. I saw the
> thread above and I agree that we should refactor it to make EXPLAIN consistent
> CREATE MATERIALIZED VIEW, but I suppose this should be discussed the other thread.
>
> I attached a updated patch removed the intorel_startup() part from.

I confirmed that this has been committed to the master branch.
Thank you!

I also noticed that the documentation of CREATE MATERIALIZED VIEW doesn't mention
search_path while it also changes search_path since it uses the REFRESH logic.
I attached a trivial patch to fix this.

Regards,
Yugo Nagata

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

Attachment Content-Type Size
doc_create_matview_search_path.patch text/x-diff 689 bytes

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Anthonin Bonnefoy 2024-08-05 07:19:08 Re: Set query_id for query contained in utility statement
Previous Message Michael Paquier 2024-08-05 06:23:05 Re: Pluggable cumulative statistics