Re: use of SPI by postgresImportForeignStatistics - Mailing list pgsql-hackers

From Corey Huinker
Subject Re: use of SPI by postgresImportForeignStatistics
Date
Msg-id CADkLM=dy18dDJ+j3-_Ofn11vt5J7zF33oTWnE6N=ePc83uS91A@mail.gmail.com
Whole thread
In response to Re: use of SPI by postgresImportForeignStatistics  (Robert Haas <robertmhaas@gmail.com>)
Responses Re: use of SPI by postgresImportForeignStatistics
Re: use of SPI by postgresImportForeignStatistics
List pgsql-hackers
>
> Would you be ok with a function like attribute_statistics_update, but purely with cstring args? Obviously the callers would have to modify their calls each time a new stat param is added, but that is seemingly preferable for you than the current situation.

I feel pretty strongly that we want a way to identify the relation by
OID rather than by schema and name.

Previous versions of pg_set_attribute_stats() were like that, but it went away as the use-cases folded into pg_restore_attribute_stats(). So it's do-able, but will make sharing code with the existing function harder in the near term. We may have to settle for something that takes the oid, but then plugs into the existing functions until such time as we can refactor the existing callers.
 
I'm not sure whether attributes
should be identified by name or by number in this context. Apart from
those things, if passing strings and leaving it up to the called
function to interpret them makes most sense, that's OK with me.

Identifying attributes by attnum is mostly for index expressions, so attname would be the preference here.
 
I agree with you that getting rid of SPI is a much higher priority
than getting rid of the FunctionCallInfo stuff altogether. I could
live with DirectFunctionCall if we don't have a better option.

Ok, good to know that I have that as a fallback position.

I think the right first step is to create the functions import_relation_stats() and import_attribute_stats() which take the oid of the destination relation and the rest of the parameters are const char pointers, which is the ideal case for a bunch of values being fetched from PQgetValue(). Those functions will for now just make the call to attribute_statistics_update() / relation_statistics_update(), but at least it will be localized inside relation_stats.c and attribute_stats.c where we can make internal refactors without disturbing anyone else.

Now, we *could* make these 2 new functions take the oid/oid+attname followed by a variadic array of keyname+value string pairs like the existing pg_restore_*_stats() functions, so the functions call signature would be somewhat future-proofed. Let me know if that violates your vision of what this should be.
 
 I'm a
bit suspicious that this will lead to code bloat in postgres_fdw or
elsewhere, but maybe not.

postgres_fdw will get slimmer for not having SPI in it. relation_stats.c and attribute_stats.c will have some temporary bloat until things can be refactored.

pgsql-hackers by date:

Previous
From: Chao Li
Date:
Subject: Re: Make frontend programs relink after libpgfeutils changes
Next
From: Enrique Sánchez
Date:
Subject: Re: Show estimated number of groups for IncrementalSort in EXPLAIN