From: Tomas Vondra <tomas.vondra@2ndquadrant.com>
To: Dean Rasheed <dean.a.rasheed@gmail.com>
Cc: David Rowley <david.rowley@2ndquadrant.com>
Cc: Kyotaro HORIGUCHI <horiguchi.kyotaro@lab.ntt.co.jp>
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: Tue, 26 Mar 2019 16:12:35 +0100
Message-ID: <20190326151235.GA3088@development> (raw)
In-Reply-To: <CAEZATCURhZv0kMsy9PoWvx+4nuvmfGi42f9kte7ZNyWOJRKKjw@mail.gmail.com>
References: <c2406825-6ebe-26e6-91b3-ba94026546a6@2ndquadrant.com>
<CAEZATCVazGRDjbZpRF6r-Asiv_-U8vcT-VA0oSZribhhmDUQHQ@mail.gmail.com>
<37ce47c5-e0cf-c6d3-6923-6b8cfd1cc6b1@2ndquadrant.com>
<CAEZATCXfk-Syby-MWLFfN-v=7HFEm-QYyX+tP=NVZ+iNw8ELUw@mail.gmail.com>
<c08dd2ce-14f0-463b-763c-13af49926d03@2ndquadrant.com>
<CAKJS1f_naKM6kzeUdDs61=++fYdtsk4T5586DZObRvUXPVmL9Q@mail.gmail.com>
<fd4c5223-6c1a-c2d5-c0e0-53db789a0bdc@2ndquadrant.com>
<CAKJS1f9o23x-utM3Jk2wji_tQR-CZgyPP5cqQgt1e_zhHMYQpw@mail.gmail.com>
<add2f0d3-4fbc-af7c-e02e-0f2b2d6dedc3@2ndquadrant.com>
<CAEZATCURhZv0kMsy9PoWvx+4nuvmfGi42f9kte7ZNyWOJRKKjw@mail.gmail.com>
On Tue, Mar 26, 2019 at 11:59:56AM +0000, Dean Rasheed wrote:
>On Mon, 25 Mar 2019 at 23:36, Tomas Vondra <tomas.vondra@2ndquadrant.com> wrote:
>>
>> Attached is an updated patch, fixing all the issues pointed out so far.
>> Unless there are some objections, I plan to commit the 0001 part by the
>> end of this CF. Part 0002 is a matter for PG13, as previously agreed.
>>
>
>Yes, I think that's reasonable. It looks to be in pretty good shape. I
>have reviewed most of the actual code, but note that I haven't
>reviewed the docs changes and I didn't spend much time reading code
>comments. It might benefit from a quick once-over comment tidy up.
>
>I just looked through the latest set of changes and I have a couple of
>additional review comments:
>
>In the comment about WIDTH_THRESHOLD, s/pg_statistic/pg_statistic_ext/.
>
>In statext_mcv_build(), I'm not convinced by the logic around keeping
>the whole MCV list if it fits. Suppose there were a small number of
>very common values, and then a bunch of uniformly distributed less
>common values. The sample might consist of all the common values, plus
>one or two instances of some of the uncommon ones, leading to a list
>that would fit, but it would not be appropriate to keep the uncommon
>values on the basis of having seen them only one or two times. The
>fact that the list of items seen fits doesn't by itself mean that
>they're all common enough to justify being kept. In the per-column
>stats case, there are a bunch of other checks that have to pass, which
>are intended to test not just that the list fits, but that it believes
>that those are all the items in the table. For MV stats, you don't
>have that, and so I think it would be best to just remove that test
>(the "if (ngroups > nitems)" test) and *always* call
>get_mincount_for_mcv_list() to determine how many MCV items to keep.
>Otherwise there is a risk of keeping too many MCV items, with the ones
>at the tail end of the list producing poor estimates.
>
I need to think about it a bit, but I think you're right we may not need
to keep those items. The reason why I decided to keep them is that they
may represent cases where the (actual frequency << base frequency). But
now that I think about it, that can probably be handled as
((1 - total_sel) / ndistinct)
So yeah, I think I'll just change the code to always us the mincount.
regards
--
Tomas Vondra http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
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, david.rowley@2ndquadrant.com, horiguchi.kyotaro@lab.ntt.co.jp, 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: <20190326151235.GA3088@development>
* 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