Hi Robert, On Tue, Jun 16, 2026 at 2:50 AM Robert Haas <[email protected]> wrote: > 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!).
Good catch! > 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? Ugh. As pg_restore_attribute_stats/pg_restore_relation_stats are volatile functions, SPI executes the queries in read-write mode, causing an error, like "schema "foo" does not exist", or other unexpected results in such a case. Not sure what to do about this issue right now. Will think about it some more. > 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. I thought it would be a good idea to use pg_restore_attribute_stats/pg_restore_relation_stats, because future changes in attribute/relation stats would be absorbed by these functions, which would lower the maintenance cost of this feature. Thanks for the comments! Best regards, Etsuro Fujita
