Date: Mon, 29 Mar 2021 13:36:24 -0500
From: Justin Pryzby <pryzby@telsasoft.com>
To: Tomas Vondra <tomas.vondra@enterprisedb.com>
Cc: pgsql-hackers@postgresql.org
Subject: Re: Merging statistics from children instead of re-sampling
 everything
Message-ID: <20210329183624.GC4431@telsasoft.com>
References: <c3162511-0e80-039e-765c-1d02d0fd0bab@enterprisedb.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <c3162511-0e80-039e-765c-1d02d0fd0bab@enterprisedb.com>
User-Agent: Mutt/1.9.4 (2018-02-28)
Archived-At: 
 <https://www.postgresql.org/message-id/20210329183624.GC4431%40telsasoft.com>
Precedence: bulk

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)

> +     if (*mcv_items > default_statistics_target)
> +             n = default_statistics_target;

It should use any non-default stats target of the parent's column

> +		 * 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