Re: extended stats on partitioned tables - Mailing list pgsql-hackers

From Zhihong Yu
Subject Re: extended stats on partitioned tables
Date
Msg-id CALNJ-vSbxsOOUfCunuRsJ=pzSPnJuTnSVTMLh3486ZKPeN6=qg@mail.gmail.com
Whole thread Raw
In response to Re: extended stats on partitioned tables  (Tomas Vondra <tomas.vondra@enterprisedb.com>)
Responses Re: extended stats on partitioned tables
List pgsql-hackers


On Sat, Dec 11, 2021 at 8:17 PM Tomas Vondra <tomas.vondra@enterprisedb.com> wrote:
Hi,

Attached is a rebased and cleaned-up version of these patches, with more
comments, refactorings etc. Justin and Zhihong, can you take a look?


0001 - Ignore extended statistics for inheritance trees

0002 - Build inherited extended stats on partitioned tables

Those are mostly just Justin's patches, with more detailed comments and
updated commit message. I've considered moving the rel->inh check to
statext_clauselist_selectivity, and then removing the check from
dependencies and MCV. But I decided no to do that, because someone might
be calling those functions directly (even if that's very unlikely).

The one thing bugging me a bit is that the regression test checks only a
GROUP BY query. It'd be nice to add queries testing MCV/dependencies
too, but that seems tricky because most queries will use per-partitions
stats.


0003 - Add stxdinherit flag to pg_statistic_ext_data

This is the patch for master, allowing to build stats for both inherits
flag values. It adds the flag to pg_stats_ext_exprs view to, reworked
how we deal with iterating both flags etc. I've adopted most of the
Justin's fixup patches, except that in plancat.c I've refactored how we
load the stats to process keys/expressions just once.

It has the same issue with regression test using just a GROUP BY query,
but if we add a test to 0001/0002, that'll fix this too.


0004 - Refactor parent ACL check

Not sure about this - I doubt saving 30 rows in an 8kB file is really
worth it. Maybe it is, but then maybe we should try cleaning up the
other ACL checks in this file too? Seems mostly orthogonal to this
thread, though.

Hi,
For patch 1, minor comment:

+           if (planner_rt_fetch(onerel->relid, root)->inh) 

Since the rte (RangeTblEntry*) doesn't seem to be used beyond checking inh, I think it would be better if the above style of checking is used throughout the patch (without introducing rte variable).

Cheers

regards

--
Tomas Vondra
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

pgsql-hackers by date:

Previous
From: Zhihong Yu
Date:
Subject: Re: extended stats on partitioned tables
Next
From: "Gunnar \"Nick\" Bluth"
Date:
Subject: [PATCH] pg_stat_toast