xiangfu0 commented on code in PR #19800:
URL: https://github.com/apache/pinot/pull/19800#discussion_r4225381031
##########
pinot-query-runtime/src/main/java/org/apache/pinot/query/runtime/operator/MultistageGroupByExecutor.java:
##########
@@ -186,15 +192,74 @@ public void processBlock(MseBlock.Data block) {
}
}
- /// Get aggregation result limited to first `maxRows` rows, ordered with
`comparator`.
- public List<Object[]> getResult(Comparator<Object[]> comparator, int
maxRows) {
+ /// Select up to `maxRows` groups using `collations`, preserving all
intermediate groups when trim is unsafe.
+ public List<Object[]> getResult(List<RelFieldCollation> collations, int
maxRows) {
+ int numKeys = _groupKeyIds.length;
+ int numCollations = collations.size();
+ AggregationFunction[] sortFunctions = new
AggregationFunction[numCollations];
+ boolean finalizeSortValues = false;
+ if (_aggType.isOutputIntermediateFormat() && !_leafReturnFinalResult) {
+ if (_groupIdGenerator.getNumGroups() <= maxRows) {
Review Comment:
Addressed in `725e9e2084`. Added one parameterized DISTINCTCOUNTHLL LEAF
case with six groups and MSE_MIN_GROUP_TRIM_SIZE=10. It exercises the early
return and verifies all six intermediate groups survive and merge across
disjoint partials to cardinalities 2 through 12. The existing ten cases still
exercise actual trimming. Validation: all 41 focused runtime tests and all four
affected-module formatting/style/license checks pass.
##########
pinot-query-runtime/src/main/java/org/apache/pinot/query/runtime/operator/MultistageGroupByExecutor.java:
##########
@@ -186,15 +192,74 @@ public void processBlock(MseBlock.Data block) {
}
}
- /// Get aggregation result limited to first `maxRows` rows, ordered with
`comparator`.
- public List<Object[]> getResult(Comparator<Object[]> comparator, int
maxRows) {
+ /// Select up to `maxRows` groups using `collations`, preserving all
intermediate groups when trim is unsafe.
+ public List<Object[]> getResult(List<RelFieldCollation> collations, int
maxRows) {
+ int numKeys = _groupKeyIds.length;
+ int numCollations = collations.size();
+ AggregationFunction[] sortFunctions = new
AggregationFunction[numCollations];
+ boolean finalizeSortValues = false;
+ if (_aggType.isOutputIntermediateFormat() && !_leafReturnFinalResult) {
+ if (_groupIdGenerator.getNumGroups() <= maxRows) {
+ return getResult(maxRows);
+ }
+ for (int i = 0; i < numCollations; i++) {
+ int index = collations.get(i).getFieldIndex();
+ if (index < numKeys) {
+ continue;
+ }
+ AggregationFunction aggFunction = _aggFunctions[index - numKeys];
+ if (aggFunction.getIntermediateResultColumnType().getStoredType() !=
ColumnDataType.OBJECT) {
+ continue;
+ }
+ // These intermediate objects already compare by the aggregate's
numeric value.
+ switch (aggFunction.getType()) {
+ case AVG, AVGMV, MINMAXRANGE, MINMAXRANGEMV, SUMPRECISION:
+ continue;
+ default:
+ break;
+ }
+ // Serialized sketches and array results lack a safe scalar trim
order. Funnel finalization consumes events.
+ // Preserve every group for the downstream aggregate and sort instead
of finalizing these intermediates.
+ if (!aggFunction.getFinalResultColumnType().getStoredType().isNumber()
+ || aggFunction.getType().name().startsWith("FUNNEL")) {
+ return getResult(Integer.MAX_VALUE);
+ }
+ sortFunctions[i] = aggFunction;
+ finalizeSortValues = true;
+ }
+ }
+ if (!finalizeSortValues) {
+ return getResult(new SortUtils.SortComparator(collations, true),
maxRows, Function.identity(),
+ Function.identity());
+ }
+ SortUtils.SortComparator[] comparators = new
SortUtils.SortComparator[numCollations];
+ for (int i = 0; i < numCollations; i++) {
+ comparators[i] = new
SortUtils.SortComparator(List.of(collations.get(i).withFieldIndex(i)), true);
+ }
+ Comparator<SortedRow> comparator = (r1, r2) -> {
+ for (int i = 0; i < numCollations; i++) {
+ int index = collations.get(i).getFieldIndex();
Review Comment:
Addressed in `725e9e2084`. Precomputed sortColumnIndexes alongside the
per-field comparators and now read the cached int indexes during heap
comparisons. This removes repeated collation list/field lookups from the object
comparator; the scalar heap path is unchanged. Validation: all 41 focused
runtime tests and all four affected-module formatting/style/license checks pass.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
