> 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
