Horton

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

Jeff Davis <pgsql@j-davis.com>

View thread View on postgr.es
From: Jeff Davis <pgsql@j-davis.com>
To: Nathan Bossart <nathandbossart@gmail.com>
Cc: pgsql-hackers@postgresql.org, Joe Conway <mail@joeconway.com>, Robert Haas <robertmhaas@gmail.com>
Date: 2024-02-28T17:29:04Z
Lists: pgsql-hackers 
On Wed, 2024-02-28 at 10:55 -0600, Nathan Bossart wrote:
> I'm afraid I don't have a better idea than adding a short note in
> each
> affected commands's page.

OK, that works for now.

Later we should also document that the functions are run as the table
owner.

> > On Fri, 2024-02-23 at 15:30 -0600, Nathan Bossart wrote:
> > > I wonder if it's worth using PGC_S_INTERACTIVE or introducing a
> > > new
> > > value
> > > for these.
> > 
> > Did you have a particular concern about PGC_S_SESSION?
> 
> My only concern is that it could obscure the source of the
> search_path
> change, which in turn might cause confusion when things fail.

That's a good point. AutoVacWorkerMain uses PGC_S_OVERRIDE, but it
doesn't have to worry about SET, because there's no real session.

The function SET clause uses PGC_S_SESSION. It's arguable whether
that's really the same source as a SET command, but it's definitely
closer.

> 
> Yeah, we would have to make it equivalent in priority to
> PGC_S_SESSION,
> which would likely require a bunch of special logic.

I'm not clear on what problem that would solve.

> I don't doubt anything you've said, but I can't help but think that
> we
> might as well handle the pg_temp risk, too.

That sounds good to me, but I didn't get many replies in that last
thread. And although it solves the problem, it is a bit awkward.

Can we get some closure on whether that !pg_temp patch is the right
approach? That was just my first idea, and it would be good to hear
what others think.

> Furthermore, I see that we use "" as a safe search_path for
> autovacuum and
> fe_utils/connect.h.  Is there any way to unite these?

We could have a single function like RestrictSearchPath(), which I
believe I had in some previous iteration. That would use the safest
search path (either excluding pg_temp or putting it at the end) and
PGC_S_SESSION, and then use it everywhere.


Regards,
	Jeff Davis

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.