Re: Partial aggregates pushdown

From: Ilya Gladyshev <i(dot)gladyshev(at)postgrespro(dot)ru>
To: Alexander Pyhalov <a(dot)pyhalov(at)postgrespro(dot)ru>
Cc: pgsql-hackers(at)postgresql(dot)org
Subject: Re: Partial aggregates pushdown
Date: 2021-11-01 21:31:35
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers


On 21.10.2021 13:55, Alexander Pyhalov wrote:
> Hi. Updated patch.
> Now aggregates with internal states can be pushed down, if they are
> marked as pushdown safe (this flag is set to true for min/max/sum),
> have internal states and associated converters.

I don't quite understand why this is restricted only to aggregates that
have 'internal' state, I feel like that should be possible for any
aggregate that has a function to convert its final result back to
aggregate state to be pushed down. While I couldn't come up with a
useful example for this, except maybe for an aggregate whose aggfinalfn
is used purely for cosmetic purposes (e.g. format the result into a
string), I still feel that it is an unnecessary restriction.

A few minor review notes to the patch:

+static List *build_conv_list(RelOptInfo *foreignrel);

this should probably be up top among other declarations.

@@ -1433,6 +1453,48 @@ postgresGetForeignPlan(PlannerInfo *root,

+ * Generate attinmeta if there are some converters:
+ * they are expecxted to return BYTEA, but real input type is likely
+ */

typo in word "expec*x*ted".

@@ -139,10 +147,13 @@ typedef struct PgFdwScanState
                                  * for a foreign join scan. */
     TupleDesc    tupdesc;        /* tuple descriptor of scan */
     AttInMetadata *attinmeta;    /* attribute datatype conversion
metadata */
+    AttInMetadata *rcvd_attinmeta;    /* metadata for received tuples,
+                                     * there's no converters */

Looks like rcvd_attinmeta is redundant and you could use attinmeta for
conversion metadata.

The last thing - the patch needs to be rebased, it doesn't apply cleanly
on top of current master.


Ilya Gladyshev

In response to


Browse pgsql-hackers by date

  From Date Subject
Next Message Mark Dilger 2021-11-01 21:42:42 Re: Delegating superuser tasks to new security roles (Was: Granting control of SUSET gucs to non-superusers)
Previous Message Tomas Vondra 2021-11-01 21:15:59 Re: Feature request for adoptive indexes