On Thu, Mar 6, 2025 at 3:48 AM Jeff Davis <pg...@j-davis.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.
