Adds AGGREGATE_CASE_TO_FILTER rule

[gortiz]

This PR adds a rule that transforms queries like SELECT SUM(CASE WHEN col1 = 'a' THEN 1 ELSE 0 END) FROM a into SELECT SUM(1) FILTER (WHERE col1 = 'a') from a.

A test is added to verify explain is compatible, although in that case more optimizations are applied and we ended up executing something like SELECT COUNT(col1) from a where col1 = 'a'

We think this optimization should be effective in most of the cases, specially applying indexes, although it may be some situations where the result is worse than expected because FILTER is sometimes slower than CASE. We plan to add in a future an option or hint to indicate Pinot to do not apply this (and other) optimizations.

[gortiz]

Thanks to the test that was failing I've discovered the issue reported as #12647. I've changed the test to meet the actual semantics we are following right now, although we should fix that in the future

[codecov-commenter]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 61.46%. Comparing base (59551e4) to head (00c142a).
Report is 128 commits behind head on master.

Additional details and impacted files

@@             Coverage Diff              @@
##             master   #12643      +/-   ##
============================================
- Coverage     61.75%   61.46%   -0.29%     
+ Complexity      207      198       -9     
============================================
  Files          2436     2452      +16     
  Lines        133233   133840     +607     
  Branches      20636    20766     +130     
============================================
- Hits          82274    82265       -9     
- Misses        44911    45413     +502     
- Partials       6048     6162     +114     

Flag	Coverage Δ
custom-integration1	<0.01% <ø> (-0.01%)	⬇️
integration	<0.01% <ø> (-0.01%)	⬇️
integration1	<0.01% <ø> (-0.01%)	⬇️
integration2	0.00% <ø> (ø)
java-11	34.62% <ø> (-27.09%)	⬇️
java-21	61.35% <ø> (-0.28%)	⬇️
skip-bytebuffers-false	61.45% <ø> (-0.30%)	⬇️
skip-bytebuffers-true	61.31% <ø> (+33.58%)	⬆️
temurin	61.46% <ø> (-0.29%)	⬇️
unittests	61.46% <ø> (-0.29%)	⬇️
unittests1	46.37% <ø> (-0.52%)	⬇️
unittests2	27.74% <ø> (+0.01%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[cyrilou242]

nice thanks!
Does this work as expected for?

DISTINCTCOUNTHLL(CASE WHEN x='foo' THEN y ELSE '' END)

Ie

DISTINCTCOUNTHLL(CASE WHEN x='foo' THEN y END)

can be transformed in

DISTINCTCOUNTHLL(y) FILTER(WHERE x = 'foo')

But

DISTINCTCOUNTHLL(CASE WHEN x='foo' THEN y ELSE '' END)

Should not be transformed because the ELSE case corresponds to a potential +1 in the count that cannot be expressed with the filter.

It's implemented here:
https://github.com/apache/calcite/blob/f0dc2b0aea46b1fd3f37e0cc126edaf82ade2344/core/src/main/java/org/apache/calcite/rel/rules/AggregateCaseToFilterRule.java#L179

Just want to make sure
aggregateCall.isDistinct() returns true when the aggregateCal is DISTINCTCOUNTHLL or a similar distinct aggregation function.

I'll check myself.

[cyrilou242]

Could confirm that the use cases I mentioned above work thanks

  @Test
  public void testAggregateCaseToFilterDistinctCountAgg_NO_REWRITE() {
    String query = "EXPLAIN PLAN FOR SELECT DISTINCTCOUNTHLL(CASE WHEN col1 = 'a' THEN 'lol' ELSE '' END) FROM a";

    String explain = _queryEnvironment.explainQuery(query, RANDOM_REQUEST_ID_GEN.nextLong());
    assertEquals(explain,
        "Execution Plan\n"
            + "LogicalAggregate(group=[{}], agg#0=[DISTINCTCOUNTHLL($0)])\n"
            + "  PinotLogicalExchange(distribution=[hash])\n"
            + "    LogicalAggregate(group=[{}], agg#0=[DISTINCTCOUNTHLL($0)])\n"
            + "      LogicalProject($f0=[CASE(=($0, _UTF-8'a'), _UTF-8'lol':VARCHAR(3) CHARACTER SET \"UTF-8\", _UTF-8'':VARCHAR(3) CHARACTER SET \"UTF-8\")])\n"
            + "        LogicalTableScan(table=[[a]])\n");
  }

  @Test
  public void testAggregateCaseToFilterDistinctCountAgg_DO_REWRITE() {
    String query = "EXPLAIN PLAN FOR SELECT DISTINCTCOUNTHLL(CASE WHEN col1 = 'a' THEN 'lol' END) FROM a";

    String explain = _queryEnvironment.explainQuery(query, RANDOM_REQUEST_ID_GEN.nextLong());
    assertEquals(explain,
        "Execution Plan\n"
            + "LogicalAggregate(group=[{}], agg#0=[DISTINCTCOUNTHLL($0)])\n"
            + "  PinotLogicalExchange(distribution=[hash])\n"
            + "    LogicalAggregate(group=[{}], agg#0=[DISTINCTCOUNTHLL($0) FILTER $1])\n"
            + "      LogicalProject($f1=[_UTF-8'lol'], $f2=[=($0, _UTF-8'a')])\n"
            + "        LogicalTableScan(table=[[a]])\n");
  }

passes thanks

[gortiz]

@cyrilou242 Given you created these tests... can you add them in a PR?

Anyway remember that there is a bug related to FILTER expressions. See #12647