Hi, On Fri, Mar 08, 2024 at 01:35:40AM -0500, Corey Huinker wrote: > Anyway, here's v7. Eagerly awaiting feedback.
Thanks! A few random comments: 1 === + The purpose of this function is to apply statistics values in an + upgrade situation that are "good enough" for system operation until Worth to add a few words about "influencing" the planner use case? 2 === +#include "catalog/pg_type.h" +#include "fmgr.h" Are those 2 needed? 3 === + if (!HeapTupleIsValid(ctup)) + elog(ERROR, "pg_class entry for relid %u vanished during statistics import", s/during statistics import/when setting statistics/? 4 === +Datum +pg_set_relation_stats(PG_FUNCTION_ARGS) +{ . . + table_close(rel, ShareUpdateExclusiveLock); + + PG_RETURN_BOOL(true); Why returning a bool? (I mean we'd throw an error or return true). 5 === + */ +Datum +pg_set_attribute_stats(PG_FUNCTION_ARGS) +{ This function is not that simple, worth to explain its logic in a comment above? 6 === + if (!HeapTupleIsValid(tuple)) + { + relation_close(rel, NoLock); + PG_RETURN_BOOL(false); + } + + attr = (Form_pg_attribute) GETSTRUCT(tuple); + if (attr->attisdropped) + { + ReleaseSysCache(tuple); + relation_close(rel, NoLock); + PG_RETURN_BOOL(false); + } Why is it returning "false" and not throwing an error? (if ok, then I think we can get rid of returning a bool). 7 === + * If this relation is an index and that index has expressions in + * it, and the attnum specified s/is an index and that index has/is an index that has/? Regards, -- Bertrand Drouvot PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com