diff --git a/engine/table/src/main/java/io/deephaven/engine/table/impl/by/AggregationContextFactory.java b/engine/table/src/main/java/io/deephaven/engine/table/impl/by/AggregationContextFactory.java index 37805c92e01..00a007fdf93 100644 --- a/engine/table/src/main/java/io/deephaven/engine/table/impl/by/AggregationContextFactory.java +++ b/engine/table/src/main/java/io/deephaven/engine/table/impl/by/AggregationContextFactory.java @@ -8,18 +8,6 @@ */ public interface AggregationContextFactory { - /** - * Should we allow substitution with a {@link KeyOnlyAggregationFactory} (e.g. selectDistinct) when there are only - * key columns? Instances whose operators could have side effects or are already {@link KeyOnlyAggregationFactory} - * should return false. - * - * @return Whether to allow a {@link KeyOnlyAggregationFactory} to be substituted for this when there are only key - * columns - */ - default boolean allowKeyOnlySubstitution() { - return false; - } - /** * Make an {@link AggregationContext} for this aggregation. * diff --git a/engine/table/src/main/java/io/deephaven/engine/table/impl/by/AggregationFactory.java b/engine/table/src/main/java/io/deephaven/engine/table/impl/by/AggregationFactory.java index a98c59febf8..a4032a59d59 100644 --- a/engine/table/src/main/java/io/deephaven/engine/table/impl/by/AggregationFactory.java +++ b/engine/table/src/main/java/io/deephaven/engine/table/impl/by/AggregationFactory.java @@ -1386,39 +1386,11 @@ public AggregationContextFactory makeAggregationContextFactory() { operators.add(formulaChunkedOperator); inputNames.add(CollectionUtil.ZERO_LENGTH_STRING_ARRAY); } else if (isWeightedAverage || isWeightedSum) { - final String weightName; - - if (isWeightedAverage) { - weightName = ((WeightedAverageSpecImpl) inputAggregationSpec) - .getWeightName(); - } else { - weightName = - ((WeightedSumSpecImpl) inputAggregationSpec).getWeightName(); - } - - final ColumnSource weightSource = table.getColumnSource(weightName); - final DoubleWeightRecordingInternalOperator weightOperator = - new DoubleWeightRecordingInternalOperator(weightSource.getChunkType()); - inputColumns.add(weightSource); - operators.add(weightOperator); - - inputNames.add(Stream - .concat(Stream.of(weightName), - Arrays.stream(comboMatchPairs).map(MatchPair::rightColumn)) - .toArray(String[]::new)); - - Arrays.stream(comboMatchPairs).forEach(mp -> { - final ColumnSource columnSource = table.getColumnSource(mp.rightColumn()); - inputColumns.add(columnSource); - inputNames.add(new String[] {weightName, mp.rightColumn()}); - if (isWeightedAverage) { - operators.add(new ChunkedWeightedAverageOperator(columnSource.getChunkType(), - weightOperator, mp.leftColumn())); - } else { - operators.add(new DoubleChunkedWeightedSumOperator(columnSource.getChunkType(), - weightOperator, mp.leftColumn())); - } - }); + final String weightName = isWeightedAverage + ? ((WeightedAverageSpecImpl) inputAggregationSpec).getWeightName() + : ((WeightedSumSpecImpl) inputAggregationSpec).getWeightName(); + WeightedAverageSumAggregationFactory.getOperatorsAndInputs(table, + weightName, isWeightedSum, comboMatchPairs, operators, inputNames, inputColumns); } else { throw new UnsupportedOperationException( "Unknown AggregationElementImpl: " + inputAggregationSpec.getClass()); diff --git a/engine/table/src/main/java/io/deephaven/engine/table/impl/by/ChunkedOperatorAggregationHelper.java b/engine/table/src/main/java/io/deephaven/engine/table/impl/by/ChunkedOperatorAggregationHelper.java index 9992a06b6e3..f5b9d90aa12 100644 --- a/engine/table/src/main/java/io/deephaven/engine/table/impl/by/ChunkedOperatorAggregationHelper.java +++ b/engine/table/src/main/java/io/deephaven/engine/table/impl/by/ChunkedOperatorAggregationHelper.java @@ -48,11 +48,6 @@ @SuppressWarnings("rawtypes") public class ChunkedOperatorAggregationHelper { - @VisibleForTesting - public static boolean KEY_ONLY_SUBSTITUTION_ENABLED = - Configuration.getInstance() - .getBooleanWithDefault("ChunkedOperatorAggregationHelper.enableKeyOnlySubstitution", true); - static final int CHUNK_SIZE = 1 << 12; public static QueryTable aggregation(AggregationContextFactory aggregationContextFactory, QueryTable queryTable, @@ -68,15 +63,6 @@ public static QueryTable aggregation(AggregationControl control, && Arrays.stream(groupByColumns).anyMatch(selectColumn -> !selectColumn.isRetain()); final QueryTable withView = !viewRequired ? queryTable : (QueryTable) queryTable.updateView(groupByColumns); - final AggregationContextFactory aggregationContextFactoryToUse; - if (KEY_ONLY_SUBSTITUTION_ENABLED - && withView.getDefinition().getColumns().length == groupByColumns.length - && aggregationContextFactory.allowKeyOnlySubstitution()) { - aggregationContextFactoryToUse = new KeyOnlyAggregationFactory(); - } else { - aggregationContextFactoryToUse = aggregationContextFactory; - } - if (queryTable.hasAttribute(Table.REVERSE_LOOKUP_ATTRIBUTE)) { withView.setAttribute(Table.REVERSE_LOOKUP_ATTRIBUTE, queryTable.getAttribute(Table.REVERSE_LOOKUP_ATTRIBUTE)); @@ -86,9 +72,9 @@ public static QueryTable aggregation(AggregationControl control, final SwapListener swapListener = withView.createSwapListenerIfRefreshing(SwapListener::new); withView.initializeWithSnapshot( - "by(" + aggregationContextFactoryToUse + ", " + Arrays.toString(groupByColumns) + ")", swapListener, + "by(" + aggregationContextFactory + ", " + Arrays.toString(groupByColumns) + ")", swapListener, (usePrev, beforeClockValue) -> { - resultHolder.setValue(aggregation(control, swapListener, aggregationContextFactoryToUse, withView, + resultHolder.setValue(aggregation(control, swapListener, aggregationContextFactory, withView, groupByColumns, usePrev)); return true; }); diff --git a/engine/table/src/main/java/io/deephaven/engine/table/impl/by/FirstOrLastByAggregationFactory.java b/engine/table/src/main/java/io/deephaven/engine/table/impl/by/FirstOrLastByAggregationFactory.java index 5e062fd7bd9..bf5891f9e72 100644 --- a/engine/table/src/main/java/io/deephaven/engine/table/impl/by/FirstOrLastByAggregationFactory.java +++ b/engine/table/src/main/java/io/deephaven/engine/table/impl/by/FirstOrLastByAggregationFactory.java @@ -26,11 +26,6 @@ public FirstOrLastByAggregationFactory(boolean isFirst, String exposeRedirection this.exposeRedirection = exposeRedirection == null ? null : NameValidator.validateColumnName(exposeRedirection); } - @Override - public boolean allowKeyOnlySubstitution() { - return true; - } - @Override public AggregationContext makeAggregationContext(@NotNull final Table table, @NotNull final String... groupByColumns) { diff --git a/engine/table/src/main/java/io/deephaven/engine/table/impl/by/FormulaAggregationFactory.java b/engine/table/src/main/java/io/deephaven/engine/table/impl/by/FormulaAggregationFactory.java index 70a8af9e7b9..66a094fa1a2 100644 --- a/engine/table/src/main/java/io/deephaven/engine/table/impl/by/FormulaAggregationFactory.java +++ b/engine/table/src/main/java/io/deephaven/engine/table/impl/by/FormulaAggregationFactory.java @@ -27,11 +27,6 @@ public FormulaAggregationFactory(@NotNull final String formula, @NotNull final S this.columnParamName = columnParamName; } - @Override - public boolean allowKeyOnlySubstitution() { - return true; - } - @Override public AggregationContext makeAggregationContext(@NotNull final Table inputTable, @NotNull final String... groupByColumnNames) { diff --git a/engine/table/src/main/java/io/deephaven/engine/table/impl/by/FreezeByAggregationFactory.java b/engine/table/src/main/java/io/deephaven/engine/table/impl/by/FreezeByAggregationFactory.java index 39435d13f28..40ad3fc6044 100644 --- a/engine/table/src/main/java/io/deephaven/engine/table/impl/by/FreezeByAggregationFactory.java +++ b/engine/table/src/main/java/io/deephaven/engine/table/impl/by/FreezeByAggregationFactory.java @@ -18,10 +18,6 @@ import java.util.*; public class FreezeByAggregationFactory implements AggregationContextFactory { - @Override - public boolean allowKeyOnlySubstitution() { - return true; - } @Override public AggregationContext makeAggregationContext(@NotNull final Table table, diff --git a/engine/table/src/main/java/io/deephaven/engine/table/impl/by/GroupByAggregationFactory.java b/engine/table/src/main/java/io/deephaven/engine/table/impl/by/GroupByAggregationFactory.java index 5a03dacef0c..54c378ee33c 100644 --- a/engine/table/src/main/java/io/deephaven/engine/table/impl/by/GroupByAggregationFactory.java +++ b/engine/table/src/main/java/io/deephaven/engine/table/impl/by/GroupByAggregationFactory.java @@ -26,11 +26,6 @@ public static AggregationContextFactory getInstance() { private GroupByAggregationFactory() {} - @Override - public boolean allowKeyOnlySubstitution() { - return true; - } - @Override public AggregationContext makeAggregationContext(@NotNull final Table inputTable, @NotNull final String... groupByColumnNames) { diff --git a/engine/table/src/main/java/io/deephaven/engine/table/impl/by/NonKeyColumnAggregationFactory.java b/engine/table/src/main/java/io/deephaven/engine/table/impl/by/NonKeyColumnAggregationFactory.java index 538ecc8cecd..11dd67461c1 100644 --- a/engine/table/src/main/java/io/deephaven/engine/table/impl/by/NonKeyColumnAggregationFactory.java +++ b/engine/table/src/main/java/io/deephaven/engine/table/impl/by/NonKeyColumnAggregationFactory.java @@ -17,11 +17,6 @@ public NonKeyColumnAggregationFactory(IterativeChunkedOperatorFactory iterativeC this.iterativeChunkedOperatorFactory = iterativeChunkedOperatorFactory; } - @Override - public boolean allowKeyOnlySubstitution() { - return true; - } - @Override public AggregationContext makeAggregationContext(@NotNull final Table table, @NotNull final String... groupByColumns) { diff --git a/engine/table/src/main/java/io/deephaven/engine/table/impl/by/SortedFirstOrLastByAggregationFactory.java b/engine/table/src/main/java/io/deephaven/engine/table/impl/by/SortedFirstOrLastByAggregationFactory.java index ed4f7f5f4f1..060fbebbfec 100644 --- a/engine/table/src/main/java/io/deephaven/engine/table/impl/by/SortedFirstOrLastByAggregationFactory.java +++ b/engine/table/src/main/java/io/deephaven/engine/table/impl/by/SortedFirstOrLastByAggregationFactory.java @@ -28,11 +28,6 @@ public SortedFirstOrLastByAggregationFactory(final boolean isFirst, final boolea this.sortColumns = sortColumns; } - @Override - public boolean allowKeyOnlySubstitution() { - return true; - } - @Override public AggregationContext makeAggregationContext(@NotNull final Table table, @NotNull final String... groupByColumns) { diff --git a/engine/table/src/main/java/io/deephaven/engine/table/impl/by/WeightedAverageSumAggregationFactory.java b/engine/table/src/main/java/io/deephaven/engine/table/impl/by/WeightedAverageSumAggregationFactory.java index 1c6711936fc..060c1bbe5da 100644 --- a/engine/table/src/main/java/io/deephaven/engine/table/impl/by/WeightedAverageSumAggregationFactory.java +++ b/engine/table/src/main/java/io/deephaven/engine/table/impl/by/WeightedAverageSumAggregationFactory.java @@ -1,13 +1,14 @@ package io.deephaven.engine.table.impl.by; -import io.deephaven.engine.table.ChunkSource; -import io.deephaven.engine.table.Table; -import io.deephaven.engine.table.ColumnSource; -import io.deephaven.chunk.*; +import io.deephaven.chunk.ChunkType; import io.deephaven.chunk.attributes.Values; +import io.deephaven.engine.table.*; +import org.apache.commons.lang3.mutable.MutableBoolean; import org.jetbrains.annotations.NotNull; import java.util.*; +import java.util.stream.Collectors; +import java.util.stream.Stream; public class WeightedAverageSumAggregationFactory implements AggregationContextFactory { private final String weightName; @@ -18,11 +19,6 @@ public WeightedAverageSumAggregationFactory(final String weightName, boolean isS this.isSum = isSum; } - @Override - public boolean allowKeyOnlySubstitution() { - return true; - } - @Override public AggregationContext makeAggregationContext(@NotNull final Table table, @NotNull final String... groupByColumns) { @@ -34,135 +30,147 @@ private AggregationContext getAllColumnOperators(Table withView, String[] groupB final int operatorColumnCount = withView.getColumnSourceMap().size() - groupByNames.size() - 1; final List operators = new ArrayList<>(operatorColumnCount + 1); + final List inputNames = new ArrayList<>(operatorColumnCount); final List> inputColumns = new ArrayList<>(operatorColumnCount + 1); - final List inputNames = new ArrayList<>(operatorColumnCount); - final List isIntegerResult = new ArrayList<>(operatorColumnCount); - final List floatColumnNames = new ArrayList<>(operatorColumnCount); - final List integerColumnNames = new ArrayList<>(operatorColumnCount); - final ColumnSource weightSource = withView.getColumnSource(weightName); - boolean weightSourceIsFloatingPoint = - weightSource.getChunkType() == ChunkType.Double || weightSource.getChunkType() == ChunkType.Float; - boolean anyIntegerResults = !weightSourceIsFloatingPoint && isSum - && withView.getColumnSourceMap().values().stream() - .anyMatch(cs -> cs.getChunkType() == ChunkType.Long || cs.getChunkType() == ChunkType.Int - || cs.getChunkType() == ChunkType.Short || cs.getChunkType() == ChunkType.Byte - || cs.getChunkType() == ChunkType.Char); - boolean anyFloatResults = weightSourceIsFloatingPoint || !isSum || withView.getColumnSourceMap().values() - .stream().anyMatch(cs -> cs.getChunkType() == ChunkType.Float || cs.getChunkType() == ChunkType.Double); + final MatchPair[] resultPairs = withView.getDefinition().getColumnStream() + .map(ColumnDefinition::getName) + .filter(cn -> !cn.equals(weightName) && !groupByNames.contains(cn)) + .map(cn -> new MatchPair(cn, cn)) + .toArray(MatchPair[]::new); + getOperatorsAndInputs(withView, weightName, isSum, resultPairs, operators, inputNames, inputColumns); - final DoubleWeightRecordingInternalOperator doubleWeightOperator; - if (anyFloatResults) { - doubleWeightOperator = new DoubleWeightRecordingInternalOperator(weightSource.getChunkType()); - // noinspection unchecked - inputColumns.add(weightSource); - operators.add(doubleWeightOperator); + // noinspection unchecked + return new AggregationContext( + operators.toArray(IterativeChunkedAggregationOperator[]::new), + inputNames.toArray(String[][]::new), + inputColumns.toArray(ChunkSource.WithPrev[]::new)); + } + + private static boolean isFloatingPoint(@NotNull final ChunkType chunkType) { + return chunkType == ChunkType.Float + || chunkType == ChunkType.Double; + } + + private static boolean isInteger(@NotNull final ChunkType chunkType) { + return chunkType == ChunkType.Char + || chunkType == ChunkType.Byte + || chunkType == ChunkType.Short + || chunkType == ChunkType.Int + || chunkType == ChunkType.Long; + } + + private enum ResultType { + INTEGER, FLOATING_POINT + } + + private static class Result { + private final MatchPair pair; + private final ResultType type; + private final ColumnSource source; + + private Result(MatchPair pair, ResultType type, ColumnSource source) { + this.pair = pair; + this.type = type; + this.source = source; + } + } + + static void getOperatorsAndInputs(Table withView, + String weightName, + boolean isSum, + MatchPair[] resultPairs, + List resultOperators, + List resultInputNames, + List> resultInputColumns) { + final ColumnSource weightSource = withView.getColumnSource(weightName); + final boolean weightSourceIsFloatingPoint; + if (isInteger(weightSource.getChunkType())) { + weightSourceIsFloatingPoint = false; + } else if (isFloatingPoint(weightSource.getChunkType())) { + weightSourceIsFloatingPoint = true; } else { - doubleWeightOperator = null; + throw new UnsupportedOperationException(String.format("Invalid type %s in weight column %s for %s", + weightSource.getType(), weightName, toString(isSum, weightName))); } + final MutableBoolean anyIntegerResults = new MutableBoolean(); + final MutableBoolean anyFloatingPointResults = new MutableBoolean(); + final List results = Arrays.stream(resultPairs).map(pair -> { + final ColumnSource inputSource = withView.getColumnSource(pair.rightColumn); + final ResultType resultType; + if (isInteger(inputSource.getChunkType())) { + if (!weightSourceIsFloatingPoint && isSum) { + anyIntegerResults.setTrue(); + resultType = ResultType.INTEGER; + } else { + anyFloatingPointResults.setTrue(); + resultType = ResultType.FLOATING_POINT; + } + } else if (isFloatingPoint(inputSource.getChunkType())) { + anyFloatingPointResults.setTrue(); + resultType = ResultType.FLOATING_POINT; + } else { + throw new UnsupportedOperationException(String.format("Invalid type %s in column %s for %s", + inputSource.getType(), pair.rightColumn, toString(isSum, weightName))); + } + return new Result(pair, resultType, inputSource); + }).collect(Collectors.toList()); + final LongWeightRecordingInternalOperator longWeightOperator; - if (anyIntegerResults) { + if (anyIntegerResults.booleanValue()) { longWeightOperator = new LongWeightRecordingInternalOperator(weightSource.getChunkType()); + resultOperators.add(longWeightOperator); + resultInputNames.add(Stream.concat( + Stream.of(weightName), + results.stream() + .filter(r -> r.type == ResultType.INTEGER).map(r -> r.pair.rightColumn)) + .toArray(String[]::new)); // noinspection unchecked - inputColumns.add(weightSource); - operators.add(longWeightOperator); + resultInputColumns.add(weightSource); } else { longWeightOperator = null; } - withView.getColumnSourceMap().forEach((name, columnSource) -> { - if (groupByNames.contains(name)) { - return; - } - if (name.equals(weightName)) { - return; - } - + final DoubleWeightRecordingInternalOperator doubleWeightOperator; + if (anyFloatingPointResults.booleanValue()) { + doubleWeightOperator = new DoubleWeightRecordingInternalOperator(weightSource.getChunkType()); + resultOperators.add(doubleWeightOperator); + resultInputNames.add(Stream.concat( + Stream.of(weightName), + results.stream() + .filter(r -> r.type == ResultType.FLOATING_POINT).map(r -> r.pair.rightColumn)) + .toArray(String[]::new)); // noinspection unchecked - inputColumns.add(columnSource); - inputNames.add(name); - if (isSum) { - final boolean isInteger; - - if (weightSourceIsFloatingPoint) { - isInteger = false; - } else { - switch (columnSource.getChunkType()) { - case Char: - case Byte: - case Short: - case Int: - case Long: - isInteger = true; - break; - case Double: - case Float: - isInteger = false; - break; - default: - throw new UnsupportedOperationException( - "Invalid chunk type for weightedSum: " + columnSource.getChunkType()); - } - } + resultInputColumns.add(weightSource); + } else { + doubleWeightOperator = null; + } - isIntegerResult.add(isInteger); - if (isInteger) { - integerColumnNames.add(name); - operators.add( - new LongChunkedWeightedSumOperator(columnSource.getChunkType(), longWeightOperator, name)); + results.forEach(r -> { + if (isSum) { + if (r.type == ResultType.INTEGER) { + resultOperators.add(new LongChunkedWeightedSumOperator( + r.source.getChunkType(), longWeightOperator, r.pair.leftColumn)); } else { - floatColumnNames.add(name); - operators.add(new DoubleChunkedWeightedSumOperator(columnSource.getChunkType(), - doubleWeightOperator, name)); + resultOperators.add(new DoubleChunkedWeightedSumOperator( + r.source.getChunkType(), doubleWeightOperator, r.pair.leftColumn)); } } else { - isIntegerResult.add(false); - floatColumnNames.add(name); - operators.add( - new ChunkedWeightedAverageOperator(columnSource.getChunkType(), doubleWeightOperator, name)); + resultOperators.add(new ChunkedWeightedAverageOperator( + r.source.getChunkType(), doubleWeightOperator, r.pair.leftColumn)); } + resultInputNames.add(new String[] {r.pair.rightColumn, weightName}); + resultInputColumns.add(r.source); }); - - final int inputColumnCount = inputNames.size(); - final int weightOperators = (anyFloatResults ? 1 : 0) + (anyIntegerResults ? 1 : 0); - final String[][] inputNameArray = new String[inputColumnCount + weightOperators][]; - int idx = 0; - if (doubleWeightOperator != null) { - inputNameArray[idx] = new String[floatColumnNames.size() + 1]; - inputNameArray[idx][0] = weightName; - for (int ii = 0; ii < floatColumnNames.size(); ++ii) { - inputNameArray[idx][1 + ii] = floatColumnNames.get(ii); - } - idx++; - } - if (longWeightOperator != null) { - inputNameArray[idx] = new String[integerColumnNames.size() + 1]; - inputNameArray[idx][0] = weightName; - for (int ii = 0; ii < integerColumnNames.size(); ++ii) { - inputNameArray[idx][1 + ii] = integerColumnNames.get(ii); - } - idx++; - } - - for (final String columnName : inputNames) { - inputNameArray[idx] = new String[2]; - inputNameArray[idx][0] = columnName; - inputNameArray[idx][1] = weightName; - idx++; - } - - // noinspection unchecked - return new AggregationContext( - operators.toArray( - IterativeChunkedAggregationOperator.ZERO_LENGTH_ITERATIVE_CHUNKED_AGGREGATION_OPERATOR_ARRAY), - inputNameArray, - inputColumns.toArray(ChunkSource.WithPrev.ZERO_LENGTH_CHUNK_SOURCE_WITH_PREV_ARRAY)); } @Override public String toString() { - return "Weighted" + (isSum ? "Sum" : "Avg") + "(" + weightName + ")"; + return toString(isSum, weightName); } + private static String toString(final boolean isSum, final String weightName) { + return "Weighted" + (isSum ? "Sum" : "Avg") + "(" + weightName + ")"; + } } diff --git a/engine/table/src/test/java/io/deephaven/engine/table/impl/QueryTableAggregationTest.java b/engine/table/src/test/java/io/deephaven/engine/table/impl/QueryTableAggregationTest.java index 6374a9c7d1f..71d264f75d6 100644 --- a/engine/table/src/test/java/io/deephaven/engine/table/impl/QueryTableAggregationTest.java +++ b/engine/table/src/test/java/io/deephaven/engine/table/impl/QueryTableAggregationTest.java @@ -432,9 +432,7 @@ protected Table e() { @Test public void testStaticBy() { - Table table = newTable(0, new HashMap<>()); - TestCase.assertEquals(0, table.groupBy().size()); - TestCase.assertEquals(0, table.groupBy().getColumns().length); + Table table = newTable(intCol("V")); try { table.groupBy("i"); TestCase.fail("Previous statement should have thrown an exception"); @@ -442,12 +440,12 @@ public void testStaticBy() { TestCase.assertEquals("Invalid column name \"i\": \"i\" is a reserved keyword", e.getMessage()); } TestCase.assertEquals(0, table.groupBy("j=i").size()); - TestCase.assertEquals(1, table.groupBy("j=i").getColumns().length); + TestCase.assertEquals(2, table.groupBy("j=i").getColumns().length); TestCase.assertEquals(int.class, table.groupBy("j=i").getColumnSource("j").getType()); - table = newTable(1, new HashMap<>()); + table = newTable(intCol("V", 100)); TestCase.assertEquals(1, table.groupBy("j=i").size()); - TestCase.assertEquals(1, table.groupBy("j=i").getColumns().length); + TestCase.assertEquals(2, table.groupBy("j=i").getColumns().length); TestCase.assertEquals(int.class, table.groupBy("j=i").getColumn("j").getType()); table = TstUtils.testRefreshingTable(RowSetFactory.fromRange(0, 2).toTracking(), @@ -804,7 +802,7 @@ public void testKeyColumnTypes() { final QueryTable table = getTable(size, random, initColumnInfos( new String[] {"Sym", "Date", "intCol", "doubleCol", "BooleanCol", "ByteCol", "CharCol", - "ShortCol", "FloatCol", "LongCol", "BigDecimalCol"}, + "ShortCol", "FloatCol", "LongCol", "BigDecimalCol", "NonKey"}, new SetGenerator<>("aa", "bb", "bc", "cc", "dd"), new UnsortedDateTimeLongGenerator(DateTimeUtils.convertDateTime("2018-10-15T09:30:00 NY"), DateTimeUtils.convertDateTime("2018-10-15T16:00:00 NY")), @@ -816,16 +814,18 @@ public void testKeyColumnTypes() { new ShortGenerator(), new FloatGenerator(), new LongGenerator(), - new BigDecimalGenerator())); - + new BigDecimalGenerator(), + new IntGenerator())); - final String[] columns = table.getColumnSourceMap().keySet().toArray(CollectionUtil.ZERO_LENGTH_STRING_ARRAY); + final Set keyColumnSet = new LinkedHashSet<>(table.getColumnSourceMap().keySet()); + keyColumnSet.remove("NonKey"); + final String[] keyColumns = keyColumnSet.toArray(CollectionUtil.ZERO_LENGTH_STRING_ARRAY); table.lastBy("Date", "Sym"); // noinspection MismatchedQueryAndUpdateOfCollection final List tables = new ArrayList<>(); - powerSet(columns, (String[] cols) -> tables.add(table.lastBy(cols))); + powerSet(keyColumns, (String[] cols) -> tables.add(table.lastBy(cols))); } @Test @@ -3525,40 +3525,6 @@ public void testIds6332() { TestCase.assertEquals(BigInteger.valueOf(100), percentile.getColumn("Value").get(0)); } - @Test - public void testIds6593() { - final Table[][] resultSets = new Table[2][]; - final boolean substitutionWasEnabled = ChunkedOperatorAggregationHelper.KEY_ONLY_SUBSTITUTION_ENABLED; - try { - for (final boolean substituteForThisIteration : new boolean[] {false, true}) { - ChunkedOperatorAggregationHelper.KEY_ONLY_SUBSTITUTION_ENABLED = substituteForThisIteration; - final BaseTable source = (BaseTable) emptyTable(100).updateView("A=i%10"); - source.getRowSet().writableCast().removeRange(50, 100); - source.setRefreshing(true); - - resultSets[substituteForThisIteration ? 1 : 0] = new Table[] { - source.groupBy("A"), - source.firstBy("A"), - source.lastBy("A"), - source.minBy("A"), - source.maxBy("A"), - source.varBy("A") - }; - - UpdateGraphProcessor.DEFAULT.runWithinUnitTestCycle(() -> { - source.getRowSet().writableCast().insertRange(50, 100); - source.notifyListeners(new TableUpdateImpl(ir(50, 100), i(), i(), RowSetShiftData.EMPTY, - ModifiedColumnSet.EMPTY)); - }); - } - } finally { - ChunkedOperatorAggregationHelper.KEY_ONLY_SUBSTITUTION_ENABLED = substitutionWasEnabled; - } - for (int ti = 0; ti < resultSets[0].length; ++ti) { - assertTableEquals(resultSets[0][ti], resultSets[1][ti]); - } - } - @Test public void testIds7553() { final Table result = emptyTable(100).updateView("K=ii%10", "V=ii/10").groupBy("K").ungroup(); diff --git a/engine/table/src/test/java/io/deephaven/engine/table/impl/QueryTableJoinTest.java b/engine/table/src/test/java/io/deephaven/engine/table/impl/QueryTableJoinTest.java index 612e20ea9d4..87dbc210cb6 100644 --- a/engine/table/src/test/java/io/deephaven/engine/table/impl/QueryTableJoinTest.java +++ b/engine/table/src/test/java/io/deephaven/engine/table/impl/QueryTableJoinTest.java @@ -956,19 +956,6 @@ public void testNaturalJoinWithGroupBy() { assertEquals(1, uValues[1].size()); assertNull(vValues[2]); - pairMatch = table2.naturalJoin(table1.groupBy("String"), "String"); - - assertEquals(2, pairMatch.size()); - assertEquals(3, pairMatch.getColumns().length); - assertEquals("String", pairMatch.getColumns()[0].getName()); - assertEquals("v", pairMatch.getColumns()[1].getName()); - assertEquals("u", pairMatch.getColumns()[2].getName()); - assertEquals(String.class, pairMatch.getColumns()[0].getType()); - assertEquals(int.class, pairMatch.getColumns()[1].getType()); - assertEquals(asList("c", "e"), asList((Object[]) pairMatch.getColumns()[0].getDirect())); - assertEquals(1, pairMatch.getColumn("v").getInt(0)); - assertEquals(2, pairMatch.getColumn("v").getInt(1)); - pairMatch = table1.naturalJoin(table2.groupBy("String"), "String=String"); assertEquals(3, pairMatch.size()); assertEquals(3, pairMatch.getColumns().length); @@ -984,19 +971,6 @@ public void testNaturalJoinWithGroupBy() { assertEquals(1, vValues[1].size()); assertNull(vValues[2]); - pairMatch = table2.naturalJoin(table1.groupBy("String"), "String=String"); - - assertEquals(2, pairMatch.size()); - assertEquals(3, pairMatch.getColumns().length); - assertEquals("String", pairMatch.getColumns()[0].getName()); - assertEquals("v", pairMatch.getColumns()[1].getName()); - assertEquals(String.class, pairMatch.getColumns()[0].getType()); - assertEquals(int.class, pairMatch.getColumns()[1].getType()); - assertEquals(asList("c", "e"), asList((Object[]) pairMatch.getColumns()[0].getDirect())); - assertEquals(1, pairMatch.getColumn("v").getInt(0)); - assertEquals(2, pairMatch.getColumn("v").getInt(1)); - - table1 = TableTools.newTable( c("String1", "c", "e", "g")); @@ -1046,21 +1020,5 @@ public void testNaturalJoinWithGroupBy() { assertEquals(1, vValues[0].size()); assertEquals(1, vValues[1].size()); assertNull(vValues[2]); - - pairMatch = table2.naturalJoin(table1.groupBy("String1"), "String2=String1"); - - assertEquals(2, pairMatch.size()); - assertEquals(3, pairMatch.getColumns().length); - assertEquals("String2", pairMatch.getColumns()[0].getName()); - assertEquals("v", pairMatch.getColumns()[1].getName()); - assertEquals("String1", pairMatch.getColumns()[2].getName()); - assertEquals(String.class, pairMatch.getColumn("String2").getType()); - assertEquals(String.class, pairMatch.getColumn("String1").getType()); - assertEquals(int.class, pairMatch.getColumn("v").getType()); - assertEquals(asList("c", "e"), asList((Object[]) pairMatch.getColumns()[0].getDirect())); - assertEquals(1, pairMatch.getColumn("v").getInt(0)); - assertEquals(2, pairMatch.getColumn("v").getInt(1)); - assertEquals(asList("c", "e"), asList((String[]) pairMatch.getColumn("String1").getDirect())); - } }