Fix unqualified catalog references in psql describe queries

在 2026/6/8 16:46, Chao Li 写道:

Hi,

While testing "[aecc55866] psql: Show comments in \dRp+, \dRs+, and \dX+", I noticed a small issue that was actually introduced by "[8185bb534] CREATE SUBSCRIPTION … SERVER”.

The problem is that, when querying pg_foreign_server, it misses the "pg_catalog" schema qualification:
```
appendPQExpBuffer(&buf,
", (select srvname from pg_foreign_server where oid=subserver) AS \"%s\"\n",
gettext_noop("Server"));
```

This is not a big problem, but it provides a way to pollute the result of \dRs+ by adding a fake pg_foreign_server earlier in search_path. See this repro:

1. Setup: create a server and a sub
```
evantest=# create extension postgres_fdw;
CREATE EXTENSION
evantest=# create publication pub;
CREATE PUBLICATION
evantest=# create server s foreign data wrapper postgres_fdw options (dbname 'postgres');
CREATE SERVER
evantest=# create user mapping for current_user server s;
CREATE USER MAPPING
evantest=# create subscription sub server s publication pub with (connect=false, slot_name=none);
WARNING: subscription was created, but is not connected
HINT: To initiate replication, you must manually create the replication slot, enable the subscription, and alter the subscription to refresh publications.
CREATE SUBSCRIPTION
evantest=# \dRs+ sub;
List of subscriptions
Name | Owner | Enabled | Publication | Binary | Streaming | Two-phase commit | Disable on error | Origin | Password required | Run as owner? | Failover | Server | Retain dead tuples | Max retention duration | Retention active | Synchronous commit | Conninfo | Receiver timeout | Skip LSN | Description
------+-------+---------+-------------+--------+-----------+------------------+------------------+--------+-------------------+---------------+----------+--------+--------------------+------------------------+------------------+--------------------+----------+------------------+------------+-------------
sub | chaol | f | {pub} | f | parallel | d | f | any | t | f | f | s | f | 0 | f | off | | -1 | 0/00000000 |
(1 row)
```

As shown above, “Server” column shows the correct server name “s”.

2. Now, pollute the result
```
evantest=# create temp table pg_foreign_server (oid oid, srvname name);
CREATE TABLE
evantest=# insert into pg_foreign_server select oid, 'fake_s'::name from pg_catalog.pg_foreign_server where srvname='s';
INSERT 0 1
evantest=# \dRs+ sub;
List of subscriptions
Name | Owner | Enabled | Publication | Binary | Streaming | Two-phase commit | Disable on error | Origin | Password required | Run as owner? | Failover | Server | Retain dead tuples | Max retention duration | Retention active | Synchronous commit | Conninfo | Receiver timeout | Skip LSN | Description
------+-------+---------+-------------+--------+-----------+------------------+------------------+--------+-------------------+---------------+----------+--------+--------------------+------------------------+------------------+--------------------+----------+------------------+------------+-------------
sub | chaol | f | {pub} | f | parallel | d | f | any | t | f | f | fake_s | f | 0 | f | off | | -1 | 0/00000000 |
(1 row)
```

Now, the "Server" column shows the fake server name that I supplied.

I just tried the repro. I never knew a way to make \dRs+ to output wrong data like this, this is interesting.

The fix is to add the schema qualification, using "pg_catalog.pg_foreign_server". In describe.c, catalog objects are generally referenced by qualified names. I found 3 other occurrences that missed schema qualification, so I fixed them as well.

There are 4 spots in total. Two are v19-new, oversights of 8185bb53476378443240d57f7d844347d5fae1bf and 2f094e7ac691abc9d2fe0f4dcf0feac4a6ce1d9c. The other two are older and might be worth back-patching. So I split the fix into 2 commits: 0001 is v19-new, and 0002 is a back-patch candidate.

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/

The patch looks good to me. I applied the patch locally and verified it with “make check-world”.

Regards,
Tingchuan Sun

On Mon, Jun 08, 2026 at 04:46:46PM +0800, Chao Li wrote:

This is not a big problem, but it provides a way to pollute the
result of \dRs+ by adding a fake pg_foreign_server earlier in
search_path.

Which is always annoying.. Will fix and double-check later, thanks.
--
Michael

Hi,

On Tue, Jun 09, 2026 at 04:08:26PM +0900, Michael Paquier wrote:

On Mon, Jun 08, 2026 at 04:46:46PM +0800, Chao Li wrote:

This is not a big problem, but it provides a way to pollute the
result of \dRs+ by adding a fake pg_foreign_server earlier in
search_path.

Which is always annoying.. Will fix and double-check later, thanks.

Now I wonder if we shoud not "protect" the operators too. They could also
lead to wrong results (if not worst).

Regards,

--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com

On Tue, Jun 09, 2026 at 09:08:50AM +0000, Bertrand Drouvot wrote:

Now I wonder if we shoud not "protect" the operators too. They could also
lead to wrong results (if not worst).

Kind of true. Still we have been pretty lax about the operators as
they also lead to less readable queries.

There was one spot missing with string_agg() two lines down one of the
pg_get_expr() changes. The spots for relation and function
qualifications are worth addressing anyway, and consistent with the
file style. Fixed the extra spot I have noticed, and applied the
result on HEAD.
--
Michael

Michael Paquier <michael@paquier.xyz> writes:

On Tue, Jun 09, 2026 at 09:08:50AM +0000, Bertrand Drouvot wrote:

Now I wonder if we shoud not "protect" the operators too. They could also
lead to wrong results (if not worst).

Kind of true. Still we have been pretty lax about the operators as
they also lead to less readable queries.

We disclaimed security against odd search_paths for these queries long ago,
precisely because wrapping every operator in PG_OPERATOR(pg_catalog.*)
would be far too tedious and destructive of readability --- not to
mention that there are some syntaxes such as IN that don't even offer
the option to do that.

I'm okay with schema-qualifying these table references, mainly because
that preserves consistency with historical style here. But let's not
go further than that.

regards, tom lane

Hi,

On Tue, Jun 09, 2026 at 10:12:48PM -0400, Tom Lane wrote:

Michael Paquier <michael@paquier.xyz> writes:

On Tue, Jun 09, 2026 at 09:08:50AM +0000, Bertrand Drouvot wrote:

Now I wonder if we shoud not "protect" the operators too. They could also
lead to wrong results (if not worst).

Kind of true. Still we have been pretty lax about the operators as
they also lead to less readable queries.

We disclaimed security against odd search_paths for these queries long ago,
precisely because wrapping every operator in PG_OPERATOR(pg_catalog.*)
would be far too tedious and destructive of readability --- not to
mention that there are some syntaxes such as IN that don't even offer
the option to do that.

I do agree that doing so would "destroy" the readability. I did not look in detail,
but what about forcing ALWAYS_SECURE_SEARCH_PATH_SQL before the queries and
restore the search_path once the query is done? (that way that would not impact
the readability)

Regards,

--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com

On 2026-06-10, Bertrand Drouvot wrote:

I do agree that doing so would "destroy" the readability. I did not
look in detail,
but what about forcing ALWAYS_SECURE_SEARCH_PATH_SQL before the queries
and
restore the search_path once the query is done? (that way that would
not impact
the readability)

I think we should just ditch the idea that operators live in schemas.

--
Álvaro Herrera

=?UTF-8?Q?=C3=81lvaro_Herrera?= <alvherre@kurilemu.de> writes:

I think we should just ditch the idea that operators live in schemas.

How would you do that without removing user-defined operators
altogether? (And thereby breaking most extensions.)

regards, tom lane

On Wed, Jun 10, 2026 at 09:51:03AM -0400, Tom Lane wrote:

=?UTF-8?Q?=C3=81lvaro_Herrera?= <alvherre@kurilemu.de> writes:

I think we should just ditch the idea that operators live in schemas.

How would you do that without removing user-defined operators
altogether? (And thereby breaking most extensions.)

I am ready to estimate that the amount of existing users that would be
pissed after such a removal would be higher than the number of users
feeling safer with their queries after this operator capability is
removed. All of them are probably already using their own search_path
for queries they care about anyway.
--
Michael

On 2026-Jun-10, Tom Lane wrote:

=?UTF-8?Q?=C3=81lvaro_Herrera?= <alvherre@kurilemu.de> writes:

I think we should just ditch the idea that operators live in schemas.

How would you do that without removing user-defined operators
altogether? (And thereby breaking most extensions.)

My proposal would be that all operators, both system-defined as well as
user-defined, live in a single namespace -- not that we forbid them from
being created. I expect extensions mostly create operators for the data
types they themselves define, not for existing system datatypes.

I think the idea of public.=(int,int) being different from
pg_catalog.=(int,int) is just too dangerous and trips people up without
giving much valuable functionality. If the extension offers
=(complex,complex) then that's fine: we would still have overloading per
the type system.

I may be missing something though. Care to point out what it is?

--
Álvaro Herrera 48°01'N 7°57'E — https://www.EnterpriseDB.com/
"El que vive para el futuro es un iluso, y el que vive para el pasado,
un imbécil" (Luis Adler, "Los tripulantes de la noche")

=?utf-8?Q?=C3=81lvaro?= Herrera <alvherre@kurilemu.de> writes:

My proposal would be that all operators, both system-defined as well as
user-defined, live in a single namespace -- not that we forbid them from
being created.

Exactly how does that improve anyone's life? It will certainly not
improve query security, rather the reverse. You could no longer put
less-trusted stuff into a schema that's not in your search_path.

Yes, it would stop people from creating operators that are exact
duplicates of system operators, but those are not the problem:
user-defined operators like that are already masked by the lookup
rules, assuming that pg_catalog is searched first as is the normal
case. The thing that is dangerous is a user-defined operator that
is made to capture cases that lack an exact system-operator match
(say, varchar = text). AFAICS your proposal puts those on exactly the
same footing as system-defined operators, and there is no recourse.

regards, tom lane

On 2026-Jun-15, Tom Lane wrote:

=?utf-8?Q?=C3=81lvaro?= Herrera <alvherre@kurilemu.de> writes:

My proposal would be that all operators, both system-defined as well as
user-defined, live in a single namespace -- not that we forbid them from
being created.

Exactly how does that improve anyone's life? It will certainly not
improve query security, rather the reverse. You could no longer put
less-trusted stuff into a schema that's not in your search_path.

I am imagining that only database owners would be able to create
operators. There isn't any case for allowing that for anybody else,
ISTM. How much need is there for "less-trusted" operators, really?

As long as it's not restricted to superusers, there is flexibility
enough.

--
Álvaro Herrera PostgreSQL Developer — https://www.EnterpriseDB.com/