public inbox for [email protected]  

help / color / mirror / Atom feed

From: Etsuro Fujita <[email protected]>
To: Robert Haas <[email protected]>
Cc: [email protected] <[email protected]>
Cc: Corey Huinker <[email protected]>
Subject: Re: use of SPI by postgresImportForeignStatistics
Date: Tue, 16 Jun 2026 21:04:37 +0900
Message-ID: <CAPmGK161TJ4G01eptv3sBqgxa4SNHDE9VRqoC6jy5Y6G3D8yDw@mail.gmail.com> (raw)
In-Reply-To: <CA+TgmoYqMtWb4zLUkT98oFnEkJ=Wz0Pw-ggDJrp9wnSXPzUaeQ@mail.gmail.com>
References: <CA+TgmoYqMtWb4zLUkT98oFnEkJ=Wz0Pw-ggDJrp9wnSXPzUaeQ@mail.gmail.com>

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

view thread (33+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected]
  Subject: Re: use of SPI by postgresImportForeignStatistics
  In-Reply-To: <CAPmGK161TJ4G01eptv3sBqgxa4SNHDE9VRqoC6jy5Y6G3D8yDw@mail.gmail.com>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox