On 12/12/21 18:52, Justin Pryzby wrote:
> + * XXX Can't we simply look at rte->inh?
> + */
> + inh = root->append_rel_array == NULL ? false :
> + root->append_rel_array[onerel->relid]->parent_relid != 0;
>
> I think so. That's what I came up with while trying to figured this out, and
> it's no great surprise that it needed to be cleaned up - thanks.
>
OK, fixed.
> In your 0003 patch, the "if inh: break" isn't removed from examine_variable(),
> but the corresponding thing is removed everywhere else.
>
Ah, you're right. And it wasn't updated in the 0002 patch either - it
should do the relkind check too, to allow partitioned tables. Fixed.
> In 0003, mcv_clauselist_selectivity still uses simple_rte_array rather than
> rt_fetch.
>
That's mostly a conscious choice, so that I don't have to include
parsetree.h. But maybe that'd be better ...
> The regression tests changed as a result of not populating stx_data; I think
> it's may be better to update like this:
>
> SELECT stxname, stxdndistinct, stxddependencies, stxdmcv, stxoid IS NULL
> FROM pg_statistic_ext s LEFT JOIN pg_statistic_ext_data d
> ON d.stxoid = s.oid
> WHERE s.stxname = 'ab1_a_b_stats';
>
Not sure I understand. Why would this be better than inner join?
> There's this part about documentation for the changes in backbranches:
>
> On Sun, Sep 26, 2021 at 03:25:50PM -0500, Justin Pryzby wrote:
>> 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;
>
> spellcheck: inheritence should be inheritance.
>
Thanks, fixed. Can you read through the commit messages and check the
attribution is correct for all the patches?
> All for now. I'm going to update the regression tests for dependencies and the
> other code paths.
>
Thanks!
--
Tomas Vondra
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company