On Sat, Dec 11, 2021 at 9:14 PM Tomas Vondra <tomas.von...@enterprisedb.com> wrote:
> > > On 12/12/21 05:38, Zhihong Yu wrote: > > > > > > On Sat, Dec 11, 2021 at 8:17 PM Tomas Vondra > > <tomas.von...@enterprisedb.com <mailto:tomas.von...@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 3, in commit message: > > > > and there no clear winner. -> and there is no clear winner. > > > > and it seem wasteful -> and it seems wasteful > > > > The there may be -> There may be > > > > Thanks, will fix. > > > + /* skip statistics with mismatching stxdinherit value */ > > + if (stat->inherit != rte->inh) > > > > Should a log be added for the above case ? > > > > Why should we log this? It's an entirely expected case - there's a > mismatch between inheritance for the relation and statistics, simply > skipping it is the right thing to do. > Hi, I agree that skipping should be fine (to avoid too much logging). Thanks > > + * Determine if we'redealing with inheritance tree. > > > > There should be a space between re and dealing. > > > > Thanks, will fix. > > > regards > > -- > Tomas Vondra > EnterpriseDB: http://www.enterprisedb.com > The Enterprise PostgreSQL Company >
