From: Tomas Vondra <tomas.vondra@2ndquadrant.com>
To: Dean Rasheed <dean.a.rasheed@gmail.com>
Cc: Kyotaro HORIGUCHI <horiguchi.kyotaro@lab.ntt.co.jp>
Cc: David Rowley <david.rowley@2ndquadrant.com>
Cc: Alvaro Herrera <alvherre@2ndquadrant.com>
Cc: Michael Paquier <michael@paquier.xyz>
Cc: Andres Freund <andres@anarazel.de>
Cc: Thomas Munro <thomas.munro@enterprisedb.com>
Cc: Bruce Momjian <bruce@momjian.us>
Cc: Mark Dilger <hornschnorter@gmail.com>
Cc: PostgreSQL Hackers <pgsql-hackers@postgresql.org>
Subject: Re: [HACKERS] PATCH: multivariate histograms and MCV lists
Date: Sat, 16 Mar 2019 16:54:01 +0100
Message-ID: <d207e075-9fb3-3a95-7811-8e0ab5292b2a@2ndquadrant.com> (raw)
In-Reply-To: <CAEZATCV+iLZhiHz5jCH=d2-4e+FLS7qqKJY_YseT+36H1u6ArA@mail.gmail.com>
References: <19f76496-dcf3-ccea-dd82-26fbed57b8f5@2ndquadrant.com>
<20190313.131951.178195776.horiguchi.kyotaro@lab.ntt.co.jp>
<CAKJS1f_6qDQj9m2H0jF4bRkZVLpfc7O9E+MxdXrq0wgv0z1NrQ@mail.gmail.com>
<20190314.205623.235434357.horiguchi.kyotaro@lab.ntt.co.jp>
<c2406825-6ebe-26e6-91b3-ba94026546a6@2ndquadrant.com>
<CAEZATCV+iLZhiHz5jCH=d2-4e+FLS7qqKJY_YseT+36H1u6ArA@mail.gmail.com>
On 3/16/19 11:55 AM, Dean Rasheed wrote:
> On Fri, 15 Mar 2019 at 00:06, Tomas Vondra <tomas.vondra@2ndquadrant.com> wrote:
>> I've noticed an annoying thing when modifying type of column not
>> included in a statistics...
>>
>> That is, we don't remove the statistics, but the estimate still changes.
>> But that's because the ALTER TABLE also resets reltuples/relpages:
>>
>> That's a bit unfortunate, and it kinda makes the whole effort to not
>> drop the statistics unnecessarily kinda pointless :-(
>>
>
> Well not entirely. Repeating that test with 100,000 rows, I get an
> initial estimate of 9850 (actual 10,000), which then drops to 2451
> after altering the column. But if you drop the dependency statistics,
> the estimate drops to 241, so clearly there is some benefit in keeping
> them in that case.
>
Sure. What I meant is that to correct the relpages/reltuples estimates
you need to do ANALYZE, which rebuilds the statistics anyway. Although
VACUUM also fixes the estimates, without the stats rebuild.
> Besides, I thought there was no extra effort in keeping the extended
> statistics in this case -- isn't it just using the column
> dependencies, so in this case UpdateStatisticsForTypeChange() never
> gets called anyway?
>
Yes, it does not get called at all. My point was that I was a little bit
confused because the test says "check change of unrelated column type
does not reset the MCV statistics" yet the estimates do actually change.
I wonder why we reset the relpages/reltuples to 0, instead of retaining
the original values, though. That would likely give us better density
estimates in estimate_rel_size, I think.
So I've tried doing that, and I've included it as 0001 into the patch
series. It seems to work, but I suppose the reset is there for a reason.
In any case, this is a preexisting issue, independent of what this patch
does or changes.
I've discovered another issue, though. Currently, clauselist_selectivity
has this as the very beginning:
/*
* If there's exactly one clause, just go directly to
* clause_selectivity(). None of what we might do below is relevant.
*/
if (list_length(clauses) == 1)
return clause_selectivity(root, (Node *) linitial(clauses),
varRelid, jointype, sjinfo);
Which however fails with queries like this:
WHERE (a = 1 OR b = 1)
because clauselist_selectivity sees it as a single clause, passes it to
clause_selectivity and the OR-clause handling simply relies on
(s1 + s2 - s1 * s2)
which entirely ignores the multivariate stats. The other similar places
in clause_selectivity() simply call clauselist_selectivity() so that's
OK, but OR-clauses don't do that.
For functional dependencies this is not a huge issue because those apply
only to AND-clauses. But there were proposals to maybe apply them to
other types of clauses, in which case it might become issue.
I think the best fix is moving the optimization after the multivariate
stats are applied. The only alternative I can think of is modifying
clauselist_selectivity so that it can be executed on OR-clauses. But
that seems much more complicated than the former option for almost no
other advantages.
I've also changed how statext_is_compatible_clause_internal() handles
the attnums bitmapset - you were right in your 3/10 message that we can
just pass the value, without creating a local bitmapset. So I've just
done that.
regards
--
Tomas Vondra http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Attachments:
[application/gzip] 0001-retain-reltuples-relpages-on-ALTER-TABLE.patch.gz (1.7K, ../d207e075-9fb3-3a95-7811-8e0ab5292b2a@2ndquadrant.com/2-0001-retain-reltuples-relpages-on-ALTER-TABLE.patch.gz)
download
[application/gzip] 0002-multivariate-MCV-lists.patch.gz (39.6K, ../d207e075-9fb3-3a95-7811-8e0ab5292b2a@2ndquadrant.com/3-0002-multivariate-MCV-lists.patch.gz)
download
[application/gzip] 0003-multivariate-histograms.patch.gz (48.7K, ../d207e075-9fb3-3a95-7811-8e0ab5292b2a@2ndquadrant.com/4-0003-multivariate-histograms.patch.gz)
download
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@2ndquadrant.com, dean.a.rasheed@gmail.com, horiguchi.kyotaro@lab.ntt.co.jp, david.rowley@2ndquadrant.com, alvherre@2ndquadrant.com, michael@paquier.xyz, andres@anarazel.de, thomas.munro@enterprisedb.com, bruce@momjian.us, hornschnorter@gmail.com
Subject: Re: [HACKERS] PATCH: multivariate histograms and MCV lists
In-Reply-To: <d207e075-9fb3-3a95-7811-8e0ab5292b2a@2ndquadrant.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