On Wed, 2025-03-05 at 23:04 -0500, Corey Huinker wrote:
>
> Anyway, here's a rebased set of the existing up-for-consideration
> patches, plus the optimization of avoiding querying on non-expression
> indexes.
Comments on 0003:
* All the argument names for pg_restore_attribute_stats match pg_stats,
except relname vs tablename. There doesn't appear to be a great answer
here, because "relname" is the natural name to use for
pg_restore_relation_stats(), so either the two restore functions will
be inconsistent, or the argument name of one of them will be
inconsistent with its respective catalog. I assume that's the
reasoning?
Correct, either we use 'tablename' to describe indexes as well, or we diverge from the system view's naming.
* Now that it's doing a namespace lookup, we should also check for the
USAGE privilege on the namespace, right?
Unless some check was being done by the 'foo.bar'::regclass cast, I understand why we should add one.
Based on the other changes we've made to this feature, I think 0003
makes sense, so I'm inclined to move ahead with it, but I'm open to
opinions.
If we do, we'll want to change downgrade the following errors to warn+return false:
* stats_check_required_arg()
* stats_lock_check_privileges()
* RecoveryInProgress
* specified both attnum and argnum
* attname/attnum does not exist, or is system column
0004 looks straightforward, though perhaps we should move some of the
code into a static function rather than indenting so many lines.
I agree, but the thread conversation had already shifted to doing just one single call to pg_stats, so this was just a demonstration.
Did you collect performance results for 0004?
No, as I wasn't sure that I could replicate Andres' setup, and the conversation was quickly moving to the aforementioned single-query idea.
