Re: use of SPI by postgresImportForeignStatistics

From: Corey Huinker <corey(dot)huinker(at)gmail(dot)com>
To: Robert Haas <robertmhaas(at)gmail(dot)com>
Cc: Etsuro Fujita <etsuro(dot)fujita(at)gmail(dot)com>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: use of SPI by postgresImportForeignStatistics
Date: 2026-06-29 08:14:11
Message-ID: CADkLM=c9BEuA9vPjE9Wco6+Lz-HSnnmBZrFhi=-yNi0K8tmZug@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

>
>
> Requiring Datums simplifies things greatly inside the existing functions,
> but pushes that complexity to the caller. My first proposal was to keep
> complexity to an absolute minimum on the caller side, but your comments
> make me think there's considerable tolerance for more complexity on the
> caller side.
>

I've had some time to look this over, and it's pretty clear that we don't
actually need the whole positional FunctionCallInfo, we just need the
fcinfo->args of it. We also need the ability to pass statistical values
along with the values that comprise the key of the
relation/attribute/object to be modified.

There are important differences in the parameters needed by the different
types of functions. The pg_restore_*() SQL function calls need to identify
the schema+relname of the relation being modified, and already have all of
the values as typed Datums, whereas the internal API calls already have an
identified and locked open Relation, but all of the statistical parameters
are just cstrings.

The solution I chose was to create common functions that take a relation
object and an array of just the statistical parameters. This requires the
pg_restore_* functions to resolve and lock the relation themselves, and
then carry forward just the subset of parameters that are statistics (right
types, but wrong number of arguments). Conversely, the internal API
functions need to map their array of cstring values to the correctly typed
Datums (wrong types, but right number of arguments in the right order).

Attached is v2. The patches are broken down into very small bites to aid
digestion and to confirm that tests pass after each comparatively minor
change.

0001-0003: standardize enum values to further indicate which enum the
values belong to. This will be valuable later when values from an array of
one length have to be mapped to values in an array of a different
length. One patch each for relation stats, attribute stats, extended stats.

0004: Refactors the stats_check_* functions that previously took an fcinfo,
but now just take a NullableDatum array.

0005-0007: Change the function signature of
(relation|attribute|extended)_statistics_update() to no longer take
FunctionCallInfo as a parameter and instead take a NullableDatum[].

0008: Changes stats_fill_fcinfo_from_args() which maps a keyword args
fcinfo to a positional fcinfo to map directly to a NullableDatum[], and
changes all callers to use the new function and signature.

0009: rename relation_statistics_update -> update_relstats and
relation_stats_argnum -> relation_args_argnum to make room for
public-facing objects in the following patch.
0010: Introduce new public facing relation_statistics_update, which takes
isnull/string arrays that will be filled out by the caller, currently
postgres_fdw, and calls update_relstats same as pg_restore_relation_stats().

0011: same as 0009, but for attribute stats
0012: same as 0010, but for attribute stats

0013: remove SPI calls from postgres_fdw, and use the newly public
relation_statistics_update() and attribute_statistics_update() functions
instead.

Attachment Content-Type Size
v2-0004-Stop-using-fcinfo-in-stats_check-functions.patch application/octet-stream 10.4 KB
v2-0002-Rename-attribute_stats_argnum-enumeriations.patch application/octet-stream 16.1 KB
v2-0001-Rename-relation_stats_argnum-enumeriations.patch application/octet-stream 4.2 KB
v2-0003-Rename-extended-stats-enumerations.patch application/octet-stream 28.1 KB
v2-0005-Make-relation_statistics_update-stop-using-fcinfo.patch application/octet-stream 3.6 KB
v2-0006-Make-attribute_statistics_update-stop-using-fcinf.patch application/octet-stream 11.0 KB
v2-0008-Change-stats_fill_fcinfo_from_arg_pairs-to-Nullab.patch application/octet-stream 6.5 KB
v2-0007-Make-extended_statistics_update-stop-using-fcinfo.patch application/octet-stream 6.7 KB
v2-0010-Add-relation_statistics_update-refactor-update_re.patch application/octet-stream 12.5 KB
v2-0009-Rename-relation_statistics_update-and-relation_st.patch application/octet-stream 2.1 KB
v2-0012-Add-attribute_statistics_update-refactor-update_a.patch application/octet-stream 23.3 KB
v2-0013-Replace-statistics-import-SPI-calls-with-new-impo.patch application/octet-stream 22.9 KB
v2-0011-Rename-attribute_statistics_update.patch application/octet-stream 2.6 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Kyotaro Horiguchi 2026-06-29 08:22:07 Re: Report oldest xmin source when autovacuum cannot remove tuples
Previous Message Ayush Tiwari 2026-06-29 08:05:25 Re: Remove the refint contrib module (for v20)