Re: extended stats on partitioned tables

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

In response to

Responses

Browse pgsql-hackers by date

  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