Re: [PATCH] Keeps tracking the uniqueness with UniqueKey

From: Andy Fan <zhihui(dot)fan1213(at)gmail(dot)com>
To: David Rowley <dgrowleyml(at)gmail(dot)com>
Cc: Ashutosh Bapat <ashutosh(dot)bapat(dot)oss(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, rushabh(dot)lathia(at)gmail(dot)com, Ashutosh Bapat <ashutosh(dot)bapat(at)2ndquadrant(dot)com>
Subject: Re: [PATCH] Keeps tracking the uniqueness with UniqueKey
Date: 2020-05-21 23:49:07
Message-ID: CAKU4AWoVHBFaJYTH7yJvE0G4uJ6yzQjDA_4nSA3NxYCmk3OKOQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Fri, May 22, 2020 at 4:52 AM David Rowley <dgrowleyml(at)gmail(dot)com> wrote:

> On Thu, 14 May 2020 at 14:39, Andy Fan <zhihui(dot)fan1213(at)gmail(dot)com> wrote:
> >
> > On Thu, May 14, 2020 at 6:20 AM David Rowley <dgrowleyml(at)gmail(dot)com>
> wrote:
> >> Having the "onerow" flag was not how I intended it to work.
> >>
> > Thanks for the detailed explanation. So I think we do need to handle
> onerow
> > specially, (It means more things than adding each column as a UniqueKey).
> > but we don't need the onerow flag since we can tell it by ukey->exprs ==
> NIL.
> >
> > During the developer of this feature, I added some Asserts as double
> checking
> > for onerow and exprs. it helps me to find some special cases. like
> > SELECT FROM multirows union SELECT FROM multirows; where targetlist is
> NIL.
> > (I find the above case returns onerow as well just now). so onerow flag
> allows us
> > check this special things with more attention. Even this is not the
> original intention
> > but looks it is the one purpose now.
>
> But surely that special case should just go in
> populate_unionrel_uniquekeys(). If the targetlist is empty, then add a
> UniqueKey with an empty set of exprs.
>
> This is correct on this special case.

> However I am feeling that removing onerow flag doesn't require much of
> code
> > changes. Almost all the special cases which are needed before are still
> needed
> > after that and all the functions based on that like relation_is_onerow
> > /add_uniquekey_onerow is still valid, we just need change the
> implementation.
> > so do you think we need to remove onerow flag totally?
>
> Well, at the moment I'm not quite understanding why it's needed. If
> it's not needed then we should remove it. If it turns out there is
> some valid reason for it, then we should keep it.
>

Currently I uses it to detect more special case which we can't image at
first, we can
understand it as it used to debug/Assert purpose only. After the mainly
code is
reviewed, that can be removed (based on the change is tiny).

--
Best Regards
Andy Fan

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message David Rowley 2020-05-21 23:53:51 Re: Hybrid Hash/Nested Loop joins and caching results from subplans
Previous Message Andy Fan 2020-05-21 23:41:11 Re: [PATCH] Keeps tracking the uniqueness with UniqueKey