Re: Extended Statistics set/restore/clear functions.

> Anyway, I have begun a review of the backend changes, and found one
> independent piece that was useful, leading to 32e27bd32082 that I have
> just applied after some renames and a couple of fixes.
>

:)

>
> A shocking thing here is that we build a RangeVar for the relation of
> the extended stats object based on the *extstat namespace*, not the
> *relation namespace*, and that we do so *after* opening the extstat
> catalog. Anyway, I think that we need to redesign that, and for
> consistency's sake do a RangeVar lookup *before* even trying to touch
> the stats data or open its catalogs. That would be beneficial on
> consistency ground, and one piece where I think that patch is designed
> wrongly is that we should give in input of the new clear and restore
> functions the *schema* and *relation* names of the table we expect to
> find, so as we can enforce first a clean RangeVar lookup, then
> manipulate the stats data. This relates to 688dc6299a5b, as well,
> except that we would be stuck with an incorrect design after release
> if we are not careful because the function schema would be stuck in
> time. This has to be right from the start.
>

Correct, that was done in the v7 patchset to conform to 688dc6299a5b.

All four names are available through pg_stats_ext, so the additional
requirements for pg_dump may not be a an issue. However, I do have some
vague memory that all four names was a problem in some way beyond just
giving the parameters more chances to disagree with reality. We'll find out
soon enough...

> All these changes have given me the attached patch for the clear()
> function. Another piece is that we did not really check that a role
> has the MAINTAIN rights of the relation where we want to manipulate
> the extended stats.
>
> A final thing is the location of the new SQL function code, let's put
> that into a new file, named extended_stats_funcs.c in the patch.
> There is nothing in the new restore and clear functions that require
> extended_stats.c.
>
> Finally, for the clear() part, the attached version addresses
> everything I have found during my review. We will need to make the
> restore() part follow the same design model with the Rangevar lookups
> of the parent relation, or we'll have trouble waiting ahead.
> --
> Michael
>

+1