Skip to content

Commit 16d061b

Browse files
authored
fix: Static aggregations when a data index uses different names for the key columns should remap names correctly (#6360)
Fixes #6359
1 parent f1105da commit 16d061b

File tree

2 files changed

+33
-8
lines changed

2 files changed

+33
-8
lines changed

engine/table/src/main/java/io/deephaven/engine/table/impl/by/ChunkedOperatorAggregationHelper.java

+19-7
Original file line numberDiff line numberDiff line change
@@ -163,14 +163,13 @@ private static QueryTable aggregation(
163163
final PermuteKernel[] permuteKernels = ac.makePermuteKernels();
164164

165165
if (dataIndex != null && initialKeys == null && !input.isRefreshing()) {
166-
return staticIndexedAggregation(dataIndex, keyNames, ac);
166+
return staticIndexedAggregation(input, dataIndex, keyNames, ac);
167167
}
168168

169169
final Table symbolTable;
170170
final boolean useSymbolTable;
171171
if (!input.isRefreshing() && control.considerSymbolTables(input, dataIndex != null, keySources)) {
172172
Assert.eq(keySources.length, "keySources.length", 1);
173-
174173
symbolTable = ((SymbolTableSource<?>) keySources[0]).getStaticSymbolTable(input.getRowSet(),
175174
control.useSymbolTableLookupCaching());
176175
useSymbolTable = control.useSymbolTables(input.size(), symbolTable.size());
@@ -1619,6 +1618,7 @@ private static void modifySlots(RowSetBuilderRandom modifiedBuilder, IntChunk<Ch
16191618

16201619
@NotNull
16211620
private static QueryTable staticIndexedAggregation(
1621+
final QueryTable input,
16221622
final BasicDataIndex dataIndex,
16231623
final String[] keyNames,
16241624
final AggregationContext ac) {
@@ -1628,10 +1628,22 @@ private static QueryTable staticIndexedAggregation(
16281628

16291629
final Map<String, ColumnSource<?>> resultColumnSourceMap = new LinkedHashMap<>();
16301630

1631-
// Add the index key columns directly to the result table.
1632-
final Table indexKeyTable = indexTable.flatten().select(keyNames);
1633-
for (final String keyName : keyNames) {
1634-
resultColumnSourceMap.put(keyName, indexKeyTable.getColumnSource(keyName));
1631+
// Add the index table's key columns directly to the result table (after flattening and selecting).
1632+
final Table indexKeyTable = indexTable.flatten().select(dataIndex.keyColumnNames().toArray(String[]::new));
1633+
// Index key names corresponding to the input key names in the order of the input key names
1634+
final String[] dataIndexKeyNames = new String[keyNames.length];
1635+
final Map<String, ColumnSource<?>> inputNamesToSources = input.getColumnSourceMap();
1636+
final Map<ColumnSource<?>, String> indexedSourcesToNames = dataIndex.keyColumnNamesByIndexedColumn();
1637+
for (int ki = 0; ki < keyNames.length; ki++) {
1638+
final String keyName = keyNames[ki];
1639+
// Note that we must be careful to find the index key column sources by indexed (input) column source, not
1640+
// by input key name, as the input key names may not match the indexed column names.
1641+
// We build dataIndexKeyNames as part of this mapping, so we can use them for the row lookup remapping.
1642+
final ColumnSource<?> keyColumnSource = inputNamesToSources.get(keyName);
1643+
final String dataIndexKeyName = indexedSourcesToNames.get(keyColumnSource);
1644+
dataIndexKeyNames[ki] = dataIndexKeyName;
1645+
final ColumnSource<?> indexKeyColumnSource = indexKeyTable.getColumnSource(dataIndexKeyName);
1646+
resultColumnSourceMap.put(keyName, indexKeyColumnSource);
16351647
}
16361648
ac.getResultColumns(resultColumnSourceMap);
16371649

@@ -1648,7 +1660,7 @@ private static QueryTable staticIndexedAggregation(
16481660
// Create a lookup function from the index table
16491661
ac.supplyRowLookup(() -> {
16501662
final ToLongFunction<Object> lookupKeyToRowKey =
1651-
DataIndexUtils.buildRowKeyMappingFunction(indexKeyTable, keyNames);
1663+
DataIndexUtils.buildRowKeyMappingFunction(indexKeyTable, dataIndexKeyNames);
16521664
return key -> (int) lookupKeyToRowKey.applyAsLong(key);
16531665
});
16541666

engine/table/src/test/java/io/deephaven/engine/table/impl/QueryTableAggregationTest.java

+14-1
Original file line numberDiff line numberDiff line change
@@ -198,13 +198,26 @@ public int initialHashTableSize(@NotNull final Table table) {
198198

199199
@Test
200200
public void testStaticGroupedByWithChunks() {
201-
final Table input1 = emptyTable(10000).update("A=Integer.toString(i % 5)", "B=i / 5");
201+
final Table input1 = emptyTable(10000).update("A=Integer.toString(i % 5)", "B=i / 5", "C=ii");
202202

203203
DataIndexer.getOrCreateDataIndex(input1, "A");
204204
DataIndexer.getOrCreateDataIndex(input1, "B");
205+
DataIndexer.getOrCreateDataIndex(input1, "A", "B");
205206

206207
individualStaticByTest(input1, null, "A");
207208
individualStaticByTest(input1, null, "B");
209+
individualStaticByTest(input1, null, "A", "B");
210+
211+
// Test scenario where the key columns have different order than in the index
212+
individualStaticByTest(input1, null, "B", "A");
213+
214+
// Test scenarios where the key columns have different names than in the index
215+
individualStaticByTest(input1.renameColumns("D=A", "E=B"), null, "D");
216+
individualStaticByTest(input1.renameColumns("D=A", "E=B"), null, "E");
217+
individualStaticByTest(input1.renameColumns("D=A", "E=B"), null, "D", "E");
218+
219+
// Test scenario where the key columns have different names and order than in the index
220+
individualStaticByTest(input1.renameColumns("D=A", "E=B"), null, "E", "D");
208221
}
209222

210223
@Test

0 commit comments

Comments
 (0)