renameFunction);
@ConcurrentMethod
@@ -343,27 +389,56 @@ public interface Table extends
/**
* Produce a new table with the specified columns moved to the leftmost position. Columns can be renamed with the
- * usual syntax, i.e. {@code "NewColumnName=OldColumnName")}.
+ * usual syntax, i.e. {@code "NewColumnName=OldColumnName")}. The renames are simultaneous and unordered, enabling
+ * direct swaps between column names. All other columns are left in their original order.
+ *
+ * {@link IllegalArgumentException} will be thrown:
+ *
+ * - if a source column does not exist
+ * - if a source column is used more than once
+ * - if a destination column is used more than once
+ *
*
* @param columnsToMove The columns to move to the left (and, optionally, to rename)
- * @return The new table, with the columns rearranged as explained above {@link #moveColumns(int, String...)}
+ * @return The new table, with the columns rearranged as explained above
*/
@ConcurrentMethod
Table moveColumnsUp(String... columnsToMove);
/**
* Produce a new table with the specified columns moved to the rightmost position. Columns can be renamed with the
- * usual syntax, i.e. {@code "NewColumnName=OldColumnName")}.
+ * usual syntax, i.e. {@code "NewColumnName=OldColumnName")}. The renames are simultaneous and unordered, enabling
+ * direct swaps between column names. All other columns are left in their original order.
+ *
+ * {@link IllegalArgumentException} will be thrown:
+ *
+ * - if a source column does not exist
+ * - if a source column is used more than once
+ * - if a destination column is used more than once
+ *
*
* @param columnsToMove The columns to move to the right (and, optionally, to rename)
- * @return The new table, with the columns rearranged as explained above {@link #moveColumns(int, String...)}
+ * @return The new table, with the columns rearranged as explained above
*/
@ConcurrentMethod
Table moveColumnsDown(String... columnsToMove);
/**
* Produce a new table with the specified columns moved to the specified {@code index}. Column indices begin at 0.
- * Columns can be renamed with the usual syntax, i.e. {@code "NewColumnName=OldColumnName")}.
+ * Columns can be renamed with the usual syntax, i.e. {@code "NewColumnName=OldColumnName")}. The renames are
+ * simultaneous and unordered, enabling direct swaps between column names. The resulting table retains the original
+ * column ordering except for the specified columns, which are inserted at the specified index, in the order of
+ * {@code columnsToMove}, after the effects of applying any renames.
+ *
+ * {@link IllegalArgumentException} will be thrown:
+ *
+ * - if a source column does not exist
+ * - if a source column is used more than once
+ * - if a destination column is used more than once
+ *
+ *
+ * Values of {@code index} outside the range of 0 to the number of columns in the table (exclusive) will be clamped
+ * to the nearest valid index.
*
* @param index The index to which the specified columns should be moved
* @param columnsToMove The columns to move to the specified index (and, optionally, to rename)
@@ -372,9 +447,6 @@ public interface Table extends
@ConcurrentMethod
Table moveColumns(int index, String... columnsToMove);
- @ConcurrentMethod
- Table moveColumns(int index, boolean moveToEnd, String... columnsToMove);
-
// -----------------------------------------------------------------------------------------------------------------
// Slice Operations
// -----------------------------------------------------------------------------------------------------------------
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 412ec955bc2..d56c8e0096a 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
@@ -5,6 +5,7 @@
import com.google.common.collect.BiMap;
import com.google.common.collect.HashBiMap;
+import io.deephaven.api.Pair;
import io.deephaven.base.Base64;
import io.deephaven.base.log.LogOutput;
import io.deephaven.base.reference.SimpleReference;
@@ -1034,7 +1035,7 @@ public void copySortableColumns(
destination.setSortableColumns(currentSortableColumns.stream().filter(shouldCopy).collect(Collectors.toList()));
}
- void copySortableColumns(BaseTable destination, MatchPair[] renamedColumns) {
+ void copySortableColumns(BaseTable destination, Collection renamedColumns) {
final Collection currentSortableColumns = getSortableColumns();
if (currentSortableColumns == null) {
return;
@@ -1047,9 +1048,9 @@ void copySortableColumns(BaseTable destination, MatchPair[] renamedColumns) {
// b) The original column exists, and has not been replaced by another. For example
// T1 = [ Col1, Col2, Col3 ]; T1.renameColumns(Col1=Col3, Col2];
if (renamedColumns != null) {
- for (MatchPair mp : renamedColumns) {
+ for (final Pair pair : renamedColumns) {
// Only the last grouping matters.
- columnMapping.forcePut(mp.leftColumn(), mp.rightColumn());
+ columnMapping.forcePut(pair.output().name(), pair.input().name());
}
}
@@ -1158,7 +1159,9 @@ void maybeCopyColumnDescriptions(final BaseTable destination) {
* @param destination the table which shall possibly have a column-description attribute created
* @param renamedColumns an array of the columns which have been renamed
*/
- void maybeCopyColumnDescriptions(final BaseTable destination, final MatchPair[] renamedColumns) {
+ void maybeCopyColumnDescriptions(
+ final BaseTable destination,
+ final Collection renamedColumns) {
// noinspection unchecked
final Map oldDescriptions =
(Map) getAttribute(Table.COLUMN_DESCRIPTIONS_ATTRIBUTE);
@@ -1168,11 +1171,13 @@ void maybeCopyColumnDescriptions(final BaseTable destination, final MatchPair
}
final Map sourceDescriptions = new HashMap<>(oldDescriptions);
- if (renamedColumns != null && renamedColumns.length != 0) {
- for (final MatchPair mp : renamedColumns) {
- final String desc = sourceDescriptions.remove(mp.rightColumn());
+ if (renamedColumns != null) {
+ for (final Pair pair : renamedColumns) {
+ final String desc = sourceDescriptions.remove(pair.input().name());
if (desc != null) {
- sourceDescriptions.put(mp.leftColumn(), desc);
+ sourceDescriptions.put(pair.output().name(), desc);
+ } else {
+ sourceDescriptions.remove(pair.output().name());
}
}
}
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 72c4d20aff0..ba1f33dae58 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
@@ -3,16 +3,7 @@
*/
package io.deephaven.engine.table.impl;
-import io.deephaven.UncheckedDeephavenException;
-import io.deephaven.api.AsOfJoinMatch;
-import io.deephaven.api.AsOfJoinRule;
-import io.deephaven.api.ColumnName;
-import io.deephaven.api.JoinAddition;
-import io.deephaven.api.JoinMatch;
-import io.deephaven.api.RangeJoinMatch;
-import io.deephaven.api.Selectable;
-import io.deephaven.api.SortColumn;
-import io.deephaven.api.Strings;
+import io.deephaven.api.*;
import io.deephaven.api.agg.*;
import io.deephaven.api.agg.spec.AggSpec;
import io.deephaven.api.agg.spec.AggSpecColumnReferences;
@@ -21,7 +12,6 @@
import io.deephaven.api.snapshot.SnapshotWhenOptions.Flag;
import io.deephaven.api.updateby.UpdateByOperation;
import io.deephaven.api.updateby.UpdateByControl;
-import io.deephaven.base.Pair;
import io.deephaven.base.verify.Assert;
import io.deephaven.base.verify.Require;
import io.deephaven.chunk.attributes.Values;
@@ -47,8 +37,6 @@
import io.deephaven.engine.table.impl.perf.BasePerformanceEntry;
import io.deephaven.engine.table.impl.perf.QueryPerformanceNugget;
import io.deephaven.engine.table.impl.rangejoin.RangeJoinOperation;
-import io.deephaven.engine.table.impl.select.MatchPairFactory;
-import io.deephaven.engine.table.impl.select.SelectColumnFactory;
import io.deephaven.engine.table.impl.updateby.UpdateBy;
import io.deephaven.engine.table.impl.select.analyzers.SelectAndViewAnalyzerWrapper;
import io.deephaven.engine.table.impl.sources.ring.RingTableTools;
@@ -79,6 +67,7 @@
import io.deephaven.util.annotations.TestUseOnly;
import io.deephaven.util.annotations.VisibleForTesting;
import org.apache.commons.lang3.mutable.Mutable;
+import org.apache.commons.lang3.mutable.MutableInt;
import org.apache.commons.lang3.mutable.MutableObject;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;
@@ -90,6 +79,7 @@
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.atomic.AtomicReferenceFieldUpdater;
+import java.util.function.Consumer;
import java.util.function.Function;
import java.util.function.Predicate;
import java.util.function.Supplier;
@@ -475,8 +465,9 @@ public ModifiedColumnSet.Transformer newModifiedColumnSetTransformer(QueryTable
* @param matchPairs the columns that map one-to-one with the result table
* @return a transformer that passes dirty details via an identity mapping
*/
- public ModifiedColumnSet.Transformer newModifiedColumnSetTransformer(QueryTable resultTable,
- MatchPair... matchPairs) {
+ public ModifiedColumnSet.Transformer newModifiedColumnSetTransformer(
+ @NotNull final QueryTable resultTable,
+ @NotNull final MatchPair... matchPairs) {
final ModifiedColumnSet[] columnSets = new ModifiedColumnSet[matchPairs.length];
for (int ii = 0; ii < matchPairs.length; ++ii) {
columnSets[ii] = resultTable.newModifiedColumnSet(matchPairs[ii].leftColumn());
@@ -484,6 +475,27 @@ public ModifiedColumnSet.Transformer newModifiedColumnSetTransformer(QueryTable
return newModifiedColumnSetTransformer(MatchPair.getRightColumns(matchPairs), columnSets);
}
+ /**
+ * Create a {@link ModifiedColumnSet.Transformer} that can be used to propagate dirty columns from this table to
+ * listeners of the provided resultTable.
+ *
+ * @param resultTable the destination table
+ * @param pairs the columns that map one-to-one with the result table
+ * @return a transformer that passes dirty details via an identity mapping
+ */
+ public ModifiedColumnSet.Transformer newModifiedColumnSetTransformer(
+ @NotNull final QueryTable resultTable,
+ @NotNull final Pair... pairs) {
+ return newModifiedColumnSetTransformer(
+ Arrays.stream(pairs)
+ .map(Pair::output)
+ .map(ColumnName::name)
+ .toArray(String[]::new),
+ Arrays.stream(pairs)
+ .map(pair -> resultTable.newModifiedColumnSet(pair.input().name()))
+ .toArray(ModifiedColumnSet[]::new));
+ }
+
/**
* Create a {@link ModifiedColumnSet.Transformer} that can be used to propagate dirty columns from this table to
* listeners of the table used to construct columnSets. It is an error if {@code columnNames} and {@code columnSets}
@@ -924,54 +936,11 @@ private String getCastFormulaInternal(Class dataType) {
}
@Override
- public Table moveColumns(int index, boolean moveToEnd, String... columnsToMove) {
+ public Table moveColumns(final int index, String... columnsToMove) {
final UpdateGraph updateGraph = getUpdateGraph();
try (final SafeCloseable ignored = ExecutionContext.getContext().withUpdateGraph(updateGraph).open()) {
- // Get the current columns
- final List currentColumns = getDefinition().getColumnNames();
-
- // Create a Set from columnsToMove. This way, we can rename and rearrange columns at once.
- final Set leftColsToMove = new HashSet<>();
- final Set rightColsToMove = new HashSet<>();
- int extraCols = 0;
-
- for (final String columnToMove : columnsToMove) {
- final String left = MatchPairFactory.getExpression(columnToMove).leftColumn;
- final String right = MatchPairFactory.getExpression(columnToMove).rightColumn;
-
- if (!leftColsToMove.add(left) || !currentColumns.contains(left) || (rightColsToMove.contains(left)
- && !left.equals(right) && leftColsToMove.stream().anyMatch(col -> col.equals(right)))) {
- extraCols++;
- }
- if (currentColumns.stream().anyMatch(currentColumn -> currentColumn.equals(right))
- && !left.equals(right)
- && rightColsToMove.add(right) && !rightColsToMove.contains(left)) {
- extraCols--;
- }
- }
- index += moveToEnd ? extraCols : 0;
-
- // vci for write, cci for currentColumns, ctmi for columnsToMove
- final SelectColumn[] viewColumns = new SelectColumn[currentColumns.size() + extraCols];
- for (int vci = 0, cci = 0, ctmi = 0; vci < viewColumns.length;) {
- if (vci >= index && ctmi < columnsToMove.length) {
- viewColumns[vci++] = SelectColumnFactory.getExpression(columnsToMove[ctmi++]);
- } else {
- // Don't add the column if it's one of the columns we're moving or if it has been renamed.
- final String currentColumn = currentColumns.get(cci++);
- if (!leftColsToMove.contains(currentColumn)
- && Arrays.stream(viewColumns).noneMatch(
- viewCol -> viewCol != null
- && viewCol.getMatchPair().leftColumn.equals(currentColumn))
- && Arrays.stream(columnsToMove)
- .noneMatch(colToMove -> MatchPairFactory.getExpression(colToMove).rightColumn
- .equals(currentColumn))) {
-
- viewColumns[vci++] = SelectColumnFactory.getExpression(currentColumn);
- }
- }
- }
- return viewOrUpdateView(Flavor.View, viewColumns);
+ return renameColumnsImpl("moveColumns(" + index + ", ", Math.max(0, index),
+ Pair.from(columnsToMove));
}
}
@@ -1217,7 +1186,7 @@ private QueryTable whereInternal(final WhereFilter... filters) {
}
List selectFilters = new LinkedList<>();
- List>>> shiftColPairs = new LinkedList<>();
+ List>>> shiftColPairs = new LinkedList<>();
for (final WhereFilter filter : filters) {
filter.init(getDefinition());
if (filter instanceof AbstractConditionFilter
@@ -1817,76 +1786,139 @@ public void onUpdate(final TableUpdate upstream) {
}
@Override
- public Table renameColumns(Collection pairs) {
+ public Table renameColumns(Collection pairs) {
final UpdateGraph updateGraph = getUpdateGraph();
try (final SafeCloseable ignored = ExecutionContext.getContext().withUpdateGraph(updateGraph).open()) {
- return renameColumnsImpl(MatchPair.fromPairs(pairs));
+ return renameColumnsImpl("renameColumns(", -1, pairs);
}
}
- private Table renameColumnsImpl(MatchPair... pairs) {
- return QueryPerformanceRecorder.withNugget("renameColumns(" + matchString(pairs) + ")",
+ private Table renameColumnsImpl(
+ @NotNull final String methodNuggetPrefix,
+ final int movePosition,
+ @NotNull final Collection pairs) {
+ final String pairsLogString = Strings.ofPairs(pairs);
+ return QueryPerformanceRecorder.withNugget(methodNuggetPrefix + pairsLogString + ")",
sizeForInstrumentation(), () -> {
- if (pairs == null || pairs.length == 0) {
+ if (pairs.isEmpty()) {
return prepareReturnThis();
}
- checkInitiateOperation();
+ Set notFound = null;
+ Set duplicateSource = null;
+ Set duplicateDest = null;
- final Map pairLookup = new HashMap<>();
- for (final MatchPair pair : pairs) {
- if (pair.leftColumn == null || pair.leftColumn.equals("")) {
- throw new IllegalArgumentException(
- "Bad left column in rename pair \"" + pair + "\"");
+ final Set newNames = new HashSet<>();
+ final Map pairLookup = new LinkedHashMap<>();
+ for (final Pair pair : pairs) {
+ if (!columns.containsKey(pair.input().name())) {
+ (notFound == null ? notFound = new LinkedHashSet<>() : notFound)
+ .add(pair.input().name());
}
- if (null == columns.get(pair.rightColumn)) {
- throw new IllegalArgumentException("Column \"" + pair.rightColumn + "\" not found");
+ if (pairLookup.put(pair.input(), pair.output()) != null) {
+ (duplicateSource == null ? duplicateSource = new LinkedHashSet<>(1) : duplicateSource)
+ .add(pair.input().name());
+ }
+ if (!newNames.add(pair.output())) {
+ (duplicateDest == null ? duplicateDest = new LinkedHashSet<>() : duplicateDest)
+ .add(pair.output().name());
}
- pairLookup.put(pair.rightColumn, pair.leftColumn);
}
- int mcsPairIdx = 0;
- final MatchPair[] modifiedColumnSetPairs = new MatchPair[columns.size()];
+ // if we accumulated any errors, build one mega error message and throw it
+ if (notFound != null || duplicateSource != null || duplicateDest != null) {
+ throw new IllegalArgumentException(Stream.of(
+ notFound == null ? null : "Column(s) not found: " + String.join(", ", notFound),
+ duplicateSource == null ? null
+ : "Duplicate source column(s): " + String.join(", ", duplicateSource),
+ duplicateDest == null ? null
+ : "Duplicate destination column(s): " + String.join(", ", duplicateDest))
+ .filter(Objects::nonNull).collect(Collectors.joining("\n")));
+ }
+ final MutableInt mcsPairIdx = new MutableInt();
+ final Pair[] modifiedColumnSetPairs = new Pair[columns.size()];
final Map> newColumns = new LinkedHashMap<>();
+
+ final Runnable moveColumns = () -> {
+ for (final Map.Entry rename : pairLookup.entrySet()) {
+ final ColumnName oldName = rename.getKey();
+ final ColumnName newName = rename.getValue();
+ final ColumnSource columnSource = columns.get(oldName.name());
+ newColumns.put(newName.name(), columnSource);
+ modifiedColumnSetPairs[mcsPairIdx.getAndIncrement()] =
+ Pair.of(newName, oldName);
+ }
+ };
+
for (final Map.Entry> entry : columns.entrySet()) {
- final String oldName = entry.getKey();
+ final ColumnName oldName = ColumnName.of(entry.getKey());
final ColumnSource columnSource = entry.getValue();
- String newName = pairLookup.get(oldName);
+ ColumnName newName = pairLookup.get(oldName);
if (newName == null) {
+ if (newNames.contains(oldName)) {
+ // this column is being replaced by a rename
+ continue;
+ }
newName = oldName;
+ } else if (movePosition >= 0) {
+ // we move this column when we get to the right position
+ continue;
}
- modifiedColumnSetPairs[mcsPairIdx++] = new MatchPair(newName, oldName);
- newColumns.put(newName, columnSource);
+
+ if (mcsPairIdx.intValue() == movePosition) {
+ moveColumns.run();
+ }
+
+ modifiedColumnSetPairs[mcsPairIdx.getAndIncrement()] =
+ Pair.of(newName, oldName);
+ newColumns.put(newName.name(), columnSource);
}
- final QueryTable queryTable = new QueryTable(rowSet, newColumns);
- if (isRefreshing()) {
- final ModifiedColumnSet.Transformer mcsTransformer =
- newModifiedColumnSetTransformer(queryTable, modifiedColumnSetPairs);
- addUpdateListener(new ListenerImpl("renameColumns(" + Arrays.deepToString(pairs) + ')',
- this, queryTable) {
- @Override
- public void onUpdate(final TableUpdate upstream) {
- final TableUpdateImpl downstream = TableUpdateImpl.copy(upstream);
- downstream.modifiedColumnSet = queryTable.getModifiedColumnSetForUpdates();
- if (upstream.modified().isNonempty()) {
- mcsTransformer.clearAndTransform(upstream.modifiedColumnSet(),
- downstream.modifiedColumnSet);
- } else {
- downstream.modifiedColumnSet.clear();
- }
- queryTable.notifyListeners(downstream);
- }
- });
+ if (mcsPairIdx.intValue() <= movePosition) {
+ moveColumns.run();
}
- propagateFlatness(queryTable);
- copyAttributes(queryTable, CopyAttributeOperation.RenameColumns);
- copySortableColumns(queryTable, pairs);
- maybeCopyColumnDescriptions(queryTable, pairs);
+ final Mutable result = new MutableObject<>();
+
+ final OperationSnapshotControl snapshotControl =
+ createSnapshotControlIfRefreshing(OperationSnapshotControl::new);
+ initializeWithSnapshot("renameColumns", snapshotControl, (usePrev, beforeClockValue) -> {
+ final QueryTable resultTable = new QueryTable(rowSet, newColumns);
+ propagateFlatness(resultTable);
+
+ copyAttributes(resultTable, CopyAttributeOperation.RenameColumns);
+ copySortableColumns(resultTable, pairs);
+ maybeCopyColumnDescriptions(resultTable, pairs);
+
+ if (snapshotControl != null) {
+ final ModifiedColumnSet.Transformer mcsTransformer =
+ newModifiedColumnSetTransformer(resultTable, modifiedColumnSetPairs);
+ final ListenerImpl listener = new ListenerImpl(
+ methodNuggetPrefix + pairsLogString + ')', this, resultTable) {
+ @Override
+ public void onUpdate(final TableUpdate upstream) {
+ final TableUpdateImpl downstream = TableUpdateImpl.copy(upstream);
+ if (upstream.modified().isNonempty()) {
+ downstream.modifiedColumnSet = resultTable.getModifiedColumnSetForUpdates();
+ mcsTransformer.clearAndTransform(upstream.modifiedColumnSet(),
+ downstream.modifiedColumnSet);
+ } else {
+ downstream.modifiedColumnSet = ModifiedColumnSet.EMPTY;
+ }
+ resultTable.notifyListeners(downstream);
+ }
+ };
+ snapshotControl.setListenerAndResult(listener, resultTable);
+ }
+
+ result.setValue(resultTable);
+
+ return true;
+ });
+
+ return result.getValue();
- return queryTable;
});
}
diff --git a/engine/table/src/main/java/io/deephaven/engine/table/impl/TableAdapter.java b/engine/table/src/main/java/io/deephaven/engine/table/impl/TableAdapter.java
index 4b25f59be2a..c724ad397bb 100644
--- a/engine/table/src/main/java/io/deephaven/engine/table/impl/TableAdapter.java
+++ b/engine/table/src/main/java/io/deephaven/engine/table/impl/TableAdapter.java
@@ -219,7 +219,7 @@ default Table renameColumns(Collection pairs) {
}
@Override
- default Table moveColumns(int index, boolean moveToEnd, String... columnsToMove) {
+ default Table moveColumns(int index, String... columnsToMove) {
return throwUnsupported();
}
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 836ca71260e..f4e251f227f 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
@@ -243,14 +243,7 @@ default Table moveColumnsUp(String... columnsToMove) {
@ConcurrentMethod
@FinalDefault
default Table moveColumnsDown(String... columnsToMove) {
- return moveColumns(numColumns() - columnsToMove.length, true, columnsToMove);
- }
-
- @Override
- @ConcurrentMethod
- @FinalDefault
- default Table moveColumns(int index, String... columnsToMove) {
- return moveColumns(index, false, columnsToMove);
+ return moveColumns(numColumns() - columnsToMove.length, columnsToMove);
}
// -----------------------------------------------------------------------------------------------------------------
diff --git a/engine/table/src/main/java/io/deephaven/engine/table/impl/UncoalescedTable.java b/engine/table/src/main/java/io/deephaven/engine/table/impl/UncoalescedTable.java
index 4599d3cdd4b..23c827532e1 100644
--- a/engine/table/src/main/java/io/deephaven/engine/table/impl/UncoalescedTable.java
+++ b/engine/table/src/main/java/io/deephaven/engine/table/impl/UncoalescedTable.java
@@ -271,8 +271,8 @@ public Table renameColumns(Collection pairs) {
@Override
@ConcurrentMethod
- public Table moveColumns(int index, boolean moveToEnd, String... columnsToMove) {
- return coalesce().moveColumns(index, moveToEnd, columnsToMove);
+ public Table moveColumns(int index, String... columnsToMove) {
+ return coalesce().moveColumns(index, columnsToMove);
}
@Override
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 58ef8ac5996..4d8b0708dec 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
@@ -664,6 +664,25 @@ public void testRenameColumns() {
fail("Expected exception");
} catch (RuntimeException ignored) {
}
+
+ // Can't rename to a dest column twice.
+ Assert.assertThrows(IllegalArgumentException.class, () -> {
+ table.renameColumns("O = Int", "O = Int");
+ });
+
+ // Check what happens when we override a column by name
+ final Table override = table.renameColumns("Double = Int");
+ Assert.assertEquals(override.getColumnSource("Double").getType(), int.class);
+ Assert.assertFalse(override.getColumnSourceMap().containsKey("Int"));
+ // Check that ordering of source columns does not matter
+ final Table override2 = table.renameColumns("Int = Double");
+ Assert.assertEquals(override2.getColumnSource("Int").getType(), double.class);
+ Assert.assertFalse(override2.getColumnSourceMap().containsKey("Double"));
+
+ // Validate that we can swap two columns simultaneously
+ final Table swapped = table.renameColumns("Double = Int", "Int = Double");
+ Assert.assertEquals(swapped.getColumnSource("Double").getType(), int.class);
+ Assert.assertEquals(swapped.getColumnSource("Int").getType(), double.class);
}
public void testRenameColumnsIncremental() {
@@ -680,14 +699,9 @@ public void testRenameColumnsIncremental() {
final EvalNugget[] en = new EvalNugget[] {
EvalNugget.from(() -> queryTable.renameColumns(List.of())),
EvalNugget.from(() -> queryTable.renameColumns("Symbol=Sym")),
- EvalNugget.from(() -> queryTable.renameColumns("Symbol=Sym", "Symbols=Sym")),
EvalNugget.from(() -> queryTable.renameColumns("Sym2=Sym", "intCol2=intCol", "doubleCol2=doubleCol")),
};
- // Verify our assumption that columns can be renamed at most once.
- Assert.assertTrue(queryTable.renameColumns("Symbol=Sym", "Symbols=Sym").hasColumns("Symbols"));
- Assert.assertFalse(queryTable.renameColumns("Symbol=Sym", "Symbols=Sym").hasColumns("Symbol"));
-
final int steps = 100;
for (int i = 0; i < steps; i++) {
if (printTableUpdates) {
@@ -697,6 +711,175 @@ public void testRenameColumnsIncremental() {
}
}
+ public void testMoveColumnsUp() {
+ final Table table = emptyTable(1).update("A = 1", "B = 2", "C = 3", "D = 4", "E = 5");
+
+ assertTableEquals(
+ emptyTable(1).update("C = 3", "A = 1", "B = 2", "D = 4", "E = 5"),
+ table.moveColumnsUp("C"));
+
+ assertTableEquals(
+ emptyTable(1).update("C = 3", "B = 2", "A = 1", "D = 4", "E = 5"),
+ table.moveColumnsUp("C", "B"));
+
+ assertTableEquals(
+ emptyTable(1).update("D = 4", "A = 1", "C = 3", "B = 2", "E = 5"),
+ table.moveColumnsUp("D", "A", "C"));
+
+ // test trivial do nothing case
+ assertTableEquals(table, table.moveColumnsUp());
+
+ // Can't move a column up twice.
+ Assert.assertThrows(IllegalArgumentException.class, () -> {
+ table.moveColumnsUp("C", "C");
+ });
+
+ // Can't rename a source column twice.
+ Assert.assertThrows(IllegalArgumentException.class, () -> {
+ table.moveColumnsUp("A1 = A", "A2 = A");
+ });
+
+ // Can't rename to a dest column twice.
+ Assert.assertThrows(IllegalArgumentException.class, () -> {
+ table.moveColumnsUp("O = A", "O = B");
+ });
+
+ assertTableEquals(
+ emptyTable(1).update("B = 3", "A = 1", "D = 4", "E = 5"),
+ table.moveColumnsUp("B = C"));
+ assertTableEquals(
+ emptyTable(1).update("B = 1", "C = 3", "D = 4", "E = 5"),
+ table.moveColumnsUp("B = A"));
+ assertTableEquals(
+ emptyTable(1).update("B = 1", "A = 2", "C = 3", "D = 4", "E = 5"),
+ table.moveColumnsUp("B = A", "A = B"));
+ }
+
+ public void testMoveColumnsDown() {
+ final Table table = emptyTable(1).update("A = 1", "B = 2", "C = 3", "D = 4", "E = 5");
+
+ assertTableEquals(
+ emptyTable(1).update("A = 1", "B = 2", "D = 4", "E = 5", "C = 3"),
+ table.moveColumnsDown("C"));
+
+ assertTableEquals(
+ emptyTable(1).update("A = 1", "D = 4", "E = 5", "C = 3", "B = 2"),
+ table.moveColumnsDown("C", "B"));
+
+ assertTableEquals(
+ emptyTable(1).update("B = 2", "E = 5", "D = 4", "A = 1", "C = 3"),
+ table.moveColumnsDown("D", "A", "C"));
+
+ // test trivial do nothing case
+ assertTableEquals(table, table.moveColumnsDown());
+
+ // Can't move a column down twice.
+ Assert.assertThrows(IllegalArgumentException.class, () -> {
+ table.moveColumnsDown("C", "C");
+ });
+
+ // Can't rename a source column twice.
+ Assert.assertThrows(IllegalArgumentException.class, () -> {
+ table.moveColumnsDown("A1 = A", "A2 = A");
+ });
+
+ // Can't rename to a dest column twice.
+ Assert.assertThrows(IllegalArgumentException.class, () -> {
+ table.moveColumnsDown("O = A", "O = B");
+ });
+
+ assertTableEquals(
+ emptyTable(1).update("A = 1", "D = 4", "E = 5", "B = 3"),
+ table.moveColumnsDown("B = C"));
+ assertTableEquals(
+ emptyTable(1).update("C = 3", "D = 4", "E = 5", "B = 1"),
+ table.moveColumnsDown("B = A"));
+ assertTableEquals(
+ emptyTable(1).update("C = 3", "D = 4", "E = 5", "B = 1", "A = 2"),
+ table.moveColumnsDown("B = A", "A = B"));
+ }
+
+ public void testMoveColumns() {
+ final Table table = emptyTable(1).update("A = 1", "B = 2", "C = 3", "D = 4", "E = 5");
+
+ // single column
+ assertTableEquals(
+ emptyTable(1).update("C = 3", "A = 1", "B = 2", "D = 4", "E = 5"),
+ table.moveColumns(-1, "C"));
+ assertTableEquals(
+ emptyTable(1).update("C = 3", "A = 1", "B = 2", "D = 4", "E = 5"),
+ table.moveColumns(0, "C"));
+ assertTableEquals(
+ emptyTable(1).update("A = 1", "C = 3", "B = 2", "D = 4", "E = 5"),
+ table.moveColumns(1, "C"));
+ assertTableEquals(
+ emptyTable(1).update("A = 1", "B = 2", "C = 3", "D = 4", "E = 5"),
+ table.moveColumns(2, "C"));
+ assertTableEquals(
+ emptyTable(1).update("A = 1", "B = 2", "D = 4", "C = 3", "E = 5"),
+ table.moveColumns(3, "C"));
+ assertTableEquals(
+ emptyTable(1).update("A = 1", "B = 2", "D = 4", "E = 5", "C = 3"),
+ table.moveColumns(4, "C"));
+ assertTableEquals(
+ emptyTable(1).update("A = 1", "B = 2", "D = 4", "E = 5", "C = 3"),
+ table.moveColumns(10, "C"));
+
+ // two columns
+ assertTableEquals(
+ emptyTable(1).update("C = 3", "B = 2", "A = 1", "D = 4", "E = 5"),
+ table.moveColumns(-1, "C", "B"));
+ assertTableEquals(
+ emptyTable(1).update("C = 3", "B = 2", "A = 1", "D = 4", "E = 5"),
+ table.moveColumns(0, "C", "B"));
+ assertTableEquals(
+ emptyTable(1).update("A = 1", "C = 3", "B = 2", "D = 4", "E = 5"),
+ table.moveColumns(1, "C", "B"));
+ assertTableEquals(
+ emptyTable(1).update("A = 1", "D = 4", "C = 3", "B = 2", "E = 5"),
+ table.moveColumns(2, "C", "B"));
+ assertTableEquals(
+ emptyTable(1).update("A = 1", "D = 4", "E = 5", "C = 3", "B = 2"),
+ table.moveColumns(3, "C", "B"));
+ assertTableEquals(
+ emptyTable(1).update("A = 1", "D = 4", "E = 5", "C = 3", "B = 2"),
+ table.moveColumns(4, "C", "B"));
+ assertTableEquals(
+ emptyTable(1).update("A = 1", "D = 4", "E = 5", "C = 3", "B = 2"),
+ table.moveColumns(10, "C", "B"));
+
+ // test trivial do nothing case
+ for (int ii = -1; ii < 10; ++ii) {
+ assertTableEquals(table, table.moveColumns(ii));
+ }
+
+ // Can't move a column down twice.
+ Assert.assertThrows(IllegalArgumentException.class, () -> {
+ table.moveColumns(2, "C", "C");
+ });
+
+ // Can't rename a source column twice.
+ Assert.assertThrows(IllegalArgumentException.class, () -> {
+ table.moveColumns(2, "A1 = A", "A2 = A");
+ });
+
+ // Can't rename to a dest column twice.
+ Assert.assertThrows(IllegalArgumentException.class, () -> {
+ table.moveColumns(2, "O = A", "O = B");
+ });
+
+
+ assertTableEquals(
+ emptyTable(1).update("A = 1", "D = 4", "B = 3", "E = 5"),
+ table.moveColumns(2, "B = C"));
+ assertTableEquals(
+ emptyTable(1).update("C = 3", "D = 4", "B = 1", "E = 5"),
+ table.moveColumns(2, "B = A"));
+ assertTableEquals(
+ emptyTable(1).update("C = 3", "D = 4", "B = 1", "A = 2", "E = 5"),
+ table.moveColumns(2, "B = A", "A = B"));
+ }
+
public static WhereFilter stringContainsFilter(
String columnName,
String... values) {
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 3edb395a20b..f82f26c7978 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
@@ -69,29 +69,13 @@ public void testMoveColumns() {
checkColumnOrder(temp, "dexab");
checkColumnValueOrder(temp, "45123");
- temp = table.moveColumns(0, "x=a", "b=x");
- checkColumnOrder(temp, "xbcde");
- checkColumnValueOrder(temp, "11345");
-
- temp = table.moveColumns(0, "x=a", "y=a", "z=a");
- checkColumnOrder(temp, "xyzbcde");
- checkColumnValueOrder(temp, "1112345");
+ temp = table.moveColumns(0, "x=a", "d");
+ checkColumnOrder(temp, "xdbce");
+ checkColumnValueOrder(temp, "14235");
temp = table.moveColumns(0, "b=a", "a=b");
checkColumnOrder(temp, "bacde");
- checkColumnValueOrder(temp, "11345");
-
- temp = table.moveColumns(0, "d=c", "d=a", "x=e");
- checkColumnOrder(temp, "dxb");
- checkColumnValueOrder(temp, "152");
-
- temp = table.moveColumns(0, "a=b", "a=c");
- checkColumnOrder(temp, "ade");
- checkColumnValueOrder(temp, "345");
-
- temp = table.moveColumns(0, "a=b", "a=c", "a=d", "a=e");
- checkColumnOrder(temp, "a");
- checkColumnValueOrder(temp, "5");
+ checkColumnValueOrder(temp, "12345");
}
public void testMoveUpColumns() {
@@ -110,14 +94,6 @@ public void testMoveUpColumns() {
temp = table.moveColumnsUp("x=e");
checkColumnOrder(temp, "xabcd");
checkColumnValueOrder(temp, "51234");
-
- temp = table.moveColumnsUp("x=a", "x=b");
- checkColumnOrder(temp, "xcde");
- checkColumnValueOrder(temp, "2345");
-
- temp = table.moveColumnsUp("x=a", "y=a");
- checkColumnOrder(temp, "xybcde");
- checkColumnValueOrder(temp, "112345");
}
public void testMoveDownColumns() {
@@ -134,19 +110,11 @@ public void testMoveDownColumns() {
temp = table.moveColumnsDown("b=a", "a=b", "c");
checkColumnOrder(temp, "debac");
- checkColumnValueOrder(temp, "45113");
-
- temp = table.moveColumnsDown("b=a", "a=b", "c", "d=a");
- checkColumnOrder(temp, "ebacd");
- checkColumnValueOrder(temp, "51131");
-
- temp = table.moveColumnsDown("x=a", "x=b");
- checkColumnOrder(temp, "cdex");
- checkColumnValueOrder(temp, "3452");
+ checkColumnValueOrder(temp, "45123");
- temp = table.moveColumnsDown("x=a", "y=a");
- checkColumnOrder(temp, "bcdexy");
- checkColumnValueOrder(temp, "234511");
+ temp = table.moveColumnsDown("b=a", "a=b", "c");
+ checkColumnOrder(temp, "debac");
+ checkColumnValueOrder(temp, "45123");
}
private void checkColumnOrder(Table t, String expectedOrder) {
diff --git a/py/server/deephaven/table.py b/py/server/deephaven/table.py
index 922e6b3dcd1..5508a543a4b 100644
--- a/py/server/deephaven/table.py
+++ b/py/server/deephaven/table.py
@@ -80,11 +80,11 @@ class NodeType(Enum):
"""An enum of node types for RollupTable"""
AGGREGATED = _JNodeType.Aggregated
"""Nodes at an aggregated (rolled up) level in the RollupTable. An aggregated level is above the constituent (
- leaf) level. These nodes have column names and types that result from applying aggregations on the source table
+ leaf) level. These nodes have column names and types that result from applying aggregations on the source table
of the RollupTable. """
CONSTITUENT = _JNodeType.Constituent
- """Nodes at the leaf level when :meth:`~deephaven.table.Table.rollup` method is called with
- include_constituent=True. The constituent level is the lowest in a rollup table. These nodes have column names
+ """Nodes at the leaf level when :meth:`~deephaven.table.Table.rollup` method is called with
+ include_constituent=True. The constituent level is the lowest in a rollup table. These nodes have column names
and types from the source table of the RollupTable. """
@@ -665,10 +665,12 @@ def drop_columns(self, cols: Union[str, Sequence[str]]) -> Table:
def move_columns(self, idx: int, cols: Union[str, Sequence[str]]) -> Table:
"""The move_columns method creates a new table with specified columns moved to a specific column index value.
+ Columns may be renamed with the same semantics as rename_columns. The renames are simultaneous and unordered,
+ enabling direct swaps between column names. Specifying a source or destination more than once is prohibited.
Args:
idx (int): the column index where the specified columns will be moved in the new table.
- cols (Union[str, Sequence[str]]) : the column name(s)
+ cols (Union[str, Sequence[str]]) : the column name(s) or the column rename expr(s) as "X = Y"
Returns:
a new table
@@ -684,10 +686,12 @@ def move_columns(self, idx: int, cols: Union[str, Sequence[str]]) -> Table:
def move_columns_down(self, cols: Union[str, Sequence[str]]) -> Table:
"""The move_columns_down method creates a new table with specified columns appearing last in order, to the far
- right.
+ right. Columns may be renamed with the same semantics as rename_columns. The renames are simultaneous and
+ unordered, enabling direct swaps between column names. Specifying a source or destination more than once is
+ prohibited.
Args:
- cols (Union[str, Sequence[str]]) : the column name(s)
+ cols (Union[str, Sequence[str]]) : the column name(s) or the column rename expr(s) as "X = Y"
Returns:
a new table
@@ -703,10 +707,12 @@ def move_columns_down(self, cols: Union[str, Sequence[str]]) -> Table:
def move_columns_up(self, cols: Union[str, Sequence[str]]) -> Table:
"""The move_columns_up method creates a new table with specified columns appearing first in order, to the far
- left.
+ left. Columns may be renamed with the same semantics as rename_columns. The renames are simultaneous and
+ unordered, enabling direct swaps between column names. Specifying a source or destination more than once is
+ prohibited.
Args:
- cols (Union[str, Sequence[str]]) : the column name(s)
+ cols (Union[str, Sequence[str]]) : the column name(s) or the column rename expr(s) as "X = Y"
Returns:
a new table
@@ -721,7 +727,9 @@ def move_columns_up(self, cols: Union[str, Sequence[str]]) -> Table:
raise DHError(e, "table move_columns_up operation failed.") from e
def rename_columns(self, cols: Union[str, Sequence[str]]) -> Table:
- """The rename_columns method creates a new table with the specified columns renamed.
+ """The rename_columns method creates a new table with the specified columns renamed. The renames are
+ simultaneous and unordered, enabling direct swaps between column names. Specifying a source or
+ destination more than once is prohibited.
Args:
cols (Union[str, Sequence[str]]) : the column rename expr(s) as "X = Y"
@@ -734,8 +742,7 @@ def rename_columns(self, cols: Union[str, Sequence[str]]) -> Table:
"""
try:
cols = to_sequence(cols)
- with auto_locking_ctx(self):
- return Table(j_table=self.j_table.renameColumns(*cols))
+ return Table(j_table=self.j_table.renameColumns(*cols))
except Exception as e:
raise DHError(e, "table rename_columns operation failed.") from e
diff --git a/py/server/tests/test_update_graph.py b/py/server/tests/test_update_graph.py
index 2cdf0d116cc..4da6db8e058 100644
--- a/py/server/tests/test_update_graph.py
+++ b/py/server/tests/test_update_graph.py
@@ -161,21 +161,6 @@ def test_auto_locking_joins(self):
with self.subTest(op=op):
result_table = left_table.aj(right_table, on="X")
- def test_auto_locking_rename_columns(self):
- with ug.shared_lock(self.test_update_graph):
- test_table = time_table("PT00:00:00.001").update(["X=i", "Y=i%13", "Z=X*Y"])
-
- cols_to_rename = [
- f"{f.name + '_2'} = {f.name}" for f in test_table.columns[::2]
- ]
-
- with self.assertRaises(DHError) as cm:
- result_table = test_table.rename_columns(cols_to_rename)
- self.assertRegex(str(cm.exception), r"IllegalStateException")
-
- ug.auto_locking = True
- result_table = test_table.rename_columns(cols_to_rename)
-
def test_auto_locking_ungroup(self):
with ug.shared_lock(self.test_update_graph):
test_table = time_table("PT00:00:00.001").update(["X=i", "Y=i%13"])
diff --git a/table-api/src/main/java/io/deephaven/api/Strings.java b/table-api/src/main/java/io/deephaven/api/Strings.java
index af285661400..b4c7e478780 100644
--- a/table-api/src/main/java/io/deephaven/api/Strings.java
+++ b/table-api/src/main/java/io/deephaven/api/Strings.java
@@ -116,6 +116,10 @@ public static String of(FilterPattern pattern, boolean invert) {
return (invert ? "!" : "") + inner;
}
+ public static String ofPairs(Collection pairs) {
+ return pairs.stream().map(Strings::of).collect(Collectors.joining(",", "[", "]"));
+ }
+
public static String of(Pair pair) {
if (pair.input().equals(pair.output())) {
return of(pair.output());