Date: Mon, 13 Dec 2021 07:53:48 -0600
From: Justin Pryzby <pryzby@telsasoft.com>
To: Tomas Vondra <tomas.vondra@enterprisedb.com>
Cc: Zhihong Yu <zyu@yugabyte.com>,
	Jaime Casanova <jcasanov@systemguards.com.ec>,
	pgsql-hackers@postgresql.org
Subject: Re: extended stats on partitioned tables
Message-ID: <20211213135348.GT17618@telsasoft.com>
References: <20211007202646.GA9522@ahch-to>
 <20211007224543.GH27491@telsasoft.com>
 <46f37069-55cb-db6a-a15a-e351a2606592@enterprisedb.com>
 <20211103231904.GP17618@telsasoft.com>
 <689accb1-76e8-5115-6ed0-336749cdca57@enterprisedb.com>
 <20211203052411.GU17618@telsasoft.com>
 <CALNJ-vQSpBxw-gjZ--p-vo_yr2yHaXXOn_g8i=DQam_aAW3GwQ@mail.gmail.com>
 <8052505e-2dd2-e8cb-26c9-cbb5dff7d750@enterprisedb.com>
 <20211212213210.GP17618@telsasoft.com>
 <d11ae731-4366-ee5f-9ffd-0bd61f5e94a6@enterprisedb.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <d11ae731-4366-ee5f-9ffd-0bd61f5e94a6@enterprisedb.com>
User-Agent: Mutt/1.9.4 (2018-02-28)
Archived-At: 
 <https://www.postgresql.org/message-id/20211213135348.GT17618%40telsasoft.com>
Precedence: bulk

On Sun, Dec 12, 2021 at 11:23:19PM +0100, Tomas Vondra wrote:
> On 12/12/21 22:32, Justin Pryzby wrote:
> > On Sun, Dec 12, 2021 at 05:17:10AM +0100, Tomas Vondra wrote:
> > > 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.
> > 
> > You mean because the quals are pushed down to the scan node.
> > 
> > Does that indicate a deficiency ?
> > 
> > If extended stats are collected for a parent table, selectivity estimates based
> > from the parent would be better; but instead we use uncorrected column
> > estimates from the child tables.
> > 
> > From what I see, we could come up with a way to avoid the pushdown, involving
> > volatile functions/foreign tables/RLS/window functions/SRF/wholerow vars/etc.
> > But would it be better if extended stats objects on partitioned tables were to
> > collect stats for both parent AND CHILD ?  I'm not sure.  Maybe that's the
> > wrong solution, but maybe we should still document that extended stats on
> > (empty) parent tables are often themselves not used/useful for selectivity
> > estimates, and the user should instead (or in addition) create stats on child
> > tables.
> > 
> > Or, maybe if there's no extended stats on the child tables, stats on the parent
> > table should be consulted ?
> 
> Maybe, but that seems like a mostly separate improvement. At this point I'm
> interested only in testing the behavior implemented in the current patches.

I don't want to change the scope of the patch, or this thread, but my point is
that the behaviour already changed once (the original regression) and now we're
planning to change it again to fix that, so we ought to decide on the expected
behavior before writing tests to verify it.

I think it may be impossible to use the "dependencies" statistic with inherited
stats.  Normally the quals would be pushed down to the child tables.  But, if
they weren't pushed down, they'd be attached to something other than a scan
node on the parent table, so the stats on that table wouldn't apply (right?).  

Maybe the useless stats types should have been prohibited on partitioned tables
since v10.  It's too late to change that, but perhaps now they shouldn't even
be collected during analyze.  The dependencies and MCV paths are never called
with rte->inh==true, so maybe we should Assert(!inh), or add a comment to that
effect.  Or the regression tests should "memorialize" the behavior.  I'm still
thinking about it.

-- 
Justin