pg.ddx.io  pgsql-hackers@postgresql.org mailing list archive  
help / color / mirror / Atom feed
From: Tomas Vondra <tomas.vondra@2ndquadrant.com>
To: Dean Rasheed <dean.a.rasheed@gmail.com>
Cc: Pierre Ducroquet <p.psql@pinaraf.info>
Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Subject: Re: PATCH: add support for IN and @> in functional-dependency statistics use
Date: Wed, 18 Mar 2020 01:29:46 +0100
Message-ID: <20200318002946.6dvblukm3cfmgir2@development> (raw)
In-Reply-To: <20200317170517.hutarq3sioyny3wa@development>
References: <CAEZATCXnQtw-ZytE+rp1Uby9KMs0hWim-unMa-nMdmS5MMgtbQ@mail.gmail.com>
	<20200312173047.stp55sxoj4sipn7q@development>
	<CAEZATCVb76OcPrA_zkBLk9J3CWPN8bRNZTCtCehbm+RFj=W-Aw@mail.gmail.com>
	<20200313160944.aqrbkyf7tkasoxfk@development>
	<20200314152159.5webesf6pfluatqj@development>
	<20200314184535.4jgkxcyg5yprvsra@development>
	<CAEZATCVV2NM0S=hZ1_qH-9B_psrC+5Tqx2xjmL9kHc9EsACSyA@mail.gmail.com>
	<20200317153706.z6pdtijr25iyr424@development>
	<CAEZATCWCszNJv4QvXYGo6JpgtM09mGpjraA=1_Bk+7MPENeARw@mail.gmail.com>
	<20200317170517.hutarq3sioyny3wa@development>

On Tue, Mar 17, 2020 at 06:05:17PM +0100, Tomas Vondra wrote:
>On Tue, Mar 17, 2020 at 04:14:26PM +0000, Dean Rasheed wrote:
>>On Tue, 17 Mar 2020 at 15:37, Tomas Vondra <tomas.vondra@2ndquadrant.com> wrote:
>>>
>>>On Tue, Mar 17, 2020 at 12:42:52PM +0000, Dean Rasheed wrote:
>>>
>>>>The other thing that I'm still concerned about is the possibility of
>>>>returning estimates with P(a,b) > P(a) or P(b). I think that such a
>>>>thing becomes much more likely with the new types of clause supported
>>>>here, because they now allow multiple values from each column, where
>>>>before we only allowed one. I took another look at the patch I posted
>>>>on the other thread, and I've convinced myself that it's correct.
>>>>Attached is an updated version, with some cosmetic tidying up and now
>>>>with some additional regression tests.
>>>
>>>Yeah, I agree that's something we need to fix. Do you plan to push the
>>>fix, or do you want me to do it?
>>>
>>
>>I can push it. Have you had a chance to review it?
>>
>
>Not yet, I'll take a look today.
>

OK, I took a look. I think from the correctness POV the patch is OK, but
I think the dependencies_clauselist_selectivity() function now got a bit
too complex. I've been able to parse it now, but I'm sure I'll have
trouble in the future :-(

Can we refactor / split it somehow and move bits of the logic to smaller
functions, or something like that?

Another thing I'd like to suggest is keeping the "old" formula, and
instead of just replacing it with

    P(a,b) = f * Min(P(a), P(b)) + (1-f) * P(a) * P(b)

but explaining how the old formula may produce nonsensical selectivity,
and how the new formula addresses that issue.

regards

-- 
Tomas Vondra                  http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services





view thread (24+ messages)  latest in thread

Message-ID: <20200318002946.6dvblukm3cfmgir2@development>
Permalink:  ../20200318002946.6dvblukm3cfmgir2@development/
Also on:    postgresql.org/message-id/20200318002946.6dvblukm3cfmgir2@development

 · 

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@2ndquadrant.com, dean.a.rasheed@gmail.com, p.psql@pinaraf.info, pgsql-hackers@lists.postgresql.org
  Subject: Re: PATCH: add support for IN and @> in functional-dependency statistics use
  In-Reply-To: <20200318002946.6dvblukm3cfmgir2@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