From: | Tomas Vondra <tomas(dot)vondra(at)enterprisedb(dot)com> |
---|---|
To: | Justin Pryzby <pryzby(at)telsasoft(dot)com> |
Cc: | Zhihong Yu <zyu(at)yugabyte(dot)com>, Jaime Casanova <jcasanov(at)systemguards(dot)com(dot)ec>, pgsql-hackers(at)postgresql(dot)org |
Subject: | Re: extended stats on partitioned tables |
Date: | 2021-12-12 21:29:39 |
Message-ID: | ba3dd29f-6fb1-2bb5-87d7-92b671c79b2a@enterprisedb.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
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
Attachment | Content-Type | Size |
---|---|---|
0004-Refactor-parent-ACL-check-20211212b.patch | text/x-patch | 6.4 KB |
0003-Add-stxdinherit-flag-to-pg_statistic_ext_d-20211212b.patch | text/x-patch | 37.4 KB |
0002-Build-inherited-extended-stats-on-partitio-20211212b.patch | text/x-patch | 9.5 KB |
0001-Ignore-extended-statistics-for-inheritance-20211212b.patch | text/x-patch | 7.6 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Thomas Munro | 2021-12-12 21:31:29 | Re: Windows now has fdatasync() |
Previous Message | Tom Lane | 2021-12-12 18:27:22 | Re: extended stats on partitioned tables |