Re: Statistics Import and Export

On Thu, Mar 6, 2025 at 3:48 AM Jeff Davis <pgsql(at)j-davis(dot)com> wrote:

> 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.