Re: Partial aggregates pushdown

From: Alexander Pyhalov <a(dot)pyhalov(at)postgrespro(dot)ru>
To: Zhihong Yu <zyu(at)yugabyte(dot)com>
Cc: PostgreSQL Developers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: Partial aggregates pushdown
Date: 2021-10-22 06:26:50
Message-ID: 9a6d8434e2a17d3257e89d608eac795a@postgrespro.ru
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Zhihong Yu писал 2021-10-22 00:43:
> Hi,
> w.r.t. 0001-Partial-aggregates-push-down-v03.patch
>

Hi.

> For partial_agg_ok(),
>
> + if (agg->aggdistinct || agg->aggvariadic || agg->aggkind !=
> AGGKIND_NORMAL || agg->aggorder != NIL)
> + ok = false;
>
> Since SearchSysCache1() is not called yet, you can return false
> directly.

Fixed.

>
> + if (aggform->aggpartialpushdownsafe != true)
>
> The above can be written as:
>
> if (!aggform->aggpartialpushdownsafe)

Fixed.

>
> For build_conv_list():
>
> + Oid converter_oid = InvalidOid;
> +
> + if (IsA(tlentry->expr, Aggref))
>
> ...
> + }
> + convlist = lappend_oid(convlist, converter_oid);
>
> Do you intend to append InvalidOid to convlist (when tlentry->expr is
> not Aggref) ?

Yes, for each tlist member (which matches fpinfo->grouped_tlist in case
when foreignrel is UPPER_REL) we need to find corresponding converter.
If we don't append InvalidOid, we can't find convlist member,
corresponding to tlist member. Added comments to build_conv_list.

Also fixed error in pg_dump.c (we selected '0' when
aggpartialconverterfn was not defined in schema, but checked for '-').

--
Best regards,
Alexander Pyhalov,
Postgres Professional

Attachment Content-Type Size
0001-Partial-aggregates-push-down-v04.patch text/x-diff 58.1 KB

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Greg Nancarrow 2021-10-22 06:29:38 Re: Added schema level support for publication.
Previous Message Andres Freund 2021-10-22 05:59:39 Re: Experimenting with hash tables inside pg_dump