Re: use of SPI by postgresImportForeignStatistics
Corey Huinker <corey.huinker@gmail.com>
From: Corey Huinker <corey.huinker@gmail.com>
To: Robert Haas <robertmhaas@gmail.com>
Cc: Etsuro Fujita <etsuro.fujita@gmail.com>, "pgsql-hackers@postgresql.org" <pgsql-hackers@postgresql.org>
Date: 2026-06-29T08:14:11Z
Lists: pgsql-hackers
Attachments
- v2-0004-Stop-using-fcinfo-in-stats_check-functions.patch (application/octet-stream) patch v2-0004
- v2-0002-Rename-attribute_stats_argnum-enumeriations.patch (application/octet-stream) patch v2-0002
- v2-0001-Rename-relation_stats_argnum-enumeriations.patch (application/octet-stream) patch v2-0001
- v2-0003-Rename-extended-stats-enumerations.patch (application/octet-stream) patch v2-0003
- v2-0005-Make-relation_statistics_update-stop-using-fcinfo.patch (application/octet-stream) patch v2-0005
- v2-0006-Make-attribute_statistics_update-stop-using-fcinf.patch (application/octet-stream) patch v2-0006
- v2-0008-Change-stats_fill_fcinfo_from_arg_pairs-to-Nullab.patch (application/octet-stream) patch v2-0008
- v2-0007-Make-extended_statistics_update-stop-using-fcinfo.patch (application/octet-stream) patch v2-0007
- v2-0010-Add-relation_statistics_update-refactor-update_re.patch (application/octet-stream) patch v2-0010
- v2-0009-Rename-relation_statistics_update-and-relation_st.patch (application/octet-stream) patch v2-0009
- v2-0012-Add-attribute_statistics_update-refactor-update_a.patch (application/octet-stream) patch v2-0012
- v2-0013-Replace-statistics-import-SPI-calls-with-new-impo.patch (application/octet-stream) patch v2-0013
- v2-0011-Rename-attribute_statistics_update.patch (application/octet-stream) patch v2-0011
> > > 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.