MIME-Version: 1.0
From: Robert Haas <robertmhaas@gmail.com>
Date: Mon, 15 Jun 2026 13:50:46 -0400
Message-ID: 
 <CA+TgmoYqMtWb4zLUkT98oFnEkJ=Wz0Pw-ggDJrp9wnSXPzUaeQ@mail.gmail.com>
Subject: use of SPI by postgresImportForeignStatistics
To: Etsuro Fujita <etsuro.fujita@gmail.com>,
	"pgsql-hackers@postgresql.org" <pgsql-hackers@postgresql.org>,
 Corey Huinker <corey.huinker@gmail.com>
Content-Type: text/plain; charset="UTF-8"
Archived-At: 
 <https://www.postgresql.org/message-id/CA%2BTgmoYqMtWb4zLUkT98oFnEkJ%3DWz0Pw-ggDJrp9wnSXPzUaeQ%40mail.gmail.com>
Precedence: bulk

Hi,

I'm concerned about the way that postgresImportForeignStatistics is
doing what it does. First, it's using SPI to execute two constant SQL
queries, attimport_sql and attclear_sql. However, it doesn't seem to
set the search_path, and these queries aren't fully robust against a
possibly-unsafe search_path (e.g. the call to
pg_restore_attribute_stats is schema-qualified, but the type names are
not!). Furthermore, the function we're calling takes a schema name and
a relation name as two separate arguments, rather than a single OID
argument. I'm not sure it's completely guaranteed that we'll end up
affecting the same relation that we locked in
postgresImportForeignStatistics, because e.g. what if the containing
schema is being concurrently renamed?

But even if it is absolutely guaranteed that we latch onto the correct
relation, this seems like the fundamentally wrong way to do this kind
of thing. It seems crazy to me that instead of exposing an interface
that would be well-suited to direct use by an FDW, the
statistics-import stuff exposes an interface that can only be
reasonably called via the FunctionCallInfo interface, which then
results in postgres_fdw needing to jump through hoops to construct and
execute SQL statements.

This doesn't look entirely easy to straighten out. A function like
attribute_statistics_update() is just not a good idea -- it mixes
together considerations that only exist when you want to call
something from SQL with general validity checking that's needed
regardless.

But I don't think we should ship this like this. Best case scenario,
it's overly complicated. Medium case scenario, it's also unreliable.
Worst case scenario, there are security vulnerabilities in here.

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