MIME-Version: 1.0
References: <e44327179e5c9015c8dda67351c04da552066017.camel@j-davis.com> <b701501541a424111ffe47be055e0ccefefd7745.camel@j-davis.com>
In-Reply-To: <b701501541a424111ffe47be055e0ccefefd7745.camel@j-davis.com>
From: Isaac Morland <isaac.morland@gmail.com>
Date: Thu, 6 Jul 2023 23:22:13 -0400
Message-ID: <CAMsGm5cpsaCEvx-kYHLE28GFGMSZfewsAcHbR2KvtTmpM_9o0g@mail.gmail.com>
Subject: Re: Fix search_path for all maintenance commands
To: Jeff Davis <pgsql@j-davis.com>
Cc: pgsql-hackers@postgresql.org, Nathan Bossart <nathandbossart@gmail.com>, 
	Robert Haas <robertmhaas@gmail.com>, Noah Misch <noah@leadboat.com>, 
	"David G. Johnston" <david.g.johnston@gmail.com>, Greg Stark <stark@mit.edu>, Tom Lane <tgl@sss.pgh.pa.us>, 
	GurjeetSingh <gurjeet@singh.im>
Content-Type: multipart/alternative; boundary="00000000000040b33605ffdd24de"
Archived-At: <https://www.postgresql.org/message-id/CAMsGm5cpsaCEvx-kYHLE28GFGMSZfewsAcHbR2KvtTmpM_9o0g%40mail.gmail.com>
Precedence: bulk

--00000000000040b33605ffdd24de
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Thu, 6 Jul 2023 at 21:39, Jeff Davis <pgsql@j-davis.com> wrote:

I apologize in advance if anything I=E2=80=99ve written below is either too=
 obvious
or too crazy or misinformed to belong here. I hope I have something to say
that is on point, but feel unsure what makes sense to say.

* It might break for users who have a functional index where the
> function implicitly depends on a search_path containing a namespace
> other than pg_catalog. My opinion is that such functional indexes are
> conceptually broken and we need to desupport them, and there will be
> some breakage, but I'm open to suggestion about how we minimize that (a
> compatibility GUC or something?).
>

I agree this is OK. If somebody has an index whole meaning depends on the
search_path, then the best that can be said is that their database hasn't
been corrupted yet. At the same time, I can see that somebody would get
upset if they couldn't upgrade their database because of this. Maybe
pg_upgrade could apply "SET search_path TO pg_catalog, pg_temp" to any
function used in a functional index that doesn't have a search_path setting
of its own? (BEGIN ATOMIC functions count, if I understand correctly, as
having a search_path setting, because the lookups happen at definition time=
)

Now I'm doing more reading and I'm worried about SET TIME ZONE (or more
precisely, its absence) and maybe some other ones.

* The fix might not go far enough or might be in the wrong place. I'm
> open to suggestion here, too. Maybe we can make it part of the general
> function call mechanism, and can be overridden by explicitly setting
> the function search path? Or maybe we need new syntax where the
> function can acquire the search path from the session explicitly, but
> uses a safe search path by default?
>

Change it so by default each function gets handled as if "SET search_path
FROM CURRENT" was applied to it? That's what I do for all my functions
(maybe hurting performance?). Expand on my pg_upgrade idea above by
applying it to all functions?

I feel that this may tie into other behaviour issues where to me it is
obvious that the expected behaviour should be different from the actual
behaviour. If a view calls a function, shouldn't it be called in the
context of the view's definer/owner? It's weird that I can write a view
that filters a table for users of the view, but as soon as the view calls
functions they run in the security context of the user of the view. Are
views security definers or not? Similar comment for triggers. Also as far
as I can tell there is no way for a security definer function to determine
who (which user) invoked it. So I can grant/deny access to run a particular
function using permissions, but I can't have the supposed security definer
define security for different callers.

Is the fundamental problem that we now find ourselves wanting to do things
that require different defaults to work smoothly? On some level I suspect
we want lexical scoping, which is what most of us have in our programming
languages, in the database; but the database has many elements of dynamic
scoping, and changing that is both a compatibility break and requires
significant changes in the way the database is designed.

--00000000000040b33605ffdd24de
Content-Type: text/html; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

<div dir=3D"ltr"><div dir=3D"ltr">On Thu, 6 Jul 2023 at 21:39, Jeff Davis &=
lt;<a href=3D"mailto:pgsql@j-davis.com">pgsql@j-davis.com</a>&gt; wrote:<br=
></div><div class=3D"gmail_quote"><div><br></div><div>I apologize in advanc=
e if anything I=E2=80=99ve written below is either too obvious or too crazy=
 or misinformed to belong here. I hope I have something to say that is on p=
oint, but feel unsure what makes sense to say.</div><div><br></div><blockqu=
ote class=3D"gmail_quote" style=3D"margin:0px 0px 0px 0.8ex;border-left-wid=
th:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-l=
eft:1ex">
* It might break for users who have a functional index where the<br>
function implicitly depends on a search_path containing a namespace<br>
other than pg_catalog. My opinion is that such functional indexes are<br>
conceptually broken and we need to desupport them, and there will be<br>
some breakage, but I&#39;m open to suggestion about how we minimize that (a=
<br>
compatibility GUC or something?).<br></blockquote><div><br></div><div>I agr=
ee this is OK. If somebody has an index whole=C2=A0meaning depends on the s=
earch_path, then the best that can be said is that their database hasn&#39;=
t been corrupted yet. At the same time, I can see that somebody would get u=
pset if they couldn&#39;t upgrade their database because of this. Maybe pg_=
upgrade could apply &quot;SET search_path TO pg_catalog, pg_temp&quot; to a=
ny function used in a functional index that doesn&#39;t have a search_path =
setting of its own? (BEGIN ATOMIC functions count, if I understand correctl=
y, as having a search_path setting, because the lookups happen at definitio=
n time)</div><div><br></div><div>Now I&#39;m doing more reading and I&#39;m=
 worried about SET TIME ZONE (or more precisely, its absence) and maybe som=
e other ones.</div><div><br></div><blockquote class=3D"gmail_quote" style=
=3D"margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;=
border-left-color:rgb(204,204,204);padding-left:1ex">
* The fix might not go far enough or might be in the wrong place. I&#39;m<b=
r>
open to suggestion here, too. Maybe we can make it part of the general<br>
function call mechanism, and can be overridden by explicitly setting<br>
the function search path? Or maybe we need new syntax where the<br>
function can acquire the search path from the session explicitly, but<br>
uses a safe search path by default?<br></blockquote><div>=C2=A0</div><div>C=
hange it so by default each function gets handled as if &quot;SET search_pa=
th FROM CURRENT&quot; was applied to it? That&#39;s what I do for all my fu=
nctions (maybe hurting performance?). Expand on my pg_upgrade idea above by=
 applying it to all functions?</div><div><br></div><div>I feel that this ma=
y tie into other behaviour issues where to me it is obvious that the expect=
ed behaviour should be different from the actual behaviour. If a view calls=
 a function, shouldn&#39;t it be called in the context of the view&#39;s de=
finer/owner? It&#39;s weird that I can write a view that filters a table fo=
r users of the view, but as soon as the view calls functions they run in th=
e security context of the user of the view. Are views security definers or =
not? Similar comment for triggers. Also as far as I can tell there is no wa=
y for a security definer function to determine who (which user) invoked it.=
 So I can grant/deny access to run a particular function using permissions,=
 but I can&#39;t have the supposed security definer define security for dif=
ferent callers.</div><div><br></div><div>Is the fundamental problem that we=
 now find ourselves wanting to do things that require different defaults to=
 work smoothly? On some level I suspect we want lexical scoping, which is w=
hat most of us have in our programming languages, in the database; but the =
database has many elements of dynamic scoping,=C2=A0and changing that is bo=
th a compatibility break and requires significant changes in the way the da=
tabase=C2=A0is designed.</div><div><br></div></div></div>

--00000000000040b33605ffdd24de--