From: Antonin Houska <ah@cybertec.at>
To: Andy Fan <zhihui.fan1213@gmail.com>
cc: legrand legrand <legrand_legrand@hotmail.com>,
    pgsql-hackers <pgsql-hackers@postgresql.org>
Subject: Re: WIP: Aggregation push-down
In-reply-to: 
 <CAKU4AWr7m8WWBJS_s3fzc460VQOibXGwtzeRPX8oq6ez9dSVng@mail.gmail.com>
References: <11172.1550780283@sss.pgh.pa.us> <23468.1551369774@localhost>
 <CAN_9JTy95UMjOVDSc-4MBrA6Fc1hX2vh=t2RMHp=yGyAV=-Xsw@mail.gmail.com>
 <1897.1562680064@spoje.net>
 <CAN_9JTzC1K-Gc3p5GD=dQeJD8k_Z5996B0rEYjVGx+pXPq8JPw@mail.gmail.com>
 <13971.1562920911@spoje.net>
 <CAN_9JTzAsBwv5zr4SRVO35OtMWQBXSLcUuY65NQ0ZtFCTOrMMA@mail.gmail.com>
 <19857.1563374392@localhost> <20190904202339.GA13644@alvherre.pgsql>
 <7087.1567663858@localhost> <1582751446111-0.post@n3.nabble.com>
 <20715.1582793501@antos>
 <CAKU4AWqK_X4L5WDsjnsFpm_Gt8Mx0+H4frC-6Pom1ro=aq44mQ@mail.gmail.com>
 <CAKU4AWr7m8WWBJS_s3fzc460VQOibXGwtzeRPX8oq6ez9dSVng@mail.gmail.com>
Comments: In-reply-to Andy Fan <zhihui.fan1213@gmail.com>
   message dated "Wed, 22 Apr 2020 11:39:58 +0800."
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <51214.1587733269.1@antos>
Content-Transfer-Encoding: quoted-printable
Date: Fri, 24 Apr 2020 15:01:09 +0200
Message-ID: <51215.1587733269@antos>
Precedence: bulk

Andy Fan <zhihui.fan1213@gmail.com> wrote:

>  > 1) v14-0001-Introduce-RelInfoList-structure.patch
>  > -------------------------------------------------
>  > =

>  > - I'm not entirely sure why we need this change. We had the list+hash
>  > before, so I assume we do this because we need the output functions?
> =

>  I believe that this is what Tom proposed in [1], see "Maybe an appropri=
ate
>  preliminary patch is to refactor the support code ..." near the end of =
that
>  message. The point is that now we need two lists: one for "plain" relat=
ions
>  and one for grouped ones.
> =

>  =

> I guess what Toms suggested[1] is to store the the grouped ones into =

> root->upper_rels rather than a separated member, see fetch_upper_rel
> or UpperRelationKind.  If we do need the list+hash method for long list =
lookup, =

> we can merge it into upper_rels.  If we want this benefits at other plac=
e rather =

> than root->upper_rels, we can store a generic type in RelInfoList, looks=
 currently
> it is bounded to RelAggInfo besides RelOptInfo.   But overall, personall=
y I think we can =

> ignore such optimization at the first stage to save the attention of the=
 core reviewers
> since they are too precious :)    Just FYI
> =

> [1] https://www.postgresql.org/message-id/9726.1542577439@sss.pgh.pa.us =


Hm, you seem to be right, not sure why I missed the point. I thought that =
the
reason Tom doesn't like storing the grouped relations in simple_rel_array =
is
that we only need the grouped base relations inside query_planner(), but
simple_rel_array is used higher in the stack. So I introduced a new field =
and
used it only in query_planner() and subroutines.

Yes, it's better to use root->upper_rels than to introduce the new field. =
I'll
adjust the patch. Thanks.

-- =

Antonin Houska
Web: https://www.cybertec-postgresql.com