xiangfu0 commented on code in PR #19800:
URL: https://github.com/apache/pinot/pull/19800#discussion_r4224478079
##########
pinot-query-runtime/src/test/java/org/apache/pinot/query/runtime/operator/AggregateOperatorTest.java:
##########
@@ -351,6 +353,133 @@ public void testGroupTrimHintOverridesContextValue() {
assertEquals(getAggregateOperator(context, nodeHint, 5, List.of(new
RelFieldCollation(1))).getGroupTrimSize(), 30);
}
+ @DataProvider
+ public Object[][] orderedGroupTrimCases() {
+ return new Object[][]{
+ {"COUNT", null, AggType.LEAF},
+ {"COUNT", null, AggType.INTERMEDIATE},
+ {"DISTINCTCOUNT", null, AggType.LEAF},
+ {"DISTINCTCOUNT", null, AggType.INTERMEDIATE},
+ {"DISTINCTCOUNTHLL", null, AggType.LEAF},
+ {"DISTINCTCOUNTHLL", null, AggType.INTERMEDIATE},
+ {"DISTINCTCOUNTSMARTHLL", null, AggType.LEAF},
+ {"DISTINCTCOUNTSMARTHLL", null, AggType.INTERMEDIATE},
+ {"DISTINCTCOUNTSMARTHLL", "threshold=1", AggType.LEAF},
+ {"DISTINCTCOUNTSMARTHLL", "threshold=1", AggType.INTERMEDIATE}
+ };
+ }
+
+ @Test(dataProvider = "orderedGroupTrimCases")
+ public void testOrderedGroupTrimPreservesIntermediateResults(String
functionName, @Nullable String parameters,
+ AggType trimmedStage) {
+ OpChainExecutionContext context =
+
OperatorTestUtil.getContext(Map.of(QueryOptionKey.MSE_MIN_GROUP_TRIM_SIZE,
"1"));
+ ColumnDataType finalType = functionName.equals("DISTINCTCOUNTHLL") ||
functionName.equals("COUNT")
+ ? ColumnDataType.LONG : INT;
Review Comment:
**Minor convention:** per the repo coding conventions (AGENTS.md, linked
from CLAUDE.md): "Keep a ternary expression on one line when it fits.
Otherwise, put `? <true-expression>` and `: <false-expression>` on separate
lines." This ternary (and the one at `MultistageGroupByExecutor.java:308-309`,
`? TypeUtils.convert(...) : null`) does not fit on one line, so the `:` branch
belongs on its own line.
--
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]
