From e60cb544a60d236ae87ccd94048951ee46f1e58d Mon Sep 17 00:00:00 2001 From: Devin Smith Date: Wed, 15 Nov 2023 06:47:25 -0800 Subject: [PATCH] Add TableDefinition column name helpers (#4813) Adds some helper methods for on `TableDefinition#checkHasColumn`, `TableDefinition#checkHasColumns`, and `TableDefinition#getColumnNameSet`. Additionally, fixes up call sites that were (ab)using `Table#getColumnSourceMap` to simply get the keySet. This invokes a potentially extraneous Table#coalesce which can be avoided in these cases. In support of common scaffolding so #4771 won't need to call `Table#getColumnSource` for validation purposes. --- .../plot/util/ArgumentValidations.java | 2 +- .../engine/table/TableDefinition.java | 28 ++++ .../table/impl/NoSuchColumnException.java | 121 ++++++++++++++++++ .../benchmark/engine/SortBenchmark.java | 3 +- .../engine/table/impl/BaseTable.java | 15 +-- .../engine/table/impl/BucketingContext.java | 8 +- .../engine/table/impl/CrossJoinHelper.java | 2 +- .../table/impl/NoSuchColumnException.java | 33 ----- .../engine/table/impl/QueryTable.java | 18 +-- .../engine/table/impl/RedefinableTable.java | 9 +- .../engine/table/impl/TableDefaults.java | 2 +- .../table/impl/TableUpdateValidator.java | 6 +- .../by/ChunkedOperatorAggregationHelper.java | 47 +++---- .../impl/by/PartitionByChunkedOperator.java | 6 +- .../hierarchical/HierarchicalTableImpl.java | 9 +- .../impl/hierarchical/RollupTableImpl.java | 2 +- .../table/impl/remote/ConstructSnapshot.java | 7 +- .../select/MultiSourceFunctionalColumn.java | 17 +-- .../snapshot/SnapshotInternalListener.java | 2 +- .../engine/table/impl/updateby/UpdateBy.java | 5 +- .../table/impl/util/DynamicTableWriter.java | 2 +- .../util/KeyedArrayBackedMutableTable.java | 2 +- .../deephaven/engine/util/OuterJoinTools.java | 4 +- .../io/deephaven/engine/util/TableTools.java | 2 +- .../engine/util/TableToolsMergeHelper.java | 8 +- .../deephaven/engine/util/TickSuppressor.java | 2 +- .../engine/util/TotalsTableBuilder.java | 14 +- .../io/deephaven/engine/util/WindowCheck.java | 2 +- .../table/impl/MultiColumnSortTest.java | 2 +- .../table/impl/QueryTableAggregationTest.java | 8 +- .../engine/table/impl/QueryTableTest.java | 12 +- .../table/impl/SelectOverheadLimiter.java | 4 +- .../engine/table/impl/TestAggBy.java | 2 +- .../engine/table/impl/TestMoveColumns.java | 8 +- .../engine/table/impl/TestTotalsTable.java | 9 +- .../table/impl/indexer/TestRowSetIndexer.java | 4 +- .../deephaven/engine/testutil/TstUtils.java | 13 +- .../locations/TableBackedColumnLocation.java | 2 +- .../jdbc/JdbcToTableAdapterTest.java | 12 +- .../table/ParquetTableReadWriteTest.java | 9 +- .../table/ops/SelectDistinctGrpcImpl.java | 10 +- 41 files changed, 282 insertions(+), 191 deletions(-) create mode 100644 engine/api/src/main/java/io/deephaven/engine/table/impl/NoSuchColumnException.java delete mode 100644 engine/table/src/main/java/io/deephaven/engine/table/impl/NoSuchColumnException.java diff --git a/Plot/src/main/java/io/deephaven/plot/util/ArgumentValidations.java b/Plot/src/main/java/io/deephaven/plot/util/ArgumentValidations.java index f78cbeee45f..2d1a4dbf9fa 100644 --- a/Plot/src/main/java/io/deephaven/plot/util/ArgumentValidations.java +++ b/Plot/src/main/java/io/deephaven/plot/util/ArgumentValidations.java @@ -841,7 +841,7 @@ public static void assertColumnsInTable(final Table t, final PlotInfo plotInfo, assertNotNull(t, "t", plotInfo); assertNotNull(cols, "cols", plotInfo); for (String c : cols) { - if (!t.getColumnSourceMap().containsKey(c)) { + if (!t.hasColumns(c)) { throw new PlotIllegalArgumentException("Column " + c + " could not be found in table.", plotInfo); } } diff --git a/engine/api/src/main/java/io/deephaven/engine/table/TableDefinition.java b/engine/api/src/main/java/io/deephaven/engine/table/TableDefinition.java index 584c09743a1..66aa6b003f7 100644 --- a/engine/api/src/main/java/io/deephaven/engine/table/TableDefinition.java +++ b/engine/api/src/main/java/io/deephaven/engine/table/TableDefinition.java @@ -9,6 +9,7 @@ import io.deephaven.base.log.LogOutput; import io.deephaven.base.log.LogOutputAppendable; import io.deephaven.base.verify.Assert; +import io.deephaven.engine.table.impl.NoSuchColumnException; import io.deephaven.io.log.impl.LogOutputStringImpl; import io.deephaven.qst.column.header.ColumnHeader; import org.jetbrains.annotations.NotNull; @@ -204,6 +205,13 @@ public Map> getColumnNameMap() { .toMap(ColumnDefinition::getName, Function.identity(), Assert::neverInvoked, LinkedHashMap::new))); } + /** + * @return An unmodifiable set of column names + */ + public Set getColumnNameSet() { + return getColumnNameMap().keySet(); + } + /** * @return A list of {@link ColumnDefinition column definitions} for all * {@link ColumnDefinition.ColumnType#Partitioning partitioning} columns in the same relative order as the @@ -295,6 +303,26 @@ public String getColumnNamesAsString() { return getColumnStream().map(ColumnDefinition::getName).collect(Collectors.joining(",")); } + /** + * Check this definition to ensure that {@code columnName} is present. + * + * @param columnName The column name to check + * @throws NoSuchColumnException If {@code columnName} is missing + */ + public final void checkHasColumn(@NotNull String columnName) { + NoSuchColumnException.throwIf(getColumnNameSet(), columnName); + } + + /** + * Check this definition to ensure that all {@code columns} are present. + * + * @param columns The column names to check + * @throws NoSuchColumnException If any {@code columns} were missing + */ + public final void checkHasColumns(@NotNull Collection columns) { + NoSuchColumnException.throwIf(getColumnNameSet(), columns); + } + /** * Tests mutual-compatibility of {@code this} and {@code other}. To be mutually compatible, they must have the same * number of columns, each matched up with {@link ColumnDefinition#isCompatible}. As such, this method has an diff --git a/engine/api/src/main/java/io/deephaven/engine/table/impl/NoSuchColumnException.java b/engine/api/src/main/java/io/deephaven/engine/table/impl/NoSuchColumnException.java new file mode 100644 index 00000000000..2b867f191b4 --- /dev/null +++ b/engine/api/src/main/java/io/deephaven/engine/table/impl/NoSuchColumnException.java @@ -0,0 +1,121 @@ +/** + * Copyright (c) 2016-2022 Deephaven Data Labs and Patent Pending + */ +package io.deephaven.engine.table.impl; + +import java.util.Collection; +import java.util.Collections; +import java.util.List; +import java.util.Set; +import java.util.function.Predicate; +import java.util.stream.Collectors; + +/** + * Exception thrown when a column is not found. + */ +public class NoSuchColumnException extends IllegalArgumentException { + + public static final String DELIMITER = ", "; + + public static final String DEFAULT_FORMAT_STR = "Unknown column names [%s], available column names are [%s]"; + + public enum Type { + MISSING, AVAILABLE, REQUESTED + } + + /** + * Equivalent to {@code throwIf(available, Collections.singleton(requested))}. + * + * @param available the available columns + * @param requested the requested columns + * @see #throwIf(Set, Collection) + */ + public static void throwIf(Set available, String requested) { + throwIf(available, Collections.singleton(requested)); + } + + /** + * Equivalent to {@code throwIf(available, requested, DEFAULT_FORMAT_STR, Type.MISSING, Type.AVAILABLE)} where + * {@code DEFAULT_FORMAT_STR} is {@value DEFAULT_FORMAT_STR}. + * + * @param available the available columns + * @param requested the requested columns + * @see #throwIf(Set, Collection, String, Type...) + */ + public static void throwIf(Set available, Collection requested) { + throwIf(available, requested, DEFAULT_FORMAT_STR, Type.MISSING, Type.AVAILABLE); + } + + /** + * Throws a {@link NoSuchColumnException} if any name from {@code requested} is not in {@code available}. The + * message will be constructed by {@link String#join(CharSequence, Iterable) joining} the respective collection with + * {@value DELIMITER} and presenting them to {@link String#format(String, Object...) format} in {@code types} order. + * + * @param available the available columns + * @param requested the requested columns + * @param formatStr the format string + * @param types the collection types order for formatting + */ + public static void throwIf(Set available, Collection requested, String formatStr, Type... types) { + final List missing = requested + .stream() + .filter(Predicate.not(available::contains)) + .collect(Collectors.toList()); + if (!missing.isEmpty()) { + final Object[] formatArgs = new Object[types.length]; + for (int i = 0; i < types.length; ++i) { + switch (types[i]) { + case MISSING: + formatArgs[i] = String.join(DELIMITER, missing); + break; + case AVAILABLE: + formatArgs[i] = String.join(DELIMITER, available); + break; + case REQUESTED: + formatArgs[i] = String.join(DELIMITER, requested); + break; + default: + throw new IllegalStateException("Unexpected case " + types[i]); + } + } + throw new NoSuchColumnException(String.format(formatStr, formatArgs)); + } + } + + /** + * Thrown when an operation can not find a required column(s). + * + *

+ * Callers may prefer to use {@link #throwIf(Set, Collection, String, Type...)} when applicable. + * + * @param message the message + */ + public NoSuchColumnException(String message) { + super(message); + } + + /** + * Thrown when an operation can not find a required column(s). + * + *

+ * Callers may prefer to use {@link #throwIf(Set, Collection)} when applicable. + * + * @param presentColumns the column names present in the table + * @param missingColumns the request column names that were not found + */ + public NoSuchColumnException(Collection presentColumns, Collection missingColumns) { + this(String.format(DEFAULT_FORMAT_STR, + String.join(DELIMITER, missingColumns), + String.join(DELIMITER, presentColumns))); + } + + /** + * Thrown when an operation can not find a required column. + * + * @param presentColumns the column names present in the table + * @param missingColumn the request column name that was not found + */ + public NoSuchColumnException(Collection presentColumns, String missingColumn) { + this(presentColumns, Collections.singleton(missingColumn)); + } +} diff --git a/engine/benchmark/src/benchmark/java/io/deephaven/benchmark/engine/SortBenchmark.java b/engine/benchmark/src/benchmark/java/io/deephaven/benchmark/engine/SortBenchmark.java index 8f2d9ed9927..d6a94b36e9d 100644 --- a/engine/benchmark/src/benchmark/java/io/deephaven/benchmark/engine/SortBenchmark.java +++ b/engine/benchmark/src/benchmark/java/io/deephaven/benchmark/engine/SortBenchmark.java @@ -138,7 +138,8 @@ public void setupEnv(BenchmarkParams params) { mcsWithSortColumn = inputTable.newModifiedColumnSet(sortCol); MutableInt ci = new MutableInt(); final String[] sortColumns = new String[inputTable.numColumns() - 1]; - inputTable.getColumnSourceMap().keySet().forEach(columnName -> { + + inputTable.getDefinition().getColumnNameSet().forEach(columnName -> { if (!columnName.equals(sortCol)) { sortColumns[ci.intValue()] = columnName; ci.increment(); diff --git a/engine/table/src/main/java/io/deephaven/engine/table/impl/BaseTable.java b/engine/table/src/main/java/io/deephaven/engine/table/impl/BaseTable.java index 7c16414c7d5..3331d99ed2d 100644 --- a/engine/table/src/main/java/io/deephaven/engine/table/impl/BaseTable.java +++ b/engine/table/src/main/java/io/deephaven/engine/table/impl/BaseTable.java @@ -1020,13 +1020,8 @@ private String formatKeyColumns(String... columns) { } @Override - public void checkAvailableColumns(@NotNull final Collection columns) { - final Map> sourceMap = getColumnSourceMap(); - final String[] missingColumns = - columns.stream().filter(col -> !sourceMap.containsKey(col)).toArray(String[]::new); - if (missingColumns.length > 0) { - throw new NoSuchColumnException(sourceMap.keySet(), Arrays.asList(missingColumns)); - } + public final void checkAvailableColumns(@NotNull final Collection columns) { + getDefinition().checkHasColumns(columns); } public void copySortableColumns( @@ -1063,7 +1058,7 @@ void copySortableColumns(BaseTable destination, MatchPair[] renamedColumns) { // Process the original set of sortable columns, adding them to the new set if one of the below // 1) The column exists in the new table and was not renamed in any way but the Identity (C1 = C1) // 2) The column does not exist in the new table, but was renamed to another (C2 = C1) - final Set resultColumnNames = destination.getDefinition().getColumnNameMap().keySet(); + final Set resultColumnNames = destination.getDefinition().getColumnNameSet(); for (final String columnName : currentSortableColumns) { // Only add it to the set of sortable columns if it hasn't changed in an unknown way final String maybeRenamedColumn = columnMapping.get(columnName); @@ -1109,9 +1104,9 @@ void copySortableColumns(BaseTable destination, SelectColumn[] selectCols) { } // Now go through the other columns in the table and add them if they were unchanged - final Map> sourceMap = destination.getColumnSourceMap(); + final Set destKeys = destination.getDefinition().getColumnNameSet(); for (String col : currentSortableSet) { - if (sourceMap.containsKey(col)) { + if (destKeys.contains(col)) { newSortableSet.add(col); } } diff --git a/engine/table/src/main/java/io/deephaven/engine/table/impl/BucketingContext.java b/engine/table/src/main/java/io/deephaven/engine/table/impl/BucketingContext.java index c89b48e4e34..46881351573 100644 --- a/engine/table/src/main/java/io/deephaven/engine/table/impl/BucketingContext.java +++ b/engine/table/src/main/java/io/deephaven/engine/table/impl/BucketingContext.java @@ -20,6 +20,7 @@ import java.time.Instant; import java.util.Arrays; import java.util.List; +import java.util.Set; import java.util.stream.Collectors; import static io.deephaven.engine.table.impl.MatchPair.matchString; @@ -41,8 +42,11 @@ class BucketingContext implements SafeCloseable { BucketingContext(final String listenerPrefix, final QueryTable leftTable, final QueryTable rightTable, MatchPair[] columnsToMatch, MatchPair[] columnsToAdd, JoinControl control) { - final List conflicts = Arrays.stream(columnsToAdd).map(MatchPair::leftColumn) - .filter(cn -> leftTable.getColumnSourceMap().containsKey(cn)).collect(Collectors.toList()); + final Set leftKeys = leftTable.getDefinition().getColumnNameSet(); + final List conflicts = Arrays.stream(columnsToAdd) + .map(MatchPair::leftColumn) + .filter(leftKeys::contains) + .collect(Collectors.toList()); if (!conflicts.isEmpty()) { throw new RuntimeException("Conflicting column names " + conflicts); } diff --git a/engine/table/src/main/java/io/deephaven/engine/table/impl/CrossJoinHelper.java b/engine/table/src/main/java/io/deephaven/engine/table/impl/CrossJoinHelper.java index 2abd3bc1a45..c6fb7284e2f 100644 --- a/engine/table/src/main/java/io/deephaven/engine/table/impl/CrossJoinHelper.java +++ b/engine/table/src/main/java/io/deephaven/engine/table/impl/CrossJoinHelper.java @@ -195,7 +195,7 @@ private static QueryTable internalJoin( jsm.startTrackingPrevValues(); final ModifiedColumnSet.Transformer leftTransformer = leftTable.newModifiedColumnSetTransformer( resultTable, - leftTable.getColumnSourceMap().keySet().toArray(CollectionUtil.ZERO_LENGTH_STRING_ARRAY)); + leftTable.getDefinition().getColumnNamesArray()); leftTable.addUpdateListener(new BaseTable.ListenerImpl(bucketingContext.listenerDescription, leftTable, resultTable) { diff --git a/engine/table/src/main/java/io/deephaven/engine/table/impl/NoSuchColumnException.java b/engine/table/src/main/java/io/deephaven/engine/table/impl/NoSuchColumnException.java deleted file mode 100644 index b03c2236206..00000000000 --- a/engine/table/src/main/java/io/deephaven/engine/table/impl/NoSuchColumnException.java +++ /dev/null @@ -1,33 +0,0 @@ -/** - * Copyright (c) 2016-2022 Deephaven Data Labs and Patent Pending - */ -package io.deephaven.engine.table.impl; - -import java.util.Collection; -import java.util.Collections; - -/** - * Exception thrown when a column is not found. - */ -public class NoSuchColumnException extends IllegalArgumentException { - /** - * Thrown when an operation can not find a required column. - * - * @param presentColumns the column names present in the table - * @param requestedColumns the request column names that were not found - */ - public NoSuchColumnException(Collection presentColumns, Collection requestedColumns) { - super("Unknown column names [" + String.join(",", requestedColumns) - + "], available column names are [" + String.join(",", presentColumns) + "]"); - } - - /** - * Thrown when an operation can not find a required column. - * - * @param presentColumns the column names present in the table - * @param requestedColumn the request column name that was not found - */ - public NoSuchColumnException(Collection presentColumns, String requestedColumn) { - this(presentColumns, Collections.singleton(requestedColumn)); - } -} diff --git a/engine/table/src/main/java/io/deephaven/engine/table/impl/QueryTable.java b/engine/table/src/main/java/io/deephaven/engine/table/impl/QueryTable.java index 1b468be1e76..60189aba160 100644 --- a/engine/table/src/main/java/io/deephaven/engine/table/impl/QueryTable.java +++ b/engine/table/src/main/java/io/deephaven/engine/table/impl/QueryTable.java @@ -355,7 +355,7 @@ public long size() { public ColumnSource getColumnSource(String sourceName) { final ColumnSource columnSource = columns.get(sourceName); if (columnSource == null) { - throw new NoSuchColumnException(columns.keySet(), Collections.singletonList(sourceName)); + throw new NoSuchColumnException(columns.keySet(), sourceName); } // noinspection unchecked return (ColumnSource) columnSource; @@ -1772,14 +1772,7 @@ public Table dropColumns(String... columnNames) { return memoizeResult(MemoizedOperationKey.dropColumns(columnNames), () -> QueryPerformanceRecorder .withNugget("dropColumns(" + Arrays.toString(columnNames) + ")", sizeForInstrumentation(), () -> { final Mutable result = new MutableObject<>(); - - final Set existingColumns = new HashSet<>(definition.getColumnNames()); - final Set columnNamesToDrop = new HashSet<>(Arrays.asList(columnNames)); - if (!existingColumns.containsAll(columnNamesToDrop)) { - columnNamesToDrop.removeAll(existingColumns); - throw new RuntimeException("Unknown columns: " + columnNamesToDrop - + ", available columns = " + getColumnSourceMap().keySet()); - } + definition.checkHasColumns(Arrays.asList(columnNames)); final Map> newColumns = new LinkedHashMap<>(columns); for (String columnName : columnNames) { newColumns.remove(columnName); @@ -1794,14 +1787,13 @@ public Table dropColumns(String... columnNames) { copyAttributes(resultTable, CopyAttributeOperation.DropColumns); copySortableColumns(resultTable, - resultTable.getDefinition().getColumnNameMap()::containsKey); + resultTable.getDefinition().getColumnNameSet()::contains); maybeCopyColumnDescriptions(resultTable); if (snapshotControl != null) { final ModifiedColumnSet.Transformer mcsTransformer = newModifiedColumnSetTransformer(resultTable, - resultTable.getColumnSourceMap().keySet() - .toArray(CollectionUtil.ZERO_LENGTH_STRING_ARRAY)); + resultTable.getDefinition().getColumnNamesArray()); final ListenerImpl listener = new ListenerImpl( "dropColumns(" + Arrays.deepToString(columnNames) + ')', this, resultTable) { @Override @@ -2400,7 +2392,7 @@ private Table snapshotIncrementalInternal(final Table base, final boolean doInit // Use the given columns (if specified); otherwise an empty array means all of my columns final String[] useStampColumns = stampColumns.length == 0 - ? getColumnSourceMap().keySet().toArray(CollectionUtil.ZERO_LENGTH_STRING_ARRAY) + ? definition.getColumnNamesArray() : stampColumns; final Map> triggerColumns = new LinkedHashMap<>(); diff --git a/engine/table/src/main/java/io/deephaven/engine/table/impl/RedefinableTable.java b/engine/table/src/main/java/io/deephaven/engine/table/impl/RedefinableTable.java index 2ca37e304ef..ed787d1baab 100644 --- a/engine/table/src/main/java/io/deephaven/engine/table/impl/RedefinableTable.java +++ b/engine/table/src/main/java/io/deephaven/engine/table/impl/RedefinableTable.java @@ -84,15 +84,8 @@ public Table dropColumns(final String... columnNames) { if (columnNames == null || columnNames.length == 0) { return this; } - final Set columnNamesToDrop = new HashSet<>(Arrays.asList(columnNames)); - final Set existingColumns = new HashSet<>(definition.getColumnNames()); - if (!existingColumns.containsAll(columnNamesToDrop)) { - columnNamesToDrop.removeAll(existingColumns); - throw new RuntimeException("Unknown columns: " + columnNamesToDrop.toString() + ", available columns = " - + getColumnSourceMap().keySet()); - } - + definition.checkHasColumns(columnNamesToDrop); List> resultColumns = new ArrayList<>(); for (ColumnDefinition cDef : definition.getColumns()) { if (!columnNamesToDrop.contains(cDef.getName())) { diff --git a/engine/table/src/main/java/io/deephaven/engine/table/impl/TableDefaults.java b/engine/table/src/main/java/io/deephaven/engine/table/impl/TableDefaults.java index ee6dad28b78..836ca71260e 100644 --- a/engine/table/src/main/java/io/deephaven/engine/table/impl/TableDefaults.java +++ b/engine/table/src/main/java/io/deephaven/engine/table/impl/TableDefaults.java @@ -98,7 +98,7 @@ default boolean hasColumns(Collection columnNames) { if (columnNames == null) { throw new IllegalArgumentException("columnNames cannot be null!"); } - return getDefinition().getColumnNameMap().keySet().containsAll(columnNames); + return getDefinition().getColumnNameSet().containsAll(columnNames); } @Override diff --git a/engine/table/src/main/java/io/deephaven/engine/table/impl/TableUpdateValidator.java b/engine/table/src/main/java/io/deephaven/engine/table/impl/TableUpdateValidator.java index 5f2190f2b29..9ec9ddf94a5 100644 --- a/engine/table/src/main/java/io/deephaven/engine/table/impl/TableUpdateValidator.java +++ b/engine/table/src/main/java/io/deephaven/engine/table/impl/TableUpdateValidator.java @@ -61,11 +61,13 @@ private TableUpdateValidator(final String description, final QueryTable tableToV this.description = description == null ? tableToValidate.getDescription() : description; this.tableToValidate = tableToValidate; this.validationMCS = tableToValidate.newModifiedColumnSet( - tableToValidate.getColumnSourceMap().keySet().toArray(CollectionUtil.ZERO_LENGTH_STRING_ARRAY)); + tableToValidate.getDefinition().getColumnNamesArray()); Assert.neq(validationMCS, "validationMCS", ModifiedColumnSet.ALL, "ModifiedColumnSet.ALL"); Assert.neq(validationMCS, "validationMCS", ModifiedColumnSet.EMPTY, "ModifiedColumnSet.EMPTY"); - columnInfos = tableToValidate.getColumnSourceMap().keySet().stream() + columnInfos = tableToValidate.getDefinition() + .getColumnStream() + .map(ColumnDefinition::getName) .map((name) -> new ColumnInfo(tableToValidate, name)) .toArray(ColumnInfo[]::new); } 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 8ba1426b302..2d6c8f8dde6 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 @@ -21,6 +21,7 @@ import io.deephaven.engine.rowset.chunkattributes.RowKeys; import io.deephaven.engine.table.*; import io.deephaven.engine.table.impl.*; +import io.deephaven.engine.table.impl.NoSuchColumnException.Type; import io.deephaven.engine.table.impl.by.typed.TypedHasherFactory; import io.deephaven.engine.table.impl.indexer.RowSetIndexer; import io.deephaven.engine.table.impl.remote.ConstructSnapshot; @@ -47,7 +48,6 @@ import java.util.*; import java.util.function.LongFunction; -import java.util.function.Predicate; import java.util.function.Supplier; import java.util.function.UnaryOperator; @@ -78,6 +78,18 @@ public static QueryTable aggregation( aggregationContextFactory, input, preserveEmpty, initialKeys, groupByColumns); } + private static void checkGroupByColumns(String context, TableDefinition tableDefinition, String[] keyNames) { + NoSuchColumnException.throwIf( + tableDefinition.getColumnNameSet(), + Arrays.asList(keyNames), + String.format( + "aggregation: not all group-by columns [%%s] are present in %s with columns [%%s]. Missing columns: [%%s]", + context), + Type.REQUESTED, + Type.AVAILABLE, + Type.MISSING); + } + @VisibleForTesting public static QueryTable aggregation( @NotNull final AggregationControl control, @@ -87,41 +99,22 @@ public static QueryTable aggregation( @Nullable final Table initialKeys, @NotNull final Collection groupByColumns) { final String[] keyNames = groupByColumns.stream().map(ColumnName::name).toArray(String[]::new); - if (!input.hasColumns(keyNames)) { - final Set colNames = input.getColumnSourceMap().keySet(); - final String[] missingColumns = Arrays.stream(keyNames) - .filter(Predicate.not(colNames::contains)) - .toArray(String[]::new);; - - throw new IllegalArgumentException("aggregation: not all group-by columns " + Arrays.toString(keyNames) - + " are present in input table with columns " - + Arrays.toString(input.getDefinition().getColumnNamesArray()) + ". Missing columns: " - + Arrays.toString(missingColumns)); - } + checkGroupByColumns("input table", input.getDefinition(), keyNames); if (initialKeys != null) { if (keyNames.length == 0) { throw new IllegalArgumentException( "aggregation: initial groups must not be specified if no group-by columns are specified"); } - if (!initialKeys.hasColumns(keyNames)) { - final Set colNames = input.getColumnSourceMap().keySet(); - final String[] missingColumns = Arrays.stream(keyNames) - .filter(Predicate.not(colNames::contains)) - .toArray(String[]::new);; - - throw new IllegalArgumentException("aggregation: not all group-by columns " + Arrays.toString(keyNames) - + " are present in initial groups table with columns " - + Arrays.toString(initialKeys.getDefinition().getColumnNamesArray()) + ". Missing columns: " - + Arrays.toString(missingColumns)); - } + checkGroupByColumns("initial groups", initialKeys.getDefinition(), keyNames); for (final String keyName : keyNames) { final ColumnDefinition inputDef = input.getDefinition().getColumn(keyName); final ColumnDefinition initialKeysDef = initialKeys.getDefinition().getColumn(keyName); if (!inputDef.isCompatible(initialKeysDef)) { - throw new IllegalArgumentException( - "aggregation: column definition mismatch between input table and initial groups table for " - + keyName + " input has " + inputDef.describeForCompatibility() - + ", initial groups has " + initialKeysDef.describeForCompatibility()); + throw new IllegalArgumentException(String.format( + "aggregation: column definition mismatch between input table and initial groups table for %s; input has %s, initial groups has %s", + keyName, + inputDef.describeForCompatibility(), + initialKeysDef.describeForCompatibility())); } } } diff --git a/engine/table/src/main/java/io/deephaven/engine/table/impl/by/PartitionByChunkedOperator.java b/engine/table/src/main/java/io/deephaven/engine/table/impl/by/PartitionByChunkedOperator.java index d053c54ee87..ad0c4c2fd74 100644 --- a/engine/table/src/main/java/io/deephaven/engine/table/impl/by/PartitionByChunkedOperator.java +++ b/engine/table/src/main/java/io/deephaven/engine/table/impl/by/PartitionByChunkedOperator.java @@ -41,6 +41,7 @@ import org.jetbrains.annotations.Nullable; import java.util.*; +import java.util.function.Predicate; import java.util.function.UnaryOperator; import java.util.stream.Collectors; @@ -167,11 +168,10 @@ public interface AttributeCopier { shiftDataBuilders = new ObjectArraySource<>(RowSetShiftData.SmartCoalescingBuilder.class); final Set keyColumnNameSet = Arrays.stream(keyColumnNames).collect(Collectors.toSet()); - final Set unadjustedParentColumnNameSet = - new LinkedHashSet<>(unadjustedParentTable.getDefinition().getColumnNames()); + final Set unadjustedParentColumnNameSet = unadjustedParentTable.getDefinition().getColumnNameSet(); final String[] retainedResultColumnNames = parentTable.getDefinition().getColumnStream() .map(ColumnDefinition::getName) - .filter(cn -> !keyColumnNameSet.contains(cn)) + .filter(Predicate.not(keyColumnNameSet::contains)) .filter(unadjustedParentColumnNameSet::contains) .toArray(String[]::new); final ModifiedColumnSet[] retainedResultModifiedColumnSets = Arrays.stream(retainedResultColumnNames) diff --git a/engine/table/src/main/java/io/deephaven/engine/table/impl/hierarchical/HierarchicalTableImpl.java b/engine/table/src/main/java/io/deephaven/engine/table/impl/hierarchical/HierarchicalTableImpl.java index 2f11ad45800..2d424a98460 100644 --- a/engine/table/src/main/java/io/deephaven/engine/table/impl/hierarchical/HierarchicalTableImpl.java +++ b/engine/table/src/main/java/io/deephaven/engine/table/impl/hierarchical/HierarchicalTableImpl.java @@ -136,13 +136,8 @@ IFACE_TYPE noopResult() { } @Override - protected void checkAvailableColumns(@NotNull final Collection columns) { - final Set availableColumns = root.getDefinition().getColumnNameMap().keySet(); - final List missingColumns = - columns.stream().filter(column -> !availableColumns.contains(column)).collect(Collectors.toList()); - if (!missingColumns.isEmpty()) { - throw new NoSuchColumnException(availableColumns, missingColumns); - } + protected final void checkAvailableColumns(@NotNull final Collection columns) { + root.getDefinition().checkHasColumns(columns); } /** diff --git a/engine/table/src/main/java/io/deephaven/engine/table/impl/hierarchical/RollupTableImpl.java b/engine/table/src/main/java/io/deephaven/engine/table/impl/hierarchical/RollupTableImpl.java index 1b4f04dfcd9..0dc6f9b7be0 100644 --- a/engine/table/src/main/java/io/deephaven/engine/table/impl/hierarchical/RollupTableImpl.java +++ b/engine/table/src/main/java/io/deephaven/engine/table/impl/hierarchical/RollupTableImpl.java @@ -465,7 +465,7 @@ public static RollupTable makeRollup( source.getAttributes(ak -> shouldCopyAttribute(ak, CopyAttributeOperation.Rollup)), source, aggregations, includeConstituents, groupByColumns, levelTables, levelRowLookups, levelNodeTableSources, null, null, null, null, null); - source.copySortableColumns(result, baseLevel.getDefinition().getColumnNameMap()::containsKey); + source.copySortableColumns(result, baseLevel.getDefinition().getColumnNameSet()::contains); result.setColumnDescriptions(AggregationDescriptions.of(aggregations)); return result; } diff --git a/engine/table/src/main/java/io/deephaven/engine/table/impl/remote/ConstructSnapshot.java b/engine/table/src/main/java/io/deephaven/engine/table/impl/remote/ConstructSnapshot.java index c5ddf344299..1f72098d501 100644 --- a/engine/table/src/main/java/io/deephaven/engine/table/impl/remote/ConstructSnapshot.java +++ b/engine/table/src/main/java/io/deephaven/engine/table/impl/remote/ConstructSnapshot.java @@ -1403,9 +1403,7 @@ private static boolean serializeAllTable( } LongSizedDataStructure.intSize("construct snapshot", snapshot.rowsIncluded.size()); - - final Map> sourceMap = table.getColumnSourceMap(); - final String[] columnSources = sourceMap.keySet().toArray(CollectionUtil.ZERO_LENGTH_STRING_ARRAY); + final String[] columnSources = table.getDefinition().getColumnNamesArray(); snapshot.dataColumns = new Object[columnSources.length]; try (final SharedContext sharedContext = @@ -1480,8 +1478,7 @@ private static boolean serializeAllTable( snapshot.rowsIncluded = snapshot.rowsAdded.copy(); } - final Map> sourceMap = table.getColumnSourceMap(); - final String[] columnSources = sourceMap.keySet().toArray(CollectionUtil.ZERO_LENGTH_STRING_ARRAY); + final String[] columnSources = table.getDefinition().getColumnNamesArray(); try (final SharedContext sharedContext = (columnSources.length > 1) ? SharedContext.makeSharedContext() : null) { diff --git a/engine/table/src/main/java/io/deephaven/engine/table/impl/select/MultiSourceFunctionalColumn.java b/engine/table/src/main/java/io/deephaven/engine/table/impl/select/MultiSourceFunctionalColumn.java index 880a841660e..29d77004a69 100644 --- a/engine/table/src/main/java/io/deephaven/engine/table/impl/select/MultiSourceFunctionalColumn.java +++ b/engine/table/src/main/java/io/deephaven/engine/table/impl/select/MultiSourceFunctionalColumn.java @@ -95,22 +95,7 @@ public List initInputs(TrackingRowSet rowSet, Map initDef(Map> columnDefinitionMap) { - final MutableObject> missingColumnsHolder = new MutableObject<>(); - sourceNames.forEach(name -> { - final ColumnDefinition sourceColumnDefinition = columnDefinitionMap.get(name); - if (sourceColumnDefinition == null) { - List missingColumnsList; - if ((missingColumnsList = missingColumnsHolder.getValue()) == null) { - missingColumnsHolder.setValue(missingColumnsList = new ArrayList<>()); - } - missingColumnsList.add(name); - } - }); - - if (missingColumnsHolder.getValue() != null) { - throw new NoSuchColumnException(columnDefinitionMap.keySet(), missingColumnsHolder.getValue()); - } - + NoSuchColumnException.throwIf(columnDefinitionMap.keySet(), sourceNames); return getColumns(); } diff --git a/engine/table/src/main/java/io/deephaven/engine/table/impl/snapshot/SnapshotInternalListener.java b/engine/table/src/main/java/io/deephaven/engine/table/impl/snapshot/SnapshotInternalListener.java index 614aa53d42c..a807965046f 100644 --- a/engine/table/src/main/java/io/deephaven/engine/table/impl/snapshot/SnapshotInternalListener.java +++ b/engine/table/src/main/java/io/deephaven/engine/table/impl/snapshot/SnapshotInternalListener.java @@ -33,7 +33,7 @@ public SnapshotInternalListener(QueryTable triggerTable, Map> resultTriggerColumns, Map> resultBaseColumns, TrackingWritableRowSet resultRowSet) { - super("snapshot " + result.getColumnSourceMap().keySet(), triggerTable, result); + super("snapshot " + result.getDefinition().getColumnNameSet(), triggerTable, result); this.triggerTable = triggerTable; this.result = result; this.lazySnapshot = lazySnapshot; diff --git a/engine/table/src/main/java/io/deephaven/engine/table/impl/updateby/UpdateBy.java b/engine/table/src/main/java/io/deephaven/engine/table/impl/updateby/UpdateBy.java index c5af29411f1..6cf58505ba4 100644 --- a/engine/table/src/main/java/io/deephaven/engine/table/impl/updateby/UpdateBy.java +++ b/engine/table/src/main/java/io/deephaven/engine/table/impl/updateby/UpdateBy.java @@ -47,6 +47,7 @@ import java.util.concurrent.atomic.AtomicIntegerArray; import java.util.concurrent.atomic.AtomicReferenceArray; import java.util.function.Consumer; +import java.util.stream.Collectors; import java.util.stream.IntStream; /** @@ -1188,7 +1189,7 @@ public static Table updateBy(@NotNull final QueryTable source, final Collection> windowSpecs = updateByOperatorFactory.getWindowOperatorSpecs(clauses); - if (windowSpecs.size() == 0) { + if (windowSpecs.isEmpty()) { throw new IllegalArgumentException("At least one operator must be specified"); } @@ -1198,7 +1199,7 @@ public static Table updateBy(@NotNull final QueryTable source, final MutableObject timestampColumnName = new MutableObject<>(null); // create an initial set of all source columns - final Set preservedColumnSet = new LinkedHashSet<>(source.getColumnSourceMap().keySet()); + final LinkedHashSet preservedColumnSet = new LinkedHashSet<>(source.getDefinition().getColumnNameSet()); final Set problems = new LinkedHashSet<>(); final Map> opResultSources = new LinkedHashMap<>(); diff --git a/engine/table/src/main/java/io/deephaven/engine/table/impl/util/DynamicTableWriter.java b/engine/table/src/main/java/io/deephaven/engine/table/impl/util/DynamicTableWriter.java index d7edcb0b118..3322f56bb88 100644 --- a/engine/table/src/main/java/io/deephaven/engine/table/impl/util/DynamicTableWriter.java +++ b/engine/table/src/main/java/io/deephaven/engine/table/impl/util/DynamicTableWriter.java @@ -778,7 +778,7 @@ private DynamicTableRow() { public PermissiveRowSetter getSetter(final String name) { final PermissiveRowSetter rowSetter = columnToSetter.get(name); if (rowSetter == null) { - if (table.getColumnSourceMap().containsKey(name)) { + if (table.hasColumns(name)) { throw new RuntimeException("Column has a constant value, can not get setter " + name); } else { throw new RuntimeException("Unknown column name " + name); diff --git a/engine/table/src/main/java/io/deephaven/engine/table/impl/util/KeyedArrayBackedMutableTable.java b/engine/table/src/main/java/io/deephaven/engine/table/impl/util/KeyedArrayBackedMutableTable.java index 5125422e47e..ad4221bbb90 100644 --- a/engine/table/src/main/java/io/deephaven/engine/table/impl/util/KeyedArrayBackedMutableTable.java +++ b/engine/table/src/main/java/io/deephaven/engine/table/impl/util/KeyedArrayBackedMutableTable.java @@ -131,7 +131,7 @@ private KeyedArrayBackedMutableTable(@NotNull TableDefinition definition, final } private void startTrackingPrev() { - getColumnSourceMap().values().forEach(ColumnSource::startTrackingPrevValues); + getColumnSources().forEach(ColumnSource::startTrackingPrevValues); } @Override diff --git a/engine/table/src/main/java/io/deephaven/engine/util/OuterJoinTools.java b/engine/table/src/main/java/io/deephaven/engine/util/OuterJoinTools.java index b4bee084930..b63c329a9d5 100644 --- a/engine/table/src/main/java/io/deephaven/engine/util/OuterJoinTools.java +++ b/engine/table/src/main/java/io/deephaven/engine/util/OuterJoinTools.java @@ -68,10 +68,10 @@ public static Table fullOuterJoin( // find a sentinel column name to use to identify right-side only rows int numAttempts = 0; String sentinelColumnName; - final Map> resultColumns = leftTable.getColumnSourceMap(); + final Set resultColumns = leftTable.getDefinition().getColumnNameSet(); do { sentinelColumnName = "__sentinel_" + (numAttempts++) + "__"; - } while (resultColumns.containsKey(sentinelColumnName)); + } while (resultColumns.contains(sentinelColumnName)); // only need match columns from the left; rename to right names and drop remaining to avoid name conflicts final List leftColumns = Streams.concat( diff --git a/engine/table/src/main/java/io/deephaven/engine/util/TableTools.java b/engine/table/src/main/java/io/deephaven/engine/util/TableTools.java index 1c9e9b666e9..b4b81a18ae5 100644 --- a/engine/table/src/main/java/io/deephaven/engine/util/TableTools.java +++ b/engine/table/src/main/java/io/deephaven/engine/util/TableTools.java @@ -1151,7 +1151,7 @@ public static byte[] computeFingerprint(Table source) throws IOException { final DataOutputStream osw = new DataOutputStream(new DigestOutputStream(new NullOutputStream(), md)); - for (final ColumnSource col : source.getColumnSourceMap().values()) { + for (final ColumnSource col : source.getColumnSources()) { processColumnForFingerprint(source.getRowSet(), col, osw); } diff --git a/engine/table/src/main/java/io/deephaven/engine/util/TableToolsMergeHelper.java b/engine/table/src/main/java/io/deephaven/engine/util/TableToolsMergeHelper.java index c369a78ba4e..ee7deb4781c 100644 --- a/engine/table/src/main/java/io/deephaven/engine/util/TableToolsMergeHelper.java +++ b/engine/table/src/main/java/io/deephaven/engine/util/TableToolsMergeHelper.java @@ -115,15 +115,15 @@ private static boolean canBreakOutUnionedTable(Table table) { if (!table.hasAttribute(Table.MERGED_TABLE_ATTRIBUTE)) { return false; } - Map> columnSourceMap = queryTable.getColumnSourceMap(); - if (columnSourceMap.isEmpty()) { + final Collection> columnSources = queryTable.getColumnSources(); + if (columnSources.isEmpty()) { return false; } - if (!columnSourceMap.values().stream().allMatch(cs -> cs instanceof UnionColumnSource)) { + if (!columnSources.stream().allMatch(cs -> cs instanceof UnionColumnSource)) { return false; } - final UnionColumnSource columnSource = (UnionColumnSource) columnSourceMap.values().iterator().next(); + final UnionColumnSource columnSource = (UnionColumnSource) columnSources.iterator().next(); return columnSource.getUnionSourceManager().isUsingComponentsSafe(); } } diff --git a/engine/table/src/main/java/io/deephaven/engine/util/TickSuppressor.java b/engine/table/src/main/java/io/deephaven/engine/util/TickSuppressor.java index 38b65ec5c77..7bf58df0337 100644 --- a/engine/table/src/main/java/io/deephaven/engine/util/TickSuppressor.java +++ b/engine/table/src/main/java/io/deephaven/engine/util/TickSuppressor.java @@ -131,7 +131,7 @@ public void onUpdate(TableUpdate upstream) { return; } - final int columnCount = resultTable.getColumnSourceMap().size(); + final int columnCount = resultTable.numColumns(); final int chunkSize = (int) Math.min(1 << 16, downstream.modified().size()); final ChunkSource.GetContext[] getContextArray = new ChunkSource.GetContext[columnCount]; diff --git a/engine/table/src/main/java/io/deephaven/engine/util/TotalsTableBuilder.java b/engine/table/src/main/java/io/deephaven/engine/util/TotalsTableBuilder.java index d3399ec18ad..5d23a5e5f85 100644 --- a/engine/table/src/main/java/io/deephaven/engine/util/TotalsTableBuilder.java +++ b/engine/table/src/main/java/io/deephaven/engine/util/TotalsTableBuilder.java @@ -6,6 +6,8 @@ import io.deephaven.api.agg.Aggregation; import io.deephaven.engine.table.Table; import io.deephaven.engine.table.ColumnSource; +import io.deephaven.engine.table.impl.NoSuchColumnException; +import io.deephaven.engine.table.impl.NoSuchColumnException.Type; import io.deephaven.util.annotations.ScriptApi; import io.deephaven.util.type.EnumValue; import io.deephaven.util.type.TypeUtils; @@ -546,12 +548,12 @@ public static Table makeTotalsTable(Table source, TotalsTableBuilder builder, St } private static void ensureColumnsExist(Table source, Set columns) { - if (!source.getColumnSourceMap().keySet().containsAll(columns)) { - final Set missing = new LinkedHashSet<>(columns); - missing.removeAll(source.getColumnSourceMap().keySet()); - throw new IllegalArgumentException("Missing columns for totals table " + missing + ", available columns " - + source.getColumnSourceMap().keySet()); - } + NoSuchColumnException.throwIf( + source.getDefinition().getColumnNameSet(), + columns, + "Missing columns for totals table [%s], available columns [%s]", + Type.MISSING, + Type.AVAILABLE); } private static String[] makeColumnFormats(Table source, TotalsTableBuilder builder) { diff --git a/engine/table/src/main/java/io/deephaven/engine/util/WindowCheck.java b/engine/table/src/main/java/io/deephaven/engine/util/WindowCheck.java index e4204268d2f..846a25a2fd7 100644 --- a/engine/table/src/main/java/io/deephaven/engine/util/WindowCheck.java +++ b/engine/table/src/main/java/io/deephaven/engine/util/WindowCheck.java @@ -195,7 +195,7 @@ public int getPos(Entry el) { this.rowKeyToEntry = new TLongObjectHashMap<>(); this.mcsTransformer = source.newModifiedColumnSetTransformer(result, - source.getColumnSourceMap().keySet().toArray(CollectionUtil.ZERO_LENGTH_STRING_ARRAY)); + source.getDefinition().getColumnNamesArray()); this.mcsNewColumns = result.newModifiedColumnSet(inWindowColumnName); this.reusableModifiedColumnSet = new ModifiedColumnSet(this.mcsNewColumns); } diff --git a/engine/table/src/test/java/io/deephaven/engine/table/impl/MultiColumnSortTest.java b/engine/table/src/test/java/io/deephaven/engine/table/impl/MultiColumnSortTest.java index d7e379f6c58..213ed2de322 100644 --- a/engine/table/src/test/java/io/deephaven/engine/table/impl/MultiColumnSortTest.java +++ b/engine/table/src/test/java/io/deephaven/engine/table/impl/MultiColumnSortTest.java @@ -58,7 +58,7 @@ private void testMultiColumnSort(int seed, int size) { new BigIntegerGenerator(BigInteger.valueOf(100000), BigInteger.valueOf(100100)), new BigDecimalGenerator(BigInteger.valueOf(100000), BigInteger.valueOf(100100)))); - final List columnNames = new ArrayList<>(table.getColumnSourceMap().keySet()); + final List columnNames = table.getDefinition().getColumnNames(); doMultiColumnTest(table, SortColumn.asc(ColumnName.of("boolCol")), SortColumn.desc(ColumnName.of("Sym"))); 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 da435bf1a47..d62c00397e9 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 @@ -116,12 +116,12 @@ private static Table individualStaticByTest(@NotNull final Table input, Arrays.stream(keySelectColumns).map(SelectColumn::getName).distinct().toArray(String[]::new); if (keyColumns.length == 0) { - expectedKeys = TableTools.emptyTable(adjustedInput.size() > 0 ? 1 : 0); + expectedKeys = TableTools.emptyTable(!adjustedInput.isEmpty() ? 1 : 0); expected = adjustedInput; } else { final Set retainedColumns = - new LinkedHashSet<>(adjustedInput.getDefinition().getColumnNameMap().keySet()); - retainedColumns.removeAll(Arrays.stream(keyNames).collect(Collectors.toSet())); + new LinkedHashSet<>(adjustedInput.getDefinition().getColumnNameSet()); + Arrays.asList(keyNames).forEach(retainedColumns::remove); final List allSelectColumns = Stream.concat(Arrays.stream(keySelectColumns), retainedColumns.stream().map(SourceColumn::new)) .collect(Collectors.toList()); @@ -887,7 +887,7 @@ public void testKeyColumnTypes() { new BigDecimalGenerator(), new IntGenerator())); - final Set keyColumnSet = new LinkedHashSet<>(table.getColumnSourceMap().keySet()); + final Set keyColumnSet = new LinkedHashSet<>(table.getDefinition().getColumnNameSet()); keyColumnSet.remove("NonKey"); final String[] keyColumns = keyColumnSet.toArray(CollectionUtil.ZERO_LENGTH_STRING_ARRAY); diff --git a/engine/table/src/test/java/io/deephaven/engine/table/impl/QueryTableTest.java b/engine/table/src/test/java/io/deephaven/engine/table/impl/QueryTableTest.java index 9c8ce50d1a1..5648009a55f 100644 --- a/engine/table/src/test/java/io/deephaven/engine/table/impl/QueryTableTest.java +++ b/engine/table/src/test/java/io/deephaven/engine/table/impl/QueryTableTest.java @@ -561,13 +561,17 @@ public void testDropColumns() { try { table.dropColumns(Collections.singletonList("DoesNotExist")); - } catch (RuntimeException e) { - assertEquals("Unknown columns: [DoesNotExist], available columns = [String, Int, Double]", e.getMessage()); + fail("Expected NoSuchColumnException"); + } catch (NoSuchColumnException e) { + assertEquals("Unknown column names [DoesNotExist], available column names are [String, Int, Double]", + e.getMessage()); } try { table.dropColumns(Arrays.asList("Int", "DoesNotExist")); - } catch (RuntimeException e) { - assertEquals("Unknown columns: [DoesNotExist], available columns = [String, Int, Double]", e.getMessage()); + fail("Expected NoSuchColumnException"); + } catch (NoSuchColumnException e) { + assertEquals("Unknown column names [DoesNotExist], available column names are [String, Int, Double]", + e.getMessage()); } } diff --git a/engine/table/src/test/java/io/deephaven/engine/table/impl/SelectOverheadLimiter.java b/engine/table/src/test/java/io/deephaven/engine/table/impl/SelectOverheadLimiter.java index 7e568a5ce67..f21d62024e6 100644 --- a/engine/table/src/test/java/io/deephaven/engine/table/impl/SelectOverheadLimiter.java +++ b/engine/table/src/test/java/io/deephaven/engine/table/impl/SelectOverheadLimiter.java @@ -131,7 +131,7 @@ public static Table clampSelectOverhead(Table input, double permittedOverhead) { { inputRecorder.getValue().setMergedListener(this); inputTransformer = ((QueryTable) input).newModifiedColumnSetTransformer(result, - result.getColumnSourceMap().keySet().toArray(CollectionUtil.ZERO_LENGTH_STRING_ARRAY)); + result.getDefinition().getColumnNamesArray()); } @Override @@ -171,7 +171,7 @@ protected void process() { new ListenerRecorder("clampSelectOverhead.flatResult()", flatResult, result); flatRecorder.setMergedListener(this); flatTransformer = ((QueryTable) flatResult).newModifiedColumnSetTransformer(result, - result.getColumnSourceMap().keySet().toArray(CollectionUtil.ZERO_LENGTH_STRING_ARRAY)); + result.getDefinition().getColumnNamesArray()); flatResult.addUpdateListener(flatRecorder); synchronized (recorders) { diff --git a/engine/table/src/test/java/io/deephaven/engine/table/impl/TestAggBy.java b/engine/table/src/test/java/io/deephaven/engine/table/impl/TestAggBy.java index 0c2174f8a7d..e7ac11a5d1a 100644 --- a/engine/table/src/test/java/io/deephaven/engine/table/impl/TestAggBy.java +++ b/engine/table/src/test/java/io/deephaven/engine/table/impl/TestAggBy.java @@ -818,7 +818,7 @@ public void testAggAllByWithFormatColumn() { assertEquals(2.0, cs.get(0)); result = dataTable.formatColumns("Doubles=Decimal(`##0.00%`)").headBy(1); - Set columnNames = result.getColumnSourceMap().keySet(); + List columnNames = result.getDefinition().getColumnNames(); assertEquals(3, columnNames.size()); // Additional column for formatting information of "Doubles" for (String colName : columnNames) { if (!colName.equalsIgnoreCase(doubleColName) && !colName.equalsIgnoreCase(intColName) && diff --git a/engine/table/src/test/java/io/deephaven/engine/table/impl/TestMoveColumns.java b/engine/table/src/test/java/io/deephaven/engine/table/impl/TestMoveColumns.java index e3f7109dad4..3edb395a20b 100644 --- a/engine/table/src/test/java/io/deephaven/engine/table/impl/TestMoveColumns.java +++ b/engine/table/src/test/java/io/deephaven/engine/table/impl/TestMoveColumns.java @@ -3,6 +3,7 @@ */ package io.deephaven.engine.table.impl; +import io.deephaven.engine.table.ColumnDefinition; import io.deephaven.engine.table.Table; import io.deephaven.engine.testutil.testcase.RefreshingTableTestCase; import io.deephaven.engine.util.TableTools; @@ -149,12 +150,15 @@ public void testMoveDownColumns() { } private void checkColumnOrder(Table t, String expectedOrder) { - final String order = t.getColumnSourceMap().keySet().stream().collect(Collectors.joining("")); + final String order = t.getDefinition() + .getColumnStream() + .map(ColumnDefinition::getName) + .collect(Collectors.joining("")); assertEquals(expectedOrder, order); } private void checkColumnValueOrder(Table t, String expectedOrder) { - final String order = t.getColumnSourceMap().values().stream().mapToInt((col) -> col.getInt(0)) + final String order = t.getColumnSources().stream().mapToInt((col) -> col.getInt(0)) .mapToObj(String::valueOf).collect(Collectors.joining("")); assertEquals(expectedOrder, order); } diff --git a/engine/table/src/test/java/io/deephaven/engine/table/impl/TestTotalsTable.java b/engine/table/src/test/java/io/deephaven/engine/table/impl/TestTotalsTable.java index eb53b5674a2..66d40ade933 100644 --- a/engine/table/src/test/java/io/deephaven/engine/table/impl/TestTotalsTable.java +++ b/engine/table/src/test/java/io/deephaven/engine/table/impl/TestTotalsTable.java @@ -20,6 +20,7 @@ import java.util.LinkedHashSet; import java.util.Map; import java.util.Random; +import java.util.Set; import static io.deephaven.engine.testutil.TstUtils.getTable; import static io.deephaven.engine.testutil.TstUtils.initColumnInfos; @@ -61,10 +62,10 @@ public void testTotalsTable() { final TotalsTableBuilder builder = new TotalsTableBuilder(); final Table totals = ExecutionContext.getContext().getUpdateGraph().exclusiveLock().computeLocked( () -> TotalsTableBuilder.makeTotalsTable(builder.applyToTable(queryTable))); - final Map> resultColumns = totals.getColumnSourceMap(); + final Set resultColumns = totals.getDefinition().getColumnNameSet(); assertEquals(1, totals.size()); assertEquals(new LinkedHashSet<>(Arrays.asList("intCol", "intCol2", "doubleCol", "doubleNullCol", "doubleCol2", - "floatCol", "byteCol", "shortCol")), resultColumns.keySet()); + "floatCol", "byteCol", "shortCol")), resultColumns); assertEquals((long) Numeric.sum((int[]) DataAccessHelpers.getColumn(queryTable, "intCol").getDirect()), DataAccessHelpers.getColumn(totals, "intCol").get(0)); @@ -85,7 +86,7 @@ public void testTotalsTable() { final Table totals2 = ExecutionContext.getContext().getUpdateGraph().exclusiveLock().computeLocked( () -> TotalsTableBuilder.makeTotalsTable(queryTable, builder)); assertEquals(new LinkedHashSet<>(Arrays.asList("Sym", "intCol2", "byteCol")), - totals2.getColumnSourceMap().keySet()); + totals2.getDefinition().getColumnNameSet()); assertEquals(Numeric.min((byte[]) DataAccessHelpers.getColumn(queryTable, "byteCol").getDirect()), DataAccessHelpers.getColumn(totals2, "byteCol").get(0)); assertEquals(DataAccessHelpers.getColumn(queryTable, "Sym").get(0), @@ -107,7 +108,7 @@ public void testTotalsTable() { assertEquals( new LinkedHashSet<>(Arrays.asList("Sym", "intCol2", "doubleCol", "doubleNullCol__Std", "doubleNullCol__Count", "doubleCol2", "byteCol", "shortCol")), - totals3.getColumnSourceMap().keySet()); + totals3.getDefinition().getColumnNameSet()); assertEquals( Numeric.max((byte[]) DataAccessHelpers.getColumn(queryTable, "byteCol").getDirect()), DataAccessHelpers.getColumn(totals3, "byteCol").getByte(0)); diff --git a/engine/table/src/test/java/io/deephaven/engine/table/impl/indexer/TestRowSetIndexer.java b/engine/table/src/test/java/io/deephaven/engine/table/impl/indexer/TestRowSetIndexer.java index 1c2a7d3cad7..637cb914643 100644 --- a/engine/table/src/test/java/io/deephaven/engine/table/impl/indexer/TestRowSetIndexer.java +++ b/engine/table/src/test/java/io/deephaven/engine/table/impl/indexer/TestRowSetIndexer.java @@ -164,8 +164,8 @@ private void testGrouping(final boolean immutableColumns, final Random random, f private final ArrayList groupingValidators = new ArrayList<>(); private void addGroupingValidator(Table originalValue, String context) { - ArrayList> columnSets2 = powerSet(originalValue.getColumnSourceMap().keySet()); - ArrayList columnNames = new ArrayList<>(originalValue.getColumnSourceMap().keySet()); + ArrayList> columnSets2 = powerSet(originalValue.getDefinition().getColumnNameSet()); + ArrayList columnNames = new ArrayList<>(originalValue.getDefinition().getColumnNameSet()); columnSets2.add(columnNames); groupingValidators.add(new GroupingValidator(context, originalValue, columnSets2)); } diff --git a/engine/test-utils/src/main/java/io/deephaven/engine/testutil/TstUtils.java b/engine/test-utils/src/main/java/io/deephaven/engine/testutil/TstUtils.java index efd539bff3a..83b96d3fe56 100644 --- a/engine/test-utils/src/main/java/io/deephaven/engine/testutil/TstUtils.java +++ b/engine/test-utils/src/main/java/io/deephaven/engine/testutil/TstUtils.java @@ -20,6 +20,8 @@ import io.deephaven.engine.table.Table; import io.deephaven.engine.table.impl.AbstractColumnSource; import io.deephaven.engine.table.impl.BaseTable; +import io.deephaven.engine.table.impl.NoSuchColumnException; +import io.deephaven.engine.table.impl.NoSuchColumnException.Type; import io.deephaven.engine.table.impl.PrevColumnSource; import io.deephaven.engine.table.impl.QueryTable; import io.deephaven.engine.table.impl.select.Formula; @@ -173,11 +175,12 @@ public static void addToTable(final Table table, final RowSet rowSet, final Colu } } - if (!usedNames.containsAll(table.getColumnSourceMap().keySet())) { - final Set expected = new LinkedHashSet<>(table.getColumnSourceMap().keySet()); - expected.removeAll(usedNames); - throw new IllegalStateException("Not all columns were populated, missing " + expected); - } + NoSuchColumnException.throwIf( + usedNames, + table.getDefinition().getColumnNameSet(), + "Not all columns were populated, missing [%s], available [%s]", + Type.MISSING, + Type.AVAILABLE); table.getRowSet().writableCast().insert(rowSet); if (table.isFlat()) { diff --git a/engine/test-utils/src/main/java/io/deephaven/engine/testutil/locations/TableBackedColumnLocation.java b/engine/test-utils/src/main/java/io/deephaven/engine/testutil/locations/TableBackedColumnLocation.java index 6c4c9a27f2a..a0cce6fe00b 100644 --- a/engine/test-utils/src/main/java/io/deephaven/engine/testutil/locations/TableBackedColumnLocation.java +++ b/engine/test-utils/src/main/java/io/deephaven/engine/testutil/locations/TableBackedColumnLocation.java @@ -26,7 +26,7 @@ public final class TableBackedColumnLocation @NotNull final TableBackedTableLocation tableLocation, @NotNull final String name) { super(tableLocation, name); - columnSource = tableLocation.table().getDefinition().getColumnNameMap().containsKey(name) + columnSource = tableLocation.table().getDefinition().getColumnNameSet().contains(name) ? ReinterpretUtils.maybeConvertToPrimitive(tableLocation.table().getColumnSource(name)) : null; } diff --git a/extensions/jdbc/src/test/java/io/deephaven/jdbc/JdbcToTableAdapterTest.java b/extensions/jdbc/src/test/java/io/deephaven/jdbc/JdbcToTableAdapterTest.java index d301bf9d153..0c882baede9 100644 --- a/extensions/jdbc/src/test/java/io/deephaven/jdbc/JdbcToTableAdapterTest.java +++ b/extensions/jdbc/src/test/java/io/deephaven/jdbc/JdbcToTableAdapterTest.java @@ -87,7 +87,7 @@ public void testEmptyTable() throws SQLException { // check no-casing column names final Set expectedNames = Set.of("Bool_Type", "TinyIntType", "SmallIntType", "Int_Type", "Big_Int_Type", "Decimal_Type", "String_Type", "DateTime_Type"); - Assert.assertEquals(expectedNames, result.getColumnSourceMap().keySet()); + Assert.assertEquals(expectedNames, result.getDefinition().getColumnNameSet()); // should be an empty table Assert.assertEquals(0, result.size()); @@ -103,7 +103,7 @@ public void testLowerCamelCasing() throws SQLException { // check no-casing column names final Set expectedNames = Set.of("boolType", "tinyIntType", "smallIntType", "intType", "bigIntType", "decimalType", "stringType", "datetimeType"); - Assert.assertEquals(expectedNames, result.getColumnSourceMap().keySet()); + Assert.assertEquals(expectedNames, result.getDefinition().getColumnNameSet()); // should be an empty table Assert.assertEquals(0, result.size()); @@ -119,7 +119,7 @@ public void testLowercaseCasing() throws SQLException { // check no-casing column names final Set expectedNames = Set.of("bool_type", "tiny_int_type", "small_int_type", "int_type", "big_int_type", "decimal_type", "string_type", "datetime_type"); - Assert.assertEquals(expectedNames, result.getColumnSourceMap().keySet()); + Assert.assertEquals(expectedNames, result.getDefinition().getColumnNameSet()); // should be an empty table Assert.assertEquals(0, result.size()); @@ -135,7 +135,7 @@ public void testUpperCamelCasing() throws SQLException { // check no-casing column names final Set expectedNames = Set.of("BoolType", "TinyIntType", "SmallIntType", "IntType", "BigIntType", "DecimalType", "StringType", "DatetimeType"); - Assert.assertEquals(expectedNames, result.getColumnSourceMap().keySet()); + Assert.assertEquals(expectedNames, result.getDefinition().getColumnNameSet()); // should be an empty table Assert.assertEquals(0, result.size()); @@ -151,7 +151,7 @@ public void testUppercaseCasing() throws SQLException { // check no-casing column names final Set expectedNames = Set.of("BOOL_TYPE", "TINY_INT_TYPE", "SMALL_INT_TYPE", "INT_TYPE", "BIG_INT_TYPE", "DECIMAL_TYPE", "STRING_TYPE", "DATETIME_TYPE"); - Assert.assertEquals(expectedNames, result.getColumnSourceMap().keySet()); + Assert.assertEquals(expectedNames, result.getDefinition().getColumnNameSet()); // should be an empty table Assert.assertEquals(0, result.size()); @@ -167,7 +167,7 @@ public void testAlternateReplacement() throws SQLException { // check no-casing column names final Set expectedNames = Set.of("BOOL_Z_TYPE", "TINY_Z_INT_Z_TYPE", "SMALL_Z_INT_Z_TYPE", "INT_Z_TYPE", "BIG_Z_INT_Z_TYPE", "DECIMAL_Z_TYPE", "STRING_Z_TYPE", "DATETIME_Z_TYPE"); - Assert.assertEquals(expectedNames, result.getColumnSourceMap().keySet()); + Assert.assertEquals(expectedNames, result.getDefinition().getColumnNameSet()); // should be an empty table Assert.assertEquals(0, result.size()); diff --git a/extensions/parquet/table/src/test/java/io/deephaven/parquet/table/ParquetTableReadWriteTest.java b/extensions/parquet/table/src/test/java/io/deephaven/parquet/table/ParquetTableReadWriteTest.java index 026617ed081..5924b9bbfa6 100644 --- a/extensions/parquet/table/src/test/java/io/deephaven/parquet/table/ParquetTableReadWriteTest.java +++ b/extensions/parquet/table/src/test/java/io/deephaven/parquet/table/ParquetTableReadWriteTest.java @@ -460,7 +460,9 @@ public void testVectorColumns() { writeReadTableTest(vectorTable, dest); // Convert the table from vector to array column - final Table arrayTable = vectorTable.updateView(vectorTable.getColumnSourceMap().keySet().stream() + final Table arrayTable = vectorTable.updateView(vectorTable.getDefinition() + .getColumnStream() + .map(ColumnDefinition::getName) .map(name -> name + " = " + name + ".toArray()") .toArray(String[]::new)); writeReadTableTest(arrayTable, dest); @@ -925,7 +927,7 @@ public void partitionedParquetWithDotFilesTest() throws IOException { ParquetTools.writeTable(someTable, secondDataFile); Table partitionedTable = ParquetTools.readTable(parentDir).select(); - final Set columnsSet = partitionedTable.getColumnSourceMap().keySet(); + final Set columnsSet = partitionedTable.getDefinition().getColumnNameSet(); assertTrue(columnsSet.size() == 2 && columnsSet.contains("A") && columnsSet.contains("X")); // Add an empty dot file and dot directory (with valid parquet files) in one of the partitions @@ -1393,8 +1395,7 @@ private void assertTableStatistics(Table inputTable, File dest) { // Verify that the columns have the correct statistics. final ParquetMetadata metadata = new ParquetTableLocationKey(dest, 0, null).getMetadata(); - final String[] colNames = - inputTable.getColumnSourceMap().keySet().toArray(CollectionUtil.ZERO_LENGTH_STRING_ARRAY); + final String[] colNames = inputTable.getDefinition().getColumnNamesArray(); for (int colIdx = 0; colIdx < inputTable.numColumns(); ++colIdx) { final String colName = colNames[colIdx]; diff --git a/server/src/main/java/io/deephaven/server/table/ops/SelectDistinctGrpcImpl.java b/server/src/main/java/io/deephaven/server/table/ops/SelectDistinctGrpcImpl.java index c96364c4ce6..0666a06ab4b 100644 --- a/server/src/main/java/io/deephaven/server/table/ops/SelectDistinctGrpcImpl.java +++ b/server/src/main/java/io/deephaven/server/table/ops/SelectDistinctGrpcImpl.java @@ -14,9 +14,9 @@ import javax.inject.Inject; import javax.inject.Singleton; -import java.util.HashSet; import java.util.List; -import java.util.Set; +import java.util.function.Predicate; +import java.util.stream.Collectors; @Singleton public class SelectDistinctGrpcImpl extends GrpcTableOperation { @@ -34,8 +34,10 @@ public Table create(final SelectDistinctRequest request, final Table parent = sourceTables.get(0).get(); // explicitly disallow column expressions - final Set requestedMissing = new HashSet<>(request.getColumnNamesList()); - requestedMissing.removeAll(parent.getDefinition().getColumnNameMap().keySet()); + final List requestedMissing = request.getColumnNamesList() + .stream() + .filter(Predicate.not(parent.getDefinition().getColumnNameSet()::contains)) + .collect(Collectors.toList()); if (!requestedMissing.isEmpty()) { throw Exceptions.statusRuntimeException(Code.FAILED_PRECONDITION, "column(s) not found: " + String.join(", ", requestedMissing));