pg.ddx.io  pgsql-hackers@postgresql.org mailing list archive  
help / color / mirror / Atom feed
From: Tomas Vondra <tomas.vondra@enterprisedb.com>
To: Justin Pryzby <pryzby@telsasoft.com>
Cc: pgsql-hackers@postgresql.org
Subject: Re: Merging statistics from children instead of re-sampling everything
Date: Mon, 29 Mar 2021 21:24:19 +0200
Message-ID: <0c3db54d-c06d-7fef-ad47-8aa4bd8485b2@enterprisedb.com> (raw)
In-Reply-To: <20210329183624.GC4431@telsasoft.com>
References: <c3162511-0e80-039e-765c-1d02d0fd0bab@enterprisedb.com>
	<20210329183624.GC4431@telsasoft.com>



On 3/29/21 8:36 PM, Justin Pryzby wrote:
> Thanks for taking a fresh look at this.
> 
> As you've written it, this can apply to either/both partitioned or inheritence.
> I imagine when "MERGE" goes away, this should apply only to partitioned tables.
> (Actually, personally I would advocate to consider applying it to *both*, but I
> don't think that's been the tendency over the last 4 years.  I wrote here about
> some arguably-gratuitous differences between inheritence and partitioning.
> https://www.postgresql.org/message-id/20180601221428.GU5164@telsasoft.com)
> 

Haven't thought about that too much at this point, but I don't see any
reason to not apply it only to both cases. I might be missing something,
but the fact that with declarative partitioning we analyze the children,
while with inheritance we don't, seems a bit strange. Not sure.

>> +     if (*mcv_items > default_statistics_target)
>> +             n = default_statistics_target;
> 
> It should use any non-default stats target of the parent's column
> 

Yeah. That's a simplification, the non-PoC code would have to look at
per-column statistics target for the target / all the children, and make
some calculation based on that. I ignored that in the PoC.

>> +		 * ignore anything but valid leaf relatiins with data, but release
> 
> sp: relatiins.
> 
>> +				elog(WARNING, "stats for %d %d not found",
>> +                                        RelationGetRelid(rels[j]), vacattrstats[i]->attr->attnum);
> 
> should be %u %d
> 
> This code duplication is undesirable:
>> +	/* Log the action if appropriate */
>> +	 * Determine which columns to analyze
Yeah. Fine for PoC, but needs to be cleaned up in future patch.

regards

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





view thread (17+ messages)  latest in thread

Message-ID: <0c3db54d-c06d-7fef-ad47-8aa4bd8485b2@enterprisedb.com>
Permalink:  ../0c3db54d-c06d-7fef-ad47-8aa4bd8485b2@enterprisedb.com/
Also on:    postgresql.org/message-id/0c3db54d-c06d-7fef-ad47-8aa4bd8485b2@enterprisedb.com

 · 

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: pgsql-hackers@postgresql.org
  Cc: tomas.vondra@enterprisedb.com, pryzby@telsasoft.com
  Subject: Re: Merging statistics from children instead of re-sampling everything
  In-Reply-To: <0c3db54d-c06d-7fef-ad47-8aa4bd8485b2@enterprisedb.com>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox