Fix v1 query engine behavior for aggregations without group by where the limit is zero

[yashmayya]

• Aggregation queries without GROUP BY clauses typically aren't used with a LIMIT clause since it semantically doesn't make sense.
• However, it isn't considered invalid, and most databases (including Postgres and MySQL) return the same aggregation results (i.e., over the entire table) regardless of what the LIMIT is - except if the LIMIT is 0, in which case an empty result set is returned.
• The v1 query engine in Pinot follows the former but not the latter - i.e., it returns the same aggregation results (i.e., over the entire table) regardless of what the LIMIT is (even if it is 0).
• This patch standardizes the Pinot behavior in this case by introducing an EmptyAggregationOperator (somewhat similar to the EmptySelectionOperator):

> EXPLAIN PLAN FOR SELECT SUM(ArrDelay) FROM airlineStats LIMIT 0;

PLAN_START(numSegmentsForThisPlan:31)
BROKER_REDUCE(limit:0)
COMBINE_AGGREGATE
AGGREGATE_EMPTY

> SELECT SUM(ArrDelay) FROM airlineStats LIMIT 0;

{"resultTable":{"dataSchema":{"columnNames":["sum(ArrDelay)"],"columnDataTypes":["DOUBLE"]},"rows":[]},"numRowsResultSet":0,"partialResult":false,"exceptions":[],"numGroupsLimitReached":false,"timeUsedMs":3,"requestId":"11345490000000018","brokerId":"Broker_192.168.29.25_8000","numDocsScanned":0,"totalDocs":9746,"numEntriesScannedInFilter":0,"numEntriesScannedPostFilter":0,"numServersQueried":1,"numServersResponded":1,"numSegmentsQueried":31,"numSegmentsProcessed":31,"numSegmentsMatched":0,"numConsumingSegmentsQueried":0,"numConsumingSegmentsProcessed":0,"numConsumingSegmentsMatched":0,"minConsumingFreshnessTimeMs":0,"numSegmentsPrunedByBroker":0,"numSegmentsPrunedByServer":0,"numSegmentsPrunedInvalid":0,"numSegmentsPrunedByLimit":0,"numSegmentsPrunedByValue":0,"brokerReduceTimeMs":0,"offlineThreadCpuTimeNs":0,"realtimeThreadCpuTimeNs":0,"offlineSystemActivitiesCpuTimeNs":0,"realtimeSystemActivitiesCpuTimeNs":0,"offlineResponseSerializationCpuTimeNs":0,"realtimeResponseSerializationCpuTimeNs":0,"offlineTotalCpuTimeNs":0,"realtimeTotalCpuTimeNs":0,"explainPlanNumEmptyFilterSegments":0,"explainPlanNumMatchAllFilterSegments":0,"traceInfo":{}}

• This patch fixes one of the cases described in Short circuit queries where no rows will be returned #13563.
• Note that this isn't an issue in the v2 multi-stage query engine because the Calcite planner is smart enough to prune sections of query plans that are known to never produce any rows.

> SET useMultiStageEngine = true; EXPLAIN PLAN FOR SELECT SUM(ArrDelay) FROM airlineStats LIMIT 0;

Execution Plan
LogicalValues(tuples=[[]])

• The existing test testGroupByAggregationWithLimitZero recently added in Fix issue with group by combine operator when limit is zero #13555 is updated because the integration test actually doesn't do any comparison for aggregation group by queries without an order by clause (also in this case we just want to verify that 0 rows are returned and that the data schema is still returned) -

  pinot/pinot-integration-test-base/src/test/java/org/apache/pinot/integration/tests/ClusterIntegrationTestUtils.java

  Lines 812 to 817 in efa4300

  	} else {
  	// aggregation group-by
  	// TODO: compare results for aggregation group by queries w/o order by
  	// Compare results for aggregation group by queries with order by
  	if (queryContext.getOrderByExpressions() != null) {

[codecov-commenter]

Codecov Report

Attention: Patch coverage is 0% with 15 lines in your changes missing coverage. Please review.

Project coverage is 62.09%. Comparing base (59551e4) to head (6184918).
Report is 731 commits behind head on master.

Files	Patch %	Lines
.../core/operator/query/EmptyAggregationOperator.java	0.00%	8 Missing ⚠️
...erator/blocks/results/AggregationResultsBlock.java	0.00%	1 Missing and 2 partials ⚠️
.../combine/merger/AggregationResultsBlockMerger.java	0.00%	1 Missing and 1 partial ⚠️
...rg/apache/pinot/core/plan/AggregationPlanNode.java	0.00%	1 Missing and 1 partial ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##             master   #13564      +/-   ##
============================================
+ Coverage     61.75%   62.09%   +0.34%     
+ Complexity      207      198       -9     
============================================
  Files          2436     2559     +123     
  Lines        133233   140935    +7702     
  Branches      20636    21870    +1234     
============================================
+ Hits          82274    87518    +5244     
- Misses        44911    46794    +1883     
- Partials       6048     6623     +575     

Flag	Coverage Δ
custom-integration1	<0.01% <0.00%> (-0.01%)	⬇️
integration	<0.01% <0.00%> (-0.01%)	⬇️
integration1	<0.01% <0.00%> (-0.01%)	⬇️
integration2	0.00% <0.00%> (ø)
java-11	62.05% <0.00%> (+0.34%)	⬆️
java-21	61.98% <0.00%> (+0.35%)	⬆️
skip-bytebuffers-false	62.07% <0.00%> (+0.33%)	⬆️
skip-bytebuffers-true	61.94% <0.00%> (+34.21%)	⬆️
temurin	62.09% <0.00%> (+0.34%)	⬆️
unittests	62.09% <0.00%> (+0.34%)	⬆️
unittests1	46.67% <0.00%> (-0.22%)	⬇️
unittests2	27.65% <0.00%> (-0.08%)	⬇️

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.

[Jackie-Jiang]

For aggregation function without group-by, we should be able to short circuit it on the broker side and always get the correct data schema.
For selection/group-by, ideally we also want to short circuit on the broker side since schema is already enforced. One missing piece is to derive the transform result type based on the input type.
You may also take a look at #13057 and we can probably solve them together.

[siddharthteotia]

@jackjlli - Can you help review this please ?

[jackjlli]

+1 on regardless of whether it's an aggregate query, selection query, order-by query or not, if the limit is set to 0, we can always short circuit it on broker side to fail fast.

[gortiz]

Although #13057 and #13831 may superseded this PR, this PR should fix one of the issues. As a note for the other two PRs, please study whether changes in this PR are still needed once the other two are ready to merge.

[gortiz]

Although #13057 and #13831 may superseded this PR, this PR should fix one of the issues. As a note for the other two PRs, please study whether changes in this PR are still needed once the other two are ready to merge.