On Sat, Sep 25, 2021 at 05:31:52PM -0500, Justin Pryzby wrote:
> It seems like your patch should also check "inh" in examine_variable and
> statext_expressions_load.
I tried adding that - I mostly kept my patches separate.
Hopefully this is more helpful than a complication.
I added at: https://commitfest.postgresql.org/35/3332/
+ /* create only the "stxdinherit=false", because that always exists */
+ datavalues[Anum_pg_statistic_ext_data_stxdinherit - 1] = ObjectIdGetDatum(false);
That'd be confusing for partitioned tables, no?
They'd always have an row with no data.
I guess it could be stxdinherit = BoolGetDatum(rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE).
(not ObjectIdGetDatum).
Then, that affects the loops which delete the tuples - neither inh nor !inh is
guaranteed, unless you check relkind there, too.
BTW, you'd need to add an "inherited" column to \dX if you added the "built"
data back.
Also, I think in backbranches we should document what's being stored in
pg_statistic_ext, since it's pretty unintuitive:
- noninherted stats (FROM ONLY) for inheritence parents;
- inherted stats (FROM *) for partitioned tables;
I think the !inh decision in 859b3003de was basically backwards.
I think it'd be rare for someone to put extended stats on a parent for
improving plans involving FROM ONLY.
But it's not worth trying to fix now, since it would change plans in
irreversible ways. Also, if the stx data were already populated, users would
have to run a manual analyze after upgrading to populate the catalog with the
data the planner would expect in the new version, or else it would end up being
the opposite of the issue I mentioned: non-inherited stats (from before the
upgrade) would be applied by the planner (after the upgrade) to inherited
queries.