Ensure to pushdown the aggregation for count(const)

[Lewuathe]

Purpose

Aggregate pushdown should be enabled even with count(const) query. Since SimplifyCountOverConstant is applied after PushAggregationIntoTableScan, the aggregation pushdown does not treat count(*) and count(const) uniformally.

The logical plan of the following query with PostgreSQL connector is fixed.

create table test_table (c1 integer, c2 varchar);

Before

> explain (type logical) select c2, count(1) from test_table group by 1;
                                                            Query Plan
-----------------------------------------------------------------------------------------------------------------------------------
 Output[c2, _col1]
 │   Layout: [c2:varchar, count:bigint]
 │   Estimates: {rows: ? (?), cpu: ?, memory: ?, network: ?}
 │   _col1 := count
 └─ RemoteExchange[GATHER]
    │   Layout: [c2:varchar, count:bigint]
    │   Estimates: {rows: ? (?), cpu: ?, memory: ?, network: ?}
    └─ Project[]
       │   Layout: [c2:varchar, count:bigint]
       │   Estimates: {rows: ? (?), cpu: ?, memory: ?, network: ?}
       └─ Aggregate(FINAL)[c2][$hashvalue]
          │   Layout: [c2:varchar, $hashvalue:bigint, count:bigint]
          │   Estimates: {rows: ? (?), cpu: ?, memory: ?, network: ?}
          │   count := count("count_0")
          └─ LocalExchange[HASH][$hashvalue] ("c2")
             │   Layout: [c2:varchar, count_0:bigint, $hashvalue:bigint]
             │   Estimates: {rows: ? (?), cpu: ?, memory: ?, network: ?}
             └─ RemoteExchange[REPARTITION][$hashvalue_1]
                │   Layout: [c2:varchar, count_0:bigint, $hashvalue_1:bigint]
                │   Estimates: {rows: ? (?), cpu: ?, memory: ?, network: ?}
                └─ Aggregate(PARTIAL)[c2][$hashvalue_2]
                   │   Layout: [c2:varchar, $hashvalue_2:bigint, count_0:bigint]
                   │   count_0 := count(*)
                   └─ ScanProject[table = postgresql:public.test_table public.test_table columns=[c2:varchar:varchar]]
                          Layout: [c2:varchar, $hashvalue_2:bigint]
                          Estimates: {rows: ? (?), cpu: ?, memory: 0B, network: 0B}/{rows: ? (?), cpu: ?, memory: 0B, network: 0B}
                          $hashvalue_2 := combine_hash(bigint '0', COALESCE("$operator$hash_code"("c2"), 0))
                          c2 := c2:varchar:varchar

After

The fix eliminates the aggregation node from the Presto side properly.

> explain (type logical) select c2, count(1) from test_table group by 1;
                                                                                  Query Plan
------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
 Output[c2, _col1]
 │   Layout: [c2_0:varchar, _presto_generated_1:bigint]
 │   Estimates: {rows: ? (?), cpu: ?, memory: 0B, network: ?}
 │   c2 := c2_0
 │   _col1 := _presto_generated_1
 └─ RemoteExchange[GATHER]
    │   Layout: [c2_0:varchar, _presto_generated_1:bigint]
    │   Estimates: {rows: ? (?), cpu: ?, memory: 0B, network: ?}
    └─ TableScan[postgresql:public.test_table public.test_table columns=[c2:varchar:varchar, count(*):_presto_generated_1:bigint:bigint] groupingSets=[[c2:varchar:varchar]]]
           Layout: [c2_0:varchar, _presto_generated_1:bigint]
           Estimates: {rows: ? (?), cpu: ?, memory: 0B, network: 0B}
           _presto_generated_1 := count(*):_presto_generated_1:bigint:bigint
           c2_0 := c2:varchar:varchar

It fixes #4362

[sajjoseph]

Thanks for fixing this.

[kasiafi]

Shall we remove the other (late) occurrence of the SimplifyCountOverConstant rule? I can't think of any case it could fire at that point.

[kokosing]

Shall we remove the other (late) occurrence of the SimplifyCountOverConstant rule? I can't think of any case it could fire at that point.

👍

[alexjo2144]

My approval may not mean much :) but LGTM

[findepi]

Merged, thanks!