Skip to content

Commit

Permalink
Fix bugs in moveColumns and renameColumns (#5193)
Browse files Browse the repository at this point in the history
  • Loading branch information
@nbauernfeind
nbauernfeind authored Feb 29, 2024
1 parent 440e26b commit 9a9938b
Show file tree
Hide file tree
Showing 11 changed files with 453 additions and 204 deletions.
88 changes: 80 additions & 8 deletions engine/api/src/main/java/io/deephaven/engine/table/Table.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -326,10 +326,56 @@ public interface Table extends
@ConcurrentMethod
Table dropColumnFormats();

/**
* Produce a new table with the specified columns renamed using the specified {@link Pair pairs}. The renames are
* simultaneous and unordered, enabling direct swaps between column names. The resulting table retains the original
* column ordering after applying the specified renames.
* <p>
* {@link IllegalArgumentException} will be thrown:
* <ul>
* <li>if a source column does not exist</li>
* <li>if a source column is used more than once</li>
* <li>if a destination column is used more than once</li>
* </ul>
*
* @param pairs The columns to rename
* @return The new table, with the columns renamed
*/
@ConcurrentMethod
Table renameColumns(Collection<Pair> pairs);

/**
* Produce a new table with the specified columns renamed using the syntax {@code "NewColumnName=OldColumnName"}.
* The renames are simultaneous and unordered, enabling direct swaps between column names. The resulting table
* retains the original column ordering after applying the specified renames.
* <p>
* {@link IllegalArgumentException} will be thrown:
* <ul>
* <li>if a source column does not exist</li>
* <li>if a source column is used more than once</li>
* <li>if a destination column is used more than once</li>
* </ul>
*
* @param pairs The columns to rename
* @return The new table, with the columns renamed
*/
@ConcurrentMethod
Table renameColumns(String... pairs);

/**
* Produce a new table with the specified columns renamed using the provided function. The renames are simultaneous
* and unordered, enabling direct swaps between column names. The resulting table retains the original column
* ordering after applying the specified renames.
* <p>
* {@link IllegalArgumentException} will be thrown:
* <ul>
* <li>if a destination column is used more than once</li>
* </ul>
*
* @param renameFunction The function to apply to each column name
* @return The new table, with the columns renamed
*/
@ConcurrentMethod
Table renameAllColumns(UnaryOperator<String> renameFunction);

@ConcurrentMethod
Expand All @@ -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.
* <p>
* {@link IllegalArgumentException} will be thrown:
* <ul>
* <li>if a source column does not exist</li>
* <li>if a source column is used more than once</li>
* <li>if a destination column is used more than once</li>
* </ul>
*
* @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.
* <p>
* {@link IllegalArgumentException} will be thrown:
* <ul>
* <li>if a source column does not exist</li>
* <li>if a source column is used more than once</li>
* <li>if a destination column is used more than once</li>
* </ul>
*
* @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.
* <p>
* {@link IllegalArgumentException} will be thrown:
* <ul>
* <li>if a source column does not exist</li>
* <li>if a source column is used more than once</li>
* <li>if a destination column is used more than once</li>
* </ul>
* <p>
* 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)
Expand All @@ -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
// -----------------------------------------------------------------------------------------------------------------
Expand Down
21 changes: 13 additions & 8 deletions engine/table/src/main/java/io/deephaven/engine/table/impl/BaseTable.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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<Pair> renamedColumns) {
final Collection<String> currentSortableColumns = getSortableColumns();
if (currentSortableColumns == null) {
return;
Expand All @@ -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());
}
}

Expand Down Expand Up @@ -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<Pair> renamedColumns) {
// noinspection unchecked
final Map<String, String> oldDescriptions =
(Map<String, String>) getAttribute(Table.COLUMN_DESCRIPTIONS_ATTRIBUTE);
Expand All @@ -1168,11 +1171,13 @@ void maybeCopyColumnDescriptions(final BaseTable<?> destination, final MatchPair
}
final Map<String, String> 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());
}
}
}
Expand Down
Loading

0 comments on commit 9a9938b

Please sign in to comment.