Fix issue with group by combine operator when limit is zero

[yashmayya]

• A query like SELECT Origin, SUM(ArrDelay) FROM mytable GROUP BY Origin LIMIT 0 was resulting in the error:

QueryExecutionError:
java.lang.IllegalArgumentException: Result size should be a non-zero positive integer
    at com.google.common.base.Preconditions.checkArgument(Preconditions.java:145)
    at org.apache.pinot.core.data.table.IndexedTable.<init>(IndexedTable.java:74)
    at org.apache.pinot.core.data.table.ConcurrentIndexedTable.<init>(ConcurrentIndexedTable.java:42)
    at org.apache.pinot.core.data.table.ConcurrentIndexedTable.<init>(ConcurrentIndexedTable.java:37)

• This check was added recently in Adapt group trim threshold to group trim size rather than vice versa #13514 to ensure that an integer overflow check doesn't become error prone due to negative values (also the negative values don't make sense for result size / trim size / trim threshold).
• However, one thing that was missed is that the result size / trim size can be 0 for aggregation queries using group by when the limit is zero -

  pinot/pinot-core/src/main/java/org/apache/pinot/core/operator/streaming/StreamingGroupByCombineOperator.java

  Line 94 in cf1a0f6

  _trimSize = limit;

• This was surfaced by the testGeneratedQueries becoming a flaky test in recent CI runs (see https://github.com/apache/pinot/actions/runs/9834355954/job/27146034854, https://github.com/apache/pinot/actions/runs/9811276363/job/27093161297 for instance). The test can randomly generate group by aggregation queries with zero limits -

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

  Line 862 in cf1a0f6

  limit = new LimitQueryFragment(RANDOM.nextInt(MAX_RESULT_LIMIT + 1));

[codecov-commenter]

Codecov Report

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

Project coverage is 62.14%. Comparing base (59551e4) to head (aa4d2cd).
Report is 715 commits behind head on master.

Files	Patch %	Lines
...org/apache/pinot/core/data/table/IndexedTable.java	0.00%	0 Missing and 3 partials ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##             master   #13555      +/-   ##
============================================
+ Coverage     61.75%   62.14%   +0.39%     
+ Complexity      207      198       -9     
============================================
  Files          2436     2557     +121     
  Lines        133233   140887    +7654     
  Branches      20636    21855    +1219     
============================================
+ Hits          82274    87556    +5282     
- Misses        44911    46707    +1796     
- Partials       6048     6624     +576     

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.06% <0.00%> (+0.35%)	⬆️
java-21	62.01% <0.00%> (+0.38%)	⬆️
skip-bytebuffers-false	62.08% <0.00%> (+0.33%)	⬆️
skip-bytebuffers-true	61.98% <0.00%> (+34.25%)	⬆️
temurin	62.14% <0.00%> (+0.39%)	⬆️
unittests	62.14% <0.00%> (+0.39%)	⬆️
unittests1	46.68% <0.00%> (-0.21%)	⬇️
unittests2	27.69% <0.00%> (-0.04%)	⬇️

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]

Thanks for fixing this!

Ideally we should short circuit the execution when LIMIT is set to 0, and only return the data schema.

Currently we have 3 places to handle short circuit:

• In BaseSingleStageBrokerRequestHandler
• In ServerQueryExecutorV1Impl
• In SelectionOnlyCombineOperator

Only the third one keeps the proper data schema. I'd suggest creating an issue to track this, and we should properly short circuit the queries that:

• Not hitting any segment after pruning
• With always false filter
• With LIMIT 0