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

help / color / mirror / Atom feed
From: Antonin Houska <ah@cybertec.at>
To: Andy Fan <zhihui.fan1213@gmail.com>
Cc: legrand legrand <legrand_legrand@hotmail.com>
Cc: pgsql-hackers <pgsql-hackers@postgresql.org>
Subject: Re: WIP: Aggregation push-down
Date: Fri, 24 Apr 2020 15:01:09 +0200
Message-ID: <51215.1587733269@antos> (raw)
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>

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 appropriate
>  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" relations
>  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 place rather 
> than root->upper_rels, we can store a generic type in RelInfoList, looks currently
> it is bounded to RelAggInfo besides RelOptInfo.   But overall, personally 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





view thread (58+ messages)  latest in thread

Message-ID: <51215.1587733269@antos>
Permalink:  ../51215.1587733269@antos/
Also on:    postgresql.org/message-id/51215.1587733269@antos

 · 

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: ah@cybertec.at, zhihui.fan1213@gmail.com, legrand_legrand@hotmail.com
  Subject: Re: WIP: Aggregation push-down
  In-Reply-To: <51215.1587733269@antos>

* 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