Skip to content

Commit bd488ae

Browse files
authored
Fix GROUP BY DISTINCTCOUNT ClassCastException in mergeDataTablesOnly (#18842)
mergeDataTablesOnly reused getIndexedTable, which unconditionally called indexedTable.finish(true, true). For a GROUP BY query with an OBJECT-typed intermediate aggregate (DISTINCTCOUNT, DISTINCTCOUNTHLL, etc.), that storeFinalResult=true call: - mutated DataSchema._columnDataTypes from OBJECT to the final-result type in place (IndexedTable.java:170-174), and - replaced each row's value with extractFinalResult(value) (Set → Integer for DISTINCTCOUNT) at IndexedTable.java:195. The subsequent buildIntermediateDataTable read DataSchema's cached _storedColumnDataTypes — populated earlier from the pre-finalize OBJECT schema and never invalidated by finish — so the OBJECT branch fired and handed the now-Integer value into BaseDistinctAggregateAggregationFunction.serializeIntermediateResult(Set), which threw ClassCastException. Fix: move finish() out of getIndexedTable and let each caller pick the right mode. reduceResult keeps finish(true, true) (downstream consumers expect final scalars). mergeDataTablesOnly calls finish(true, false) so aggregate values stay as intermediates and round-trip through buildIntermediateDataTable correctly. Adds a testGroupByDistinctCountObjectRoundTrip regression: server emits intermediate OBJECT-encoded Sets, merge produces a re-injectable intermediate DataTable, and reducing that intermediate matches a direct reduce of the same servers.
1 parent 9ceca0f commit bd488ae

2 files changed

Lines changed: 76 additions & 4 deletions

File tree

pinot-core/src/main/java/org/apache/pinot/core/query/reduce/GroupByDataTableReducer.java

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -133,11 +133,11 @@ public DataTable mergeDataTablesOnly(String tableName, DataSchema dataSchema,
133133
// When servers are configured to return final aggregate state, the input DataTables hold final
134134
// (not intermediate) values, so the merge-only contract — "produce an intermediate DataTable that
135135
// can be re-merged via the normal reduce path" — cannot be honored.
136-
if (_queryContext.isServerReturnFinalResult()) {
136+
if (_queryContext.isServerReturnFinalResult() || _queryContext.isServerReturnFinalResultKeyUnpartitioned()) {
137137
throw new UnsupportedOperationException(
138138
"Merge-only reduction is not supported when servers return final aggregate results "
139-
+ "(server.returnFinalResult / isServerReturnFinalResult); input would be final-typed, "
140-
+ "not intermediate.");
139+
+ "(serverReturnFinalResult / serverReturnFinalResultKeyUnpartitioned); input would be "
140+
+ "final-typed, not intermediate.");
141141
}
142142
dataSchema = ReducerDataSchemaUtils.canonicalizeDataSchemaForGroupBy(_queryContext, dataSchema);
143143
try {
@@ -147,6 +147,8 @@ public DataTable mergeDataTablesOnly(String tableName, DataSchema dataSchema,
147147
Collection<DataTable> dataTables = dataTableMap.values();
148148
// Reuse the regular reduce's merge: builds the IndexedTable of group keys + intermediate agg state.
149149
IndexedTable indexedTable = getIndexedTable(dataSchema, dataTables, reducerContext);
150+
// Keep aggregate values as intermediates so the output can be re-merged through the regular reduce path.
151+
indexedTable.finish(false, false);
150152
DataTable mergedDataTable = buildIntermediateDataTable(dataSchema, indexedTable);
151153
if (indexedTable.isTrimmed() && _queryContext.isUnsafeTrim()) {
152154
mergedDataTable.getMetadata().put(MetadataKey.GROUPS_TRIMMED.getName(), "true");
@@ -175,6 +177,10 @@ private void reduceResult(BrokerResponseNative brokerResponseNative, DataSchema
175177
BrokerMetrics brokerMetrics) {
176178
// NOTE: This step will modify the data schema and also return final aggregate results.
177179
IndexedTable indexedTable = getIndexedTable(dataSchema, dataTables, reducerContext);
180+
// Sort + finalize: mutate _dataSchema column types to final-result types and replace each row's value
181+
// with extractFinalResult(...). Required by the regular reduce path: the downstream consumers
182+
// (PostAggregationHandler, HavingFilterHandler, ResultTable serialization) expect final scalars.
183+
indexedTable.finish(true, true);
178184
if (indexedTable.isTrimmed() && _queryContext.isUnsafeTrim()) {
179185
brokerResponseNative.setGroupsTrimmed(true);
180186
}
@@ -408,7 +414,12 @@ public void runJob() {
408414
}
409415
}
410416

411-
indexedTable.finish(true, true);
417+
// NOTE: finish(...) is invoked by the caller, not here. The two callers want different semantics:
418+
// - reduceResult (regular reduce → ResultTable) needs finish(true, true) to extract final results.
419+
// - mergeDataTablesOnly (merge-only intermediate output) needs finish(true, false) so the aggregate
420+
// values stay as intermediates (e.g. Set for DISTINCTCOUNT, not Integer); otherwise the next
421+
// buildIntermediateDataTable's serializeIntermediateResult(...) call casts the final scalar back to
422+
// the intermediate type and throws ClassCastException.
412423
return indexedTable;
413424
}
414425

pinot-core/src/test/java/org/apache/pinot/core/query/reduce/MergeDataTablesOnlyTest.java

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,34 @@ public void testGroupByRoundTrip() {
118118
assertRoundTrip(query, serverTables);
119119
}
120120

121+
/// Verifies that a GROUP BY query selecting DISTINCTCOUNT keeps OBJECT aggregate state re-mergeable.
122+
@Test
123+
public void testGroupByDistinctCountObjectRoundTrip()
124+
throws IOException {
125+
String query = "SELECT col1, DISTINCTCOUNT(col2) FROM testTable GROUP BY col1";
126+
BrokerRequest brokerRequest = CalciteSqlCompiler.compileToBrokerRequest(query);
127+
BrokerResponseNative baseline = reduce(brokerRequest, toMap(buildDistinctCountServerTables(query)));
128+
DataTable merged = merge(brokerRequest, toMap(buildDistinctCountServerTables(query)));
129+
assertNotNull(merged, "merge produced null");
130+
BrokerResponseNative viaMerge = reduce(brokerRequest, singletonMap(merged));
131+
assertResultTablesEquivalent(baseline.getResultTable(), viaMerge.getResultTable());
132+
}
133+
134+
/// Two per-server intermediate DataTables for selecting `DISTINCTCOUNT(col2)` grouped by `col1`.
135+
private static List<DataTable> buildDistinctCountServerTables(String query)
136+
throws IOException {
137+
AggregationFunction aggFunction = aggFunctions(query)[0];
138+
DataSchema schema = new DataSchema(new String[]{"col1", "distinctcount(col2)"},
139+
new ColumnDataType[]{ColumnDataType.INT, ColumnDataType.OBJECT});
140+
return List.of(
141+
buildGroupByWithObject(schema, aggFunction,
142+
new Object[]{1, new IntOpenHashSet(new int[]{1, 2, 3})},
143+
new Object[]{2, new IntOpenHashSet(new int[]{4, 5})}),
144+
buildGroupByWithObject(schema, aggFunction,
145+
new Object[]{1, new IntOpenHashSet(new int[]{3, 4, 5})},
146+
new Object[]{3, new IntOpenHashSet(new int[]{6, 7})}));
147+
}
148+
121149
@Test
122150
public void testDistinctRoundTrip() {
123151
String query = "SELECT DISTINCT col1 FROM testTable";
@@ -151,6 +179,18 @@ public void testGroupByServerReturnFinalResultRejected() {
151179
assertThrows(UnsupportedOperationException.class, () -> merge(brokerRequest, map));
152180
}
153181

182+
@Test
183+
public void testGroupByServerReturnFinalResultKeyUnpartitionedRejected() {
184+
BrokerRequest brokerRequest = CalciteSqlCompiler.compileToBrokerRequest(
185+
"SELECT col1, DISTINCTCOUNT(col2) FROM testTable GROUP BY col1");
186+
brokerRequest.getPinotQuery()
187+
.putToQueryOptions(Broker.Request.QueryOptionKey.SERVER_RETURN_FINAL_RESULT_KEY_UNPARTITIONED, "true");
188+
DataSchema schema = new DataSchema(new String[]{"col1", "distinctcount(col2)"},
189+
new ColumnDataType[]{ColumnDataType.INT, ColumnDataType.INT});
190+
Map<ServerRoutingInstance, DataTable> map = singletonMap(buildGroupBy(schema, new Object[][]{{1, 2}}));
191+
assertThrows(UnsupportedOperationException.class, () -> merge(brokerRequest, map));
192+
}
193+
154194
@Test
155195
public void testGroupByLimitZeroRoundTrip() {
156196
// LIMIT 0 on group-by: mergeDataTablesOnly does NOT apply LIMIT (intermediate is unlimited),
@@ -489,6 +529,27 @@ private static DataTable buildGroupBy(DataSchema schema, Object[][] rows) {
489529
}
490530
}
491531

532+
/**
533+
* Builds a GROUP BY DataTable with one INT key column followed by one OBJECT aggregate column whose
534+
* value is the intermediate {@code Set} (or other aggregate state) the server would have emitted.
535+
* Mirrors what {@code GroupByResultsBlock.getDataTable()} produces on the server side for the
536+
* non-null-handling path: scalar key written directly, OBJECT column written via
537+
* {@code serializeIntermediateResult}.
538+
*/
539+
private static DataTable buildGroupByWithObject(DataSchema schema, AggregationFunction aggFunction,
540+
Object[]... rows)
541+
throws IOException {
542+
DataTableBuilder builder = DataTableBuilderFactory.getDataTableBuilder(schema);
543+
for (Object[] row : rows) {
544+
builder.startRow();
545+
// Single INT key column at index 0; OBJECT aggregate intermediate at index 1.
546+
builder.setColumn(0, (int) row[0]);
547+
builder.setColumn(1, aggFunction.serializeIntermediateResult(row[1]));
548+
builder.finishRow();
549+
}
550+
return builder.build();
551+
}
552+
492553
private static DataTable buildObjectRow(DataSchema schema, AggregationFunction aggFunction, Object intermediate)
493554
throws IOException {
494555
DataTableBuilder builder = DataTableBuilderFactory.getDataTableBuilder(schema);

0 commit comments

Comments
 (0)