Message-ID: <31ae1bf73e8a1dae3ee7ef2dd43a9bbda1e1ab86.camel@j-davis.com>
Subject: Re: MAINTAIN privilege -- what do we need to un-revert it?
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: Wed, 28 Feb 2024 09:29:04 -0800
In-Reply-To: <20240228165523.GA2435539@nathanxps13>
References: <d4ccaf3658cb3c281ec88c851a09733cd9482f22.camel@j-davis.com>
	 <20240214190226.GA373350@nathanxps13>
	 <4d497d82e4516037dc1082d5a62d199ea00d62e7.camel@j-davis.com>
	 <20240223213055.GA1805646@nathanxps13>
	 <196d1c86d43a147e777bb74159774f255dbe73e3.camel@j-davis.com>
	 <20240228165523.GA2435539@nathanxps13>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
User-Agent: Evolution 3.44.4-0ubuntu2 
MIME-Version: 1.0
Archived-At: 
 <https://www.postgresql.org/message-id/31ae1bf73e8a1dae3ee7ef2dd43a9bbda1e1ab86.camel%40j-davis.com>
Precedence: bulk

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.
> >=20
> > Did you have a particular concern about PGC_S_SESSION?
>=20
> 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.

>=20
> 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.=C2=A0 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