Jeff Davis <pg...@j-davis.com> writes:
> Thank you, this has improved a lot and the fundamentals are very close.
> I think it could benefit from a bit more time to settle on a few
> issues:
Yeah ... it feels like we aren't quite going to manage to get this
over the line for v17. We could commit with the hope that these
last details will get sorted later, but that path inevitably leads
to a mess.
> 1. SECTION_NONE. Conceptually, stats are more like data, and so
> intuitively I would expect this in the SECTION_DATA or
> SECTION_POST_DATA. However, the two most important use cases (in my
> opinion) don't involve dumping the data: pg_upgrade (data doesn't come
> from the dump) and planner simulations/repros. Perhaps the section we
> place it in is not a critical decision, but we will need to stick with
> it for a long time, and I'm not sure that we have consensus on that
> point.
I think it'll be a serious, serious error for this not to be
SECTION_DATA. Maybe POST_DATA is OK, but even that seems like
an implementation compromise not "the way it ought to be".
> 2. We changed the stats import function API to be VARIADIC very
> recently. After we have a bit of time to think on it, I'm not 100% sure
> we will want to stick with that new API. It's not easy to document,
> which is something I always like to consider.
Perhaps. I think the argument of wanting to be able to salvage
something even in the presence of unrecognized stats types is
stronger, but I agree this could use more time in the oven.
Unlike many other things in this patch, this would be nigh
impossible to reconsider later.
> 3. The error handling also changed recently to change soft errors (i.e.
> type input errors) to warnings. I like this change but I'd need a bit
> more time to get comfortable with how this is done, there is not a lot
> of precedent for doing this kind of thing.
I don't think there's much disagreement that that's the right thing,
but yeah there could be bugs or some more to do in this area.
regards, tom lane
