From 0bde6fb65c4ba3e5c1f2b788b6980fe6b0f9a035 Mon Sep 17 00:00:00 2001 From: Devin Smith Date: Wed, 1 Mar 2023 11:50:51 -0800 Subject: [PATCH 1/6] Add dropColumns as TableOperations / TableSpec This is in support of #3473, where for adapting implementation purposes, it's cleaner and easier to use dropColumns --- .../java/io/deephaven/engine/table/Table.java | 6 --- .../engine/table/impl/TableDefaults.java | 7 --- .../PartitionedTableProxyImpl.java | 6 +++ .../client/impl/BatchTableRequestBuilder.java | 13 +++++ .../io/deephaven/graphviz/LabelBuilder.java | 8 +++ .../io/deephaven/qst/TableAdapterImpl.java | 7 +++ .../deephaven/qst/table/DropColumnsTable.java | 50 +++++++++++++++++++ .../deephaven/qst/table/ParentsVisitor.java | 5 ++ .../io/deephaven/qst/table/TableBase.java | 25 ++++++++++ .../io/deephaven/qst/table/TableSpec.java | 2 + .../qst/table/TableVisitorGeneric.java | 5 ++ .../io/deephaven/api/TableOperations.java | 27 ++++++++++ .../deephaven/api/TableOperationsAdapter.java | 15 ++++++ .../api/TableOperationsDefaults.java | 14 ++++++ 14 files changed, 177 insertions(+), 13 deletions(-) create mode 100644 qst/src/main/java/io/deephaven/qst/table/DropColumnsTable.java diff --git a/engine/api/src/main/java/io/deephaven/engine/table/Table.java b/engine/api/src/main/java/io/deephaven/engine/table/Table.java index 309a63d378a..cd364d4311c 100644 --- a/engine/api/src/main/java/io/deephaven/engine/table/Table.java +++ b/engine/api/src/main/java/io/deephaven/engine/table/Table.java @@ -303,12 +303,6 @@ public interface Table extends @ConcurrentMethod Table updateView(Selectable... newColumns); - @ConcurrentMethod - Table dropColumns(Collection columnNames); - - @ConcurrentMethod - Table dropColumns(String... columnNames); - @ConcurrentMethod Table dropColumnFormats(); 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 0b567edf744..bf9f17d7879 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 @@ -263,13 +263,6 @@ default Table updateView(Selectable... newColumns) { return updateView(List.of(newColumns)); } - @Override - @ConcurrentMethod - @FinalDefault - default Table dropColumns(Collection columnNames) { - return dropColumns(columnNames.toArray(CollectionUtil.ZERO_LENGTH_STRING_ARRAY)); - } - @Override @ConcurrentMethod @FinalDefault diff --git a/engine/table/src/main/java/io/deephaven/engine/table/impl/partitioned/PartitionedTableProxyImpl.java b/engine/table/src/main/java/io/deephaven/engine/table/impl/partitioned/PartitionedTableProxyImpl.java index 081ea3615d6..6e258ffc761 100644 --- a/engine/table/src/main/java/io/deephaven/engine/table/impl/partitioned/PartitionedTableProxyImpl.java +++ b/engine/table/src/main/java/io/deephaven/engine/table/impl/partitioned/PartitionedTableProxyImpl.java @@ -15,6 +15,7 @@ import io.deephaven.engine.liveness.LivenessArtifact; import io.deephaven.engine.table.MatchPair; import io.deephaven.engine.table.PartitionedTable; +import io.deephaven.engine.table.PartitionedTable.Proxy; import io.deephaven.engine.table.Table; import io.deephaven.engine.table.TableDefinition; import io.deephaven.engine.table.TableUpdate; @@ -571,5 +572,10 @@ public PartitionedTable.Proxy ungroup(boolean nullFill, Collection ct.ungroup(nullFill, columnsToUngroup)); } + @Override + public PartitionedTable.Proxy dropColumns(String... columnNames) { + return basicTransform(ct -> ct.dropColumns(columnNames)); + } + // endregion TableOperations Implementation } diff --git a/java-client/session/src/main/java/io/deephaven/client/impl/BatchTableRequestBuilder.java b/java-client/session/src/main/java/io/deephaven/client/impl/BatchTableRequestBuilder.java index 2a22b99a1c5..a7f3fef63a6 100644 --- a/java-client/session/src/main/java/io/deephaven/client/impl/BatchTableRequestBuilder.java +++ b/java-client/session/src/main/java/io/deephaven/client/impl/BatchTableRequestBuilder.java @@ -41,6 +41,7 @@ import io.deephaven.proto.backplane.grpc.CreateInputTableRequest.InputTableKind.InMemoryAppendOnly; import io.deephaven.proto.backplane.grpc.CreateInputTableRequest.InputTableKind.InMemoryKeyBacked; import io.deephaven.proto.backplane.grpc.CrossJoinTablesRequest; +import io.deephaven.proto.backplane.grpc.DropColumnsRequest; import io.deephaven.proto.backplane.grpc.EmptyTableRequest; import io.deephaven.proto.backplane.grpc.ExactJoinTablesRequest; import io.deephaven.proto.backplane.grpc.FetchTableRequest; @@ -73,6 +74,7 @@ import io.deephaven.qst.table.AsOfJoinTable; import io.deephaven.qst.table.Clock.Visitor; import io.deephaven.qst.table.ClockSystem; +import io.deephaven.qst.table.DropColumnsTable; import io.deephaven.qst.table.EmptyTable; import io.deephaven.qst.table.ExactJoinTable; import io.deephaven.qst.table.HeadTable; @@ -505,6 +507,17 @@ public void visit(UngroupTable ungroupTable) { out = op(Builder::setUngroup, request); } + @Override + public void visit(DropColumnsTable dropColumnsTable) { + final DropColumnsRequest.Builder request = DropColumnsRequest.newBuilder() + .setResultId(ticket) + .setSourceId(ref(dropColumnsTable.parent())); + for (ColumnName dropColumn : dropColumnsTable.dropColumns()) { + request.addColumnNames(dropColumn.name()); + } + out = op(Builder::setDropColumns, request); + } + private SelectOrUpdateRequest selectOrUpdate(SingleParentTable x, Collection columns) { SelectOrUpdateRequest.Builder builder = diff --git a/qst/graphviz/src/main/java/io/deephaven/graphviz/LabelBuilder.java b/qst/graphviz/src/main/java/io/deephaven/graphviz/LabelBuilder.java index 79b7e8ddaf4..93111856794 100644 --- a/qst/graphviz/src/main/java/io/deephaven/graphviz/LabelBuilder.java +++ b/qst/graphviz/src/main/java/io/deephaven/graphviz/LabelBuilder.java @@ -7,6 +7,7 @@ import io.deephaven.qst.table.AggregateAllTable; import io.deephaven.qst.table.AggregateTable; import io.deephaven.qst.table.AsOfJoinTable; +import io.deephaven.qst.table.DropColumnsTable; import io.deephaven.qst.table.EmptyTable; import io.deephaven.qst.table.ExactJoinTable; import io.deephaven.qst.table.HeadTable; @@ -198,6 +199,13 @@ public void visit(UngroupTable ungroupTable) { sb.append("])"); } + @Override + public void visit(DropColumnsTable dropColumnsTable) { + sb.append("dropColumns(["); + append(Strings::of, dropColumnsTable.dropColumns(), sb); + sb.append("])"); + } + private void join(String name, Join j) { sb.append(name).append("(["); append(Strings::of, j.matches(), sb); diff --git a/qst/src/main/java/io/deephaven/qst/TableAdapterImpl.java b/qst/src/main/java/io/deephaven/qst/TableAdapterImpl.java index 16e5da7659f..75cfba9daed 100644 --- a/qst/src/main/java/io/deephaven/qst/TableAdapterImpl.java +++ b/qst/src/main/java/io/deephaven/qst/TableAdapterImpl.java @@ -10,6 +10,7 @@ import io.deephaven.qst.table.AggregateAllTable; import io.deephaven.qst.table.AggregateTable; import io.deephaven.qst.table.AsOfJoinTable; +import io.deephaven.qst.table.DropColumnsTable; import io.deephaven.qst.table.EmptyTable; import io.deephaven.qst.table.ExactJoinTable; import io.deephaven.qst.table.HeadTable; @@ -309,6 +310,12 @@ public void visit(UngroupTable ungroupTable) { .ungroup(ungroupTable.nullFill(), ungroupTable.ungroupColumns())); } + @Override + public void visit(DropColumnsTable dropColumnsTable) { + addOp(dropColumnsTable, + parentOps(dropColumnsTable).dropColumns(dropColumnsTable.dropColumns().toArray(new ColumnName[0]))); + } + private final class OutputTable implements Output { private final TABLE table; diff --git a/qst/src/main/java/io/deephaven/qst/table/DropColumnsTable.java b/qst/src/main/java/io/deephaven/qst/table/DropColumnsTable.java new file mode 100644 index 00000000000..abdbe5f1635 --- /dev/null +++ b/qst/src/main/java/io/deephaven/qst/table/DropColumnsTable.java @@ -0,0 +1,50 @@ +/** + * Copyright (c) 2016-2022 Deephaven Data Labs and Patent Pending + */ +package io.deephaven.qst.table; + +import io.deephaven.annotations.NodeStyle; +import io.deephaven.api.ColumnName; +import org.immutables.value.Value.Check; +import org.immutables.value.Value.Default; +import org.immutables.value.Value.Immutable; + +import java.util.List; + +@Immutable +@NodeStyle +public abstract class DropColumnsTable extends TableBase implements SingleParentTable { + + public static Builder builder() { + return ImmutableDropColumnsTable.builder(); + } + + public abstract TableSpec parent(); + + public abstract List dropColumns(); + + @Override + public final V walk(V visitor) { + visitor.visit(this); + return visitor; + } + + @Check + final void checkNonEmpty() { + if (dropColumns().isEmpty()) { + throw new IllegalArgumentException("Expected dropColumns() to be non-empty"); + } + } + + interface Builder { + Builder parent(TableSpec parent); + + Builder addDropColumns(ColumnName element); + + Builder addDropColumns(ColumnName... elements); + + Builder addAllDropColumns(Iterable elements); + + DropColumnsTable build(); + } +} diff --git a/qst/src/main/java/io/deephaven/qst/table/ParentsVisitor.java b/qst/src/main/java/io/deephaven/qst/table/ParentsVisitor.java index 97c54db2c2a..261c3a74b99 100644 --- a/qst/src/main/java/io/deephaven/qst/table/ParentsVisitor.java +++ b/qst/src/main/java/io/deephaven/qst/table/ParentsVisitor.java @@ -296,6 +296,11 @@ public void visit(UngroupTable ungroupTable) { out = single(ungroupTable); } + @Override + public void visit(DropColumnsTable dropColumnsTable) { + out = single(dropColumnsTable); + } + private static class Search { private final Predicate excludePaths; diff --git a/qst/src/main/java/io/deephaven/qst/table/TableBase.java b/qst/src/main/java/io/deephaven/qst/table/TableBase.java index 364abd0aba0..f4ff2305d40 100644 --- a/qst/src/main/java/io/deephaven/qst/table/TableBase.java +++ b/qst/src/main/java/io/deephaven/qst/table/TableBase.java @@ -932,6 +932,31 @@ public final UngroupTable ungroup(boolean nullFill, Collection columnNames) { + final DropColumnsTable.Builder builder = DropColumnsTable.builder() + .parent(this); + for (String columnName : columnNames) { + builder.addDropColumns(ColumnName.of(columnName)); + } + return builder.build(); + } + + @Override + public final DropColumnsTable dropColumns(ColumnName[] columnNames) { + return DropColumnsTable.builder().parent(this).addDropColumns(columnNames).build(); + } + @Override public final V walk(V visitor) { visitor.visit(this); diff --git a/qst/src/main/java/io/deephaven/qst/table/TableSpec.java b/qst/src/main/java/io/deephaven/qst/table/TableSpec.java index 8a9cd067879..a4e9cc48f68 100644 --- a/qst/src/main/java/io/deephaven/qst/table/TableSpec.java +++ b/qst/src/main/java/io/deephaven/qst/table/TableSpec.java @@ -157,5 +157,7 @@ interface Visitor { void visit(UpdateByTable updateByTable); void visit(UngroupTable ungroupTable); + + void visit(DropColumnsTable dropColumnsTable); } } diff --git a/qst/src/main/java/io/deephaven/qst/table/TableVisitorGeneric.java b/qst/src/main/java/io/deephaven/qst/table/TableVisitorGeneric.java index aa9a16f78f5..4d12140a0d6 100644 --- a/qst/src/main/java/io/deephaven/qst/table/TableVisitorGeneric.java +++ b/qst/src/main/java/io/deephaven/qst/table/TableVisitorGeneric.java @@ -151,4 +151,9 @@ public void visit(UpdateByTable updateByTable) { public void visit(UngroupTable ungroupTable) { accept(ungroupTable); } + + @Override + public void visit(DropColumnsTable dropColumnsTable) { + accept(dropColumnsTable); + } } diff --git a/table-api/src/main/java/io/deephaven/api/TableOperations.java b/table-api/src/main/java/io/deephaven/api/TableOperations.java index 409987abf03..6d51e01c386 100644 --- a/table-api/src/main/java/io/deephaven/api/TableOperations.java +++ b/table-api/src/main/java/io/deephaven/api/TableOperations.java @@ -1300,4 +1300,31 @@ TOPS updateBy(UpdateByControl control, Collection o TOPS ungroup(boolean nullFill, Collection columnsToUngroup); // ------------------------------------------------------------------------------------------- + + /** + * Drop the {@code columnNames} from {@code this} table. + * + * @param columnNames the columns to drop + * @return the table + */ + TOPS dropColumns(String... columnNames); + + /** + * Drop the {@code columnNames} from {@code this} table. + * + * @param columnNames the columns to drop + * @return the table + */ + TOPS dropColumns(Collection columnNames); + + /** + * Drop the {@code columnNames} from {@code this} table. + * + * @param columnNames the columns to drop + * @return the table + */ + TOPS dropColumns(ColumnName[] columnNames); + + // ------------------------------------------------------------------------------------------- + } diff --git a/table-api/src/main/java/io/deephaven/api/TableOperationsAdapter.java b/table-api/src/main/java/io/deephaven/api/TableOperationsAdapter.java index c26abb4b48b..3cbfcacae41 100644 --- a/table-api/src/main/java/io/deephaven/api/TableOperationsAdapter.java +++ b/table-api/src/main/java/io/deephaven/api/TableOperationsAdapter.java @@ -685,4 +685,19 @@ public final TOPS_1 ungroup(boolean nullFill, String... columnsToUngroup) { public final TOPS_1 ungroup(boolean nullFill, Collection columnsToUngroup) { return adapt(delegate.ungroup(nullFill, columnsToUngroup)); } + + @Override + public final TOPS_1 dropColumns(String... columnNames) { + return adapt(delegate.dropColumns(columnNames)); + } + + @Override + public final TOPS_1 dropColumns(Collection columnNames) { + return adapt(delegate.dropColumns(columnNames)); + } + + @Override + public final TOPS_1 dropColumns(ColumnName[] columnNames) { + return adapt(delegate.dropColumns(columnNames)); + } } diff --git a/table-api/src/main/java/io/deephaven/api/TableOperationsDefaults.java b/table-api/src/main/java/io/deephaven/api/TableOperationsDefaults.java index a3d432185b8..6b32f14144c 100644 --- a/table-api/src/main/java/io/deephaven/api/TableOperationsDefaults.java +++ b/table-api/src/main/java/io/deephaven/api/TableOperationsDefaults.java @@ -690,6 +690,20 @@ default TOPS wavgBy(String weightColumn, Collection groupByColumns) { // ------------------------------------------------------------------------------------------- + @Override + @ConcurrentMethod + default TOPS dropColumns(Collection columnNames) { + return dropColumns(columnNames.toArray(new String[0])); + } + + @Override + @ConcurrentMethod + default TOPS dropColumns(ColumnName[] columnNames) { + return dropColumns(Arrays.stream(columnNames).map(ColumnName::name).toArray(String[]::new)); + } + + // ------------------------------------------------------------------------------------------- + static Collection splitToCollection(String string) { return string.trim().isEmpty() ? Collections.emptyList() : Arrays.stream(string.split(",")).map(String::trim).filter(s -> !s.isEmpty()) From bbaf7abd3a055ec4cf34ddfc70d64d0ff7ae58b5 Mon Sep 17 00:00:00 2001 From: Devin Smith Date: Wed, 1 Mar 2023 13:38:42 -0800 Subject: [PATCH 2/6] Correct builder visibility --- qst/src/main/java/io/deephaven/qst/table/DropColumnsTable.java | 2 +- qst/src/main/java/io/deephaven/qst/table/TimeTable.java | 2 +- qst/src/main/java/io/deephaven/qst/table/UngroupTable.java | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/qst/src/main/java/io/deephaven/qst/table/DropColumnsTable.java b/qst/src/main/java/io/deephaven/qst/table/DropColumnsTable.java index abdbe5f1635..df1d13cdaa6 100644 --- a/qst/src/main/java/io/deephaven/qst/table/DropColumnsTable.java +++ b/qst/src/main/java/io/deephaven/qst/table/DropColumnsTable.java @@ -36,7 +36,7 @@ final void checkNonEmpty() { } } - interface Builder { + public interface Builder { Builder parent(TableSpec parent); Builder addDropColumns(ColumnName element); diff --git a/qst/src/main/java/io/deephaven/qst/table/TimeTable.java b/qst/src/main/java/io/deephaven/qst/table/TimeTable.java index c827f1ad048..275343db7ce 100644 --- a/qst/src/main/java/io/deephaven/qst/table/TimeTable.java +++ b/qst/src/main/java/io/deephaven/qst/table/TimeTable.java @@ -79,7 +79,7 @@ final void checkTimeout() { } } - interface Builder { + public interface Builder { Builder clock(Clock clock); Builder interval(Duration interval); diff --git a/qst/src/main/java/io/deephaven/qst/table/UngroupTable.java b/qst/src/main/java/io/deephaven/qst/table/UngroupTable.java index 938e603ac77..48a9f507583 100644 --- a/qst/src/main/java/io/deephaven/qst/table/UngroupTable.java +++ b/qst/src/main/java/io/deephaven/qst/table/UngroupTable.java @@ -33,7 +33,7 @@ public final V walk(V visitor) { return visitor; } - interface Builder { + public interface Builder { Builder parent(TableSpec parent); Builder addUngroupColumns(ColumnName element); From c95e0e9339b2dfef431781c66630f3b6d3654953 Mon Sep 17 00:00:00 2001 From: Devin Smith Date: Wed, 1 Mar 2023 15:11:36 -0800 Subject: [PATCH 3/6] Fix python drop_columns --- py/server/deephaven/table.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/py/server/deephaven/table.py b/py/server/deephaven/table.py index de5d961b67b..6c2d9286008 100644 --- a/py/server/deephaven/table.py +++ b/py/server/deephaven/table.py @@ -440,7 +440,7 @@ def drop_columns(self, cols: Union[str, Sequence[str]]) -> Table: """ try: cols = to_sequence(cols) - return Table(j_table=self.j_table.dropColumns(*cols)) + return Table(j_table=self.j_table.dropColumns(j_array_list(cols))) except Exception as e: raise DHError(e, "table drop_columns operation failed.") from e From 637263024d59aa4798b0c5744db8a8d7183462e2 Mon Sep 17 00:00:00 2001 From: Devin Smith Date: Wed, 1 Mar 2023 15:23:50 -0800 Subject: [PATCH 4/6] Migrate to vararg so we don't need to fix python layer --- .../deephaven/engine/table/impl/QueryTableTest.java | 13 ++++++++----- py/server/deephaven/table.py | 2 +- .../main/java/io/deephaven/qst/table/TableBase.java | 2 +- .../main/java/io/deephaven/api/TableOperations.java | 2 +- .../io/deephaven/api/TableOperationsAdapter.java | 2 +- .../io/deephaven/api/TableOperationsDefaults.java | 2 +- 6 files changed, 13 insertions(+), 10 deletions(-) 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 d5fdc9f9cc2..78774477fa6 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 @@ -553,18 +553,21 @@ public void testDropColumns() { final List> colSources = Arrays.asList(TableTools.objColSource("c", "e", "g"), colSource(2, 4, 6), colSource(1.0, 2.0, 3.0)); final Table table = newTable(3, colNames, colSources); - assertEquals(3, table.dropColumns().getColumnSources().size()); + assertEquals(3, table.dropColumns(Collections.emptyList()).getColumnSources().size()); Collection> columnSourcesAfterDrop = table.getColumnSources(); ColumnSource[] columnsAfterDrop = columnSourcesAfterDrop.toArray(ColumnSource.ZERO_LENGTH_COLUMN_SOURCE_ARRAY); - Collection> columnSources = table.dropColumns().getColumnSources(); + Collection> columnSources = + table.dropColumns(Collections.emptyList()).getColumnSources(); ColumnSource[] columns = columnSources.toArray(ColumnSource.ZERO_LENGTH_COLUMN_SOURCE_ARRAY); assertSame(columns[0], columnsAfterDrop[0]); assertSame(columns[1], columnsAfterDrop[1]); assertSame(columns[2], columnsAfterDrop[2]); - assertSame(table.getColumnSource("String"), table.dropColumns().getColumnSource("String")); - assertSame(table.getColumnSource("Int"), table.dropColumns().getColumnSource("Int")); - assertSame(table.getColumnSource("Double"), table.dropColumns().getColumnSource("Double")); + assertSame(table.getColumnSource("String"), + table.dropColumns(Collections.emptyList()).getColumnSource("String")); + assertSame(table.getColumnSource("Int"), table.dropColumns(Collections.emptyList()).getColumnSource("Int")); + assertSame(table.getColumnSource("Double"), + table.dropColumns(Collections.emptyList()).getColumnSource("Double")); assertEquals(2, table.dropColumns("Int").getColumnSources().size()); columnSourcesAfterDrop = table.dropColumns("Int").getColumnSources(); diff --git a/py/server/deephaven/table.py b/py/server/deephaven/table.py index 6c2d9286008..de5d961b67b 100644 --- a/py/server/deephaven/table.py +++ b/py/server/deephaven/table.py @@ -440,7 +440,7 @@ def drop_columns(self, cols: Union[str, Sequence[str]]) -> Table: """ try: cols = to_sequence(cols) - return Table(j_table=self.j_table.dropColumns(j_array_list(cols))) + return Table(j_table=self.j_table.dropColumns(*cols)) except Exception as e: raise DHError(e, "table drop_columns operation failed.") from e diff --git a/qst/src/main/java/io/deephaven/qst/table/TableBase.java b/qst/src/main/java/io/deephaven/qst/table/TableBase.java index f4ff2305d40..46bdd17fe73 100644 --- a/qst/src/main/java/io/deephaven/qst/table/TableBase.java +++ b/qst/src/main/java/io/deephaven/qst/table/TableBase.java @@ -953,7 +953,7 @@ public final DropColumnsTable dropColumns(Collection columnNames) { } @Override - public final DropColumnsTable dropColumns(ColumnName[] columnNames) { + public final DropColumnsTable dropColumns(ColumnName... columnNames) { return DropColumnsTable.builder().parent(this).addDropColumns(columnNames).build(); } diff --git a/table-api/src/main/java/io/deephaven/api/TableOperations.java b/table-api/src/main/java/io/deephaven/api/TableOperations.java index 6d51e01c386..eb9fa4a75b0 100644 --- a/table-api/src/main/java/io/deephaven/api/TableOperations.java +++ b/table-api/src/main/java/io/deephaven/api/TableOperations.java @@ -1323,7 +1323,7 @@ TOPS updateBy(UpdateByControl control, Collection o * @param columnNames the columns to drop * @return the table */ - TOPS dropColumns(ColumnName[] columnNames); + TOPS dropColumns(ColumnName... columnNames); // ------------------------------------------------------------------------------------------- diff --git a/table-api/src/main/java/io/deephaven/api/TableOperationsAdapter.java b/table-api/src/main/java/io/deephaven/api/TableOperationsAdapter.java index 3cbfcacae41..29cfd1eec5e 100644 --- a/table-api/src/main/java/io/deephaven/api/TableOperationsAdapter.java +++ b/table-api/src/main/java/io/deephaven/api/TableOperationsAdapter.java @@ -697,7 +697,7 @@ public final TOPS_1 dropColumns(Collection columnNames) { } @Override - public final TOPS_1 dropColumns(ColumnName[] columnNames) { + public final TOPS_1 dropColumns(ColumnName... columnNames) { return adapt(delegate.dropColumns(columnNames)); } } diff --git a/table-api/src/main/java/io/deephaven/api/TableOperationsDefaults.java b/table-api/src/main/java/io/deephaven/api/TableOperationsDefaults.java index 6b32f14144c..0dd874d83b3 100644 --- a/table-api/src/main/java/io/deephaven/api/TableOperationsDefaults.java +++ b/table-api/src/main/java/io/deephaven/api/TableOperationsDefaults.java @@ -698,7 +698,7 @@ default TOPS dropColumns(Collection columnNames) { @Override @ConcurrentMethod - default TOPS dropColumns(ColumnName[] columnNames) { + default TOPS dropColumns(ColumnName... columnNames) { return dropColumns(Arrays.stream(columnNames).map(ColumnName::name).toArray(String[]::new)); } From afeba6cdb4d6fea044b10169aeb966725692518d Mon Sep 17 00:00:00 2001 From: Devin Smith Date: Tue, 7 Mar 2023 09:58:35 -0800 Subject: [PATCH 5/6] Review --- .../src/main/java/io/deephaven/api/TableOperations.java | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/table-api/src/main/java/io/deephaven/api/TableOperations.java b/table-api/src/main/java/io/deephaven/api/TableOperations.java index eb9fa4a75b0..ffaa164abf4 100644 --- a/table-api/src/main/java/io/deephaven/api/TableOperations.java +++ b/table-api/src/main/java/io/deephaven/api/TableOperations.java @@ -1302,27 +1302,30 @@ TOPS updateBy(UpdateByControl control, Collection o // ------------------------------------------------------------------------------------------- /** - * Drop the {@code columnNames} from {@code this} table. + * Creates a new table without the {@code columnNames} from {@code this}. * * @param columnNames the columns to drop * @return the table */ + @ConcurrentMethod TOPS dropColumns(String... columnNames); /** - * Drop the {@code columnNames} from {@code this} table. + * Creates a new table without the {@code columnNames} from {@code this}. * * @param columnNames the columns to drop * @return the table */ + @ConcurrentMethod TOPS dropColumns(Collection columnNames); /** - * Drop the {@code columnNames} from {@code this} table. + * Creates a new table without the {@code columnNames} from {@code this}. * * @param columnNames the columns to drop * @return the table */ + @ConcurrentMethod TOPS dropColumns(ColumnName... columnNames); // ------------------------------------------------------------------------------------------- From bc07eca957f7843f12b419952d55a921b3ea09cf Mon Sep 17 00:00:00 2001 From: Devin Smith Date: Wed, 8 Mar 2023 11:41:09 -0800 Subject: [PATCH 6/6] Make dropColumns with null or empty a no-op --- .../engine/table/impl/QueryTable.java | 38 +++++-------------- .../deephaven/qst/table/DropColumnsTable.java | 7 ---- 2 files changed, 10 insertions(+), 35 deletions(-) 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 a3f6d696988..1b20a93dd0d 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 @@ -1106,10 +1106,7 @@ public Table flatten() { } if (isFlat()) { - if (isRefreshing()) { - manageWithCurrentScope(); - } - return this; + return prepareReturnThis(); } return getResult(new FlattenOperation(this)); @@ -1342,10 +1339,7 @@ private void propagateGrouping(SelectColumn[] selectColumns, QueryTable resultTa @Override public Table view(final Collection viewColumns) { if (viewColumns == null || viewColumns.isEmpty()) { - if (isRefreshing()) { - manageWithCurrentScope(); - } - return this; + return prepareReturnThis(); } return viewOrUpdateView(Flavor.View, SelectColumn.from(viewColumns)); } @@ -1357,10 +1351,7 @@ public Table updateView(final Collection viewColumns) { private Table viewOrUpdateView(Flavor flavor, final SelectColumn... viewColumns) { if (viewColumns == null || viewColumns.length == 0) { - if (isRefreshing()) { - manageWithCurrentScope(); - } - return this; + return prepareReturnThis(); } final String humanReadablePrefix = flavor.toString(); @@ -1477,6 +1468,9 @@ public Table lazyUpdate(final Collection newColumns) { @Override public Table dropColumns(String... columnNames) { + if (columnNames == null || columnNames.length == 0) { + return prepareReturnThis(); + } return memoizeResult(MemoizedOperationKey.dropColumns(columnNames), () -> QueryPerformanceRecorder .withNugget("dropColumns(" + Arrays.toString(columnNames) + ")", sizeForInstrumentation(), () -> { final Mutable result = new MutableObject<>(); @@ -1547,10 +1541,7 @@ public Table renameColumns(MatchPair... pairs) { return QueryPerformanceRecorder.withNugget("renameColumns(" + matchString(pairs) + ")", sizeForInstrumentation(), () -> { if (pairs == null || pairs.length == 0) { - if (isRefreshing()) { - manageWithCurrentScope(); - } - return this; + return prepareReturnThis(); } checkInitiateOperation(); @@ -2199,18 +2190,12 @@ static ColumnSource maybeTransformToPrimitive(final ColumnSource columnSou public Table sort(Collection columnsToSortBy) { final SortPair[] sortPairs = SortPair.from(columnsToSortBy); if (sortPairs.length == 0) { - if (isRefreshing()) { - manageWithCurrentScope(); - } - return this; + return prepareReturnThis(); } else if (sortPairs.length == 1) { final String columnName = sortPairs[0].getColumn(); final SortingOrder order = sortPairs[0].getOrder(); if (SortedColumnsAttribute.isSortedBy(this, columnName, order)) { - if (isRefreshing()) { - manageWithCurrentScope(); - } - return this; + return prepareReturnThis(); } } @@ -2263,10 +2248,7 @@ public Table ungroup(boolean nullFill, Collection columnsT return QueryPerformanceRecorder.withNugget("ungroup(" + Arrays.toString(columnsToUngroupBy) + ")", sizeForInstrumentation(), () -> { if (columnsToUngroupBy.length == 0) { - if (isRefreshing()) { - manageWithCurrentScope(); - } - return this; + return prepareReturnThis(); } checkInitiateOperation(); diff --git a/qst/src/main/java/io/deephaven/qst/table/DropColumnsTable.java b/qst/src/main/java/io/deephaven/qst/table/DropColumnsTable.java index df1d13cdaa6..6b9bc746722 100644 --- a/qst/src/main/java/io/deephaven/qst/table/DropColumnsTable.java +++ b/qst/src/main/java/io/deephaven/qst/table/DropColumnsTable.java @@ -29,13 +29,6 @@ public final V walk(V visitor) { return visitor; } - @Check - final void checkNonEmpty() { - if (dropColumns().isEmpty()) { - throw new IllegalArgumentException("Expected dropColumns() to be non-empty"); - } - } - public interface Builder { Builder parent(TableSpec parent);