pg.ddx.io  pgsql-hackers@postgresql.org mailing list archive  

help / color / mirror / Atom feed
From: Tomas Vondra <tomas.vondra@enterprisedb.com>
To: Dean Rasheed <dean.a.rasheed@gmail.com>
Cc: Justin Pryzby <pryzby@telsasoft.com>
Cc: PostgreSQL Hackers <pgsql-hackers@postgresql.org>
Subject: Re: PoC/WIP: Extended statistics on expressions
Date: Wed, 24 Mar 2021 17:48:17 +0100
Message-ID: <a7d17aec-234d-c7f2-bfdc-99c87767a507@enterprisedb.com> (raw)
In-Reply-To: <CAEZATCW2WHfsrbDLzwMvD+kOsBemLhvKwoHGhzR7STtac74XTg@mail.gmail.com>
References: <f0f7e00c-7ccb-9412-0147-a06a60777f06@enterprisedb.com>
	<CAEZATCWF66BWJq-OwLuP5LGK6W9LDYxCQwLxqB36qmq3b1Ch8Q@mail.gmail.com>
	<f4dac079-6bc4-ccc0-0fd8-3e2e2da28d92@enterprisedb.com>
	<CAEZATCXz2WO9ZctRL68_HrsPbpre0GRmip83TgznoeLoWm4=Dg@mail.gmail.com>
	<CAEZATCXcX0gm2LMb+xObroS-C+a5FXEqgR5RgQ9NWKjPC=9sNA@mail.gmail.com>
	<ce50ee26-7135-c435-cae8-073ad59eba60@enterprisedb.com>
	<CAEZATCWgZPJGbj2ndXsoD6_MuJ=H1Y-gdTMdkE=n1VsA6WX+RA@mail.gmail.com>
	<9f6654b1-ca31-f979-0fe8-137683818273@enterprisedb.com>
	<20210324081402.GB28335@telsasoft.com>
	<b8359875-b91f-ff48-4c6c-9b3c1c35d880@enterprisedb.com>
	<20210324091708.GC28335@telsasoft.com>
	<1ff3fc47-8f6e-7839-c3ff-6b694956f92e@enterprisedb.com>
	<CAEZATCWYCY=2yEyxa6BwimCPQ_RZHy5GOb6U6i6dGHGQdjQESQ@mail.gmail.com>
	<e4556e47-940d-58a6-73fd-16c5815dd2cd@enterprisedb.com>
	<CAEZATCW2WHfsrbDLzwMvD+kOsBemLhvKwoHGhzR7STtac74XTg@mail.gmail.com>



On 3/24/21 5:28 PM, Dean Rasheed wrote:
> On Wed, 24 Mar 2021 at 14:48, Tomas Vondra
> <tomas.vondra@enterprisedb.com> wrote:
>>
>> AFAIK the primary issue here is that the two places disagree. While
>> estimate_num_groups does this
>>
>>     varnos = pull_varnos(root, (Node *) varshere);
>>     if (bms_membership(varnos) == BMS_SINGLETON)
>>     { ... }
>>
>> the add_unique_group_expr does this
>>
>>     varnos = pull_varnos(root, (Node *) groupexpr);
>>
>> That is, one looks at the group expression, while the other look at vars
>> extracted from it by pull_var_clause(). Apparently for PlaceHolderVar
>> this can differ, causing the crash.
>>
>> So we need to change one of those places - my fix tweaked the second
>> place to also look at the vars, but maybe we should change the other
>> place? Or maybe it's not the right fix for PlaceHolderVars ...
>>
> 
> I think that it doesn't make any difference which place is changed.
> 
> This is a case of an expression with no stats. With your change,
> you'll get a single GroupExprInfo containing a list of
> VariableStatData's for each of it's Var's, whereas if you changed it
> the other way, you'd get a separate GroupExprInfo for each Var. But I
> think they'd both end up being treated the same by
> estimate_multivariate_ndistinct(), since there wouldn't be any stats
> matching the expression, only the individual Var's. Maybe changing the
> first place would be the more bulletproof fix though.
> 

Yeah, I think that's true. I'll do a bit more research / experiments.

As for the changes proposed in the create_statistics, do we really want
to use univariate / multivariate there? Yes, the terms are correct, but
I'm not sure how many people looking at CREATE STATISTICS will
understand them.


regards

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





view thread (107+ messages)  latest in thread

Message-ID: <a7d17aec-234d-c7f2-bfdc-99c87767a507@enterprisedb.com>
Permalink:  ../a7d17aec-234d-c7f2-bfdc-99c87767a507@enterprisedb.com/
Also on:    postgresql.org/message-id/a7d17aec-234d-c7f2-bfdc-99c87767a507@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, dean.a.rasheed@gmail.com, pryzby@telsasoft.com
  Subject: Re: PoC/WIP: Extended statistics on expressions
  In-Reply-To: <a7d17aec-234d-c7f2-bfdc-99c87767a507@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