Re: CREATE FUNCTION ... SEARCH { DEFAULT | SYSTEM | SESSION }

On Wed, Aug 16, 2023 at 1:45 PM Jeff Davis <pgsql(at)j-davis(dot)com> wrote:
> On Wed, 2023-08-16 at 08:51 +0200, Peter Eisentraut wrote:
> > On 12.08.23 04:35, Jeff Davis wrote:
> > > The attached patch implements a new SEARCH clause for CREATE
> > > FUNCTION.
> > > The SEARCH clause controls the search_path used when executing
> > > functions that were created without a SET clause.
> >
> > I don't understand this. This adds a new option for cases where the
> > existing option wasn't specified. Why not specify the existing
> > option
> > then? Is it not good enough? Can we improve it?
>
> SET search_path = '...' not good enough in my opinion.
>
> 1. Not specifying a SET clause falls back to the session's search_path,
> which is a bad default because it leads to all kinds of inconsistent
> behavior and security concerns.
>
> 2. There's no way to explicitly request that you'd actually like to use
> the session's search_path, so it makes it very hard to ever change the
> default.
>
> 3. It's user-unfriendly. A safe search_path that would be suitable for
> most functions is "SET search_path = pg_catalog, pg_temp", which is
> arcane, and requires some explanation.
>
> 4. search_path for the session is conceptually different than for a
> function. A session should be context-sensitive and the same query
> should (quite reasonably) behave differently for different sessions and
> users to sort out things like object name conflicts, etc. A function
> should (ordinarily) be context-insensitive, especially when used in
> something like an index expression or constraint. Having different
> syntax helps separate those concepts.
>
> 5. There's no way to prevent pg_temp from being included in the
> search_path. This is separately fixable, but having the proposed SEARCH
> syntax is likely to make for a better user experience in the common
> cases.
>
> I'm open to suggestion about other ways to improve it, but SEARCH is
> what I came up with.

The one thing that I really like about your proposal is that you
explicitly included a way of specifying that the prevailing
search_path should be used. If we move to any kind of a system where
the default behavior is something other than that, then we need that
as an option. Another, related thing that I recently discovered would
be useful is a way to say "I'd like to switch the search_path to X,
but I'd also like to discover what the prevailing search_path was just
before entering this function." For example, if I have a function that
is SECURITY DEFINER which takes some executable code as an input, I
might want to arrange to eventually execute that code with the
caller's user ID and search_path, but I can't discover the caller's
search_path unless I don't set it, and that's a dangerous thing to do.

However, my overall concern here is that this feels like it's
reinventing the wheel. We already have a way of setting search_path;
this gives us a second one. If we had no existing mechanism for that,
I think this would definitely be an improvement, and quite possibly
better than the current mechanism. But given that we had a mechanism
already, if we added this, we'd then have two, which seems like the
wrong number.

I'm inclined to think that if there are semantics that we currently
lack, we should think of extending the current syntax to support them.
Right now you can SET search_path = 'specific value' or SET
search_path FROM CURRENT or leave it out. We could introduce a new way
of spelling "leave it out," like RESET search_path or whatever. We
could introduce a new setting that doesn't set the search_path at all
but reverts to the old value on function exit, like SET search_path
USING CALL or whatever. And we could think of making SET search_path
FROM CURRENT or any new semantics we introduce the default in a future
release, or even make the default behavior depend on an evil
behavior-changing GUC as you proposed. I'm not quite sure what we
should do here conceptually, but I don't see why having a completely
new syntax for doing it really helps.

--
Robert Haas
EDB: http://www.enterprisedb.com