Horton

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

Yugo Nagata <nagata@sraoss.co.jp>

View thread View on postgr.es
From: Yugo NAGATA <nagata@sraoss.co.jp>
To: Jeff Davis <pgsql@j-davis.com>
Cc: Noah Misch <noah@leadboat.com>, Michael Paquier <michael@paquier.xyz>, Nathan Bossart <nathandbossart@gmail.com>, pgsql-hackers@postgresql.org, Joe Conway <mail@joeconway.com>, Robert Haas <robertmhaas@gmail.com>
Date: 2024-08-02T07:13:01Z
Lists: pgsql-hackers

Attachments

On Thu, 01 Aug 2024 11:31:53 -0700
Jeff Davis <pgsql@j-davis.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.

Regards,
Yugo Nagata

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

Commits

  1. doc: Note that CREATE MATERIALIZED VIEW restricts search_path.

  2. Small refactoring around ExecCreateTableAs().

  3. Add is_create parameter to RefreshMatviewByOid().

  4. Remove unused ParamListInfo argument from ExecRefreshMatView.

  5. When creating materialized views, use REFRESH to load data.

  6. Add missing RestrictSearchPath() calls.

  7. Fix search_path to a safe value during maintenance operations.

  8. Revert MAINTAIN privilege and pg_maintain predefined role.

  9. Avoid repeated name lookups during table and index DDL.

  10. Add a materialized view relations.