Re: [HACKERS] WIP: Aggregation push-down

On Tue, Jul 9, 2019 at 9:47 PM Antonin Houska <ah(at)cybertec(dot)at> wrote:

> Richard Guo <riguo(at)pivotal(dot)io> wrote:
>
> > Another rebase is needed for the patches.
>
> Done.
>

I didn't fully follow the whole thread and mainly looked into the latest
patch set. So what are the considerations for abandoning the aggmultifn
concept? In my opinion, aggmultifn would enable us to do a lot more
types of transformation. For example, consider the query below:

select sum(foo.c) from foo join bar on foo.b = bar.b group by foo.a, bar.a;

With the latest patch, the plan looks like:

Finalize HashAggregate <------ sum(psum)
Group Key: foo.a, bar.a
-> Hash Join
Hash Cond: (bar.b = foo.b)
-> Seq Scan on bar
-> Hash
-> Partial HashAggregate <------ sum(foo.c) as psum
Group Key: foo.a, foo.b
-> Seq Scan on foo

If we have aggmultifn, we can perform the query this way:

Finalize HashAggregate <------ sum(foo.c)*cnt
Group Key: foo.a, bar.a
-> Hash Join
Hash Cond: (foo.b = bar.b)
-> Seq Scan on foo
-> Hash
-> Partial HashAggregate <------ count(*) as cnt
Group Key: bar.a, bar.b
-> Seq Scan on bar

And this way:

Finalize HashAggregate <------ sum(psum)*cnt
Group Key: foo.a, bar.a
-> Hash Join
Hash Cond: (foo.b = bar.b)
-> Partial HashAggregate <------ sum(foo.c) as psum
Group Key: foo.a, foo.b
-> Seq Scan on foo
-> Hash
-> Partial HashAggregate <------ count(*) as cnt
Group Key: bar.a, bar.b
-> Seq Scan on bar

My another question is in function add_grouped_path(), when creating
sorted aggregation path on top of subpath. If the subpath is not sorted,
then the sorted aggregation path would not be generated. Why not in this
case we create a sort path on top of subpath first and then create group
aggregation path on top of the sort path?

Core dump when running one query in agg_pushdown.sql

EXPLAIN ANALYZE
SELECT p.x, avg(c1.v) FROM agg_pushdown_parent AS p JOIN agg_pushdown_child1
AS c1 ON c1.parent = p.i GROUP BY p.i;

#0 0x00000000006def98 in CheckVarSlotCompatibility (slot=0x0, attnum=1,
vartype=23) at execExprInterp.c:1850
#1 0x00000000006def5d in CheckExprStillValid (state=0x2b63a28,
econtext=0x2ba4958) at execExprInterp.c:1814
#2 0x00000000006dee38 in ExecInterpExprStillValid (state=0x2b63a28,
econtext=0x2ba4958, isNull=0x7fff7cd16a37) at execExprInterp.c:1763
#3 0x00000000007144dd in ExecEvalExpr (state=0x2b63a28,
econtext=0x2ba4958, isNull=0x7fff7cd16a37)
at ../../../src/include/executor/executor.h:288
#4 0x0000000000715475 in ExecIndexEvalRuntimeKeys (econtext=0x2ba4958,
runtimeKeys=0x2b63910, numRuntimeKeys=1) at nodeIndexscan.c:630
#5 0x000000000071533b in ExecReScanIndexScan (node=0x2b62bf8) at
nodeIndexscan.c:568
#6 0x00000000006d4ce6 in ExecReScan (node=0x2b62bf8) at execAmi.c:182
#7 0x00000000007152a0 in ExecIndexScan (pstate=0x2b62bf8) at
nodeIndexscan.c:530

This is really a cool feature. Thank you for working on this.

Thanks
Richard