Fix bugs in moveColumns and renameColumns

engine/api/src/main/java/io/deephaven/engine/table/Table.java

@@ -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
@@ -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)
@@ -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
     // -----------------------------------------------------------------------------------------------------------------

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<Pair> renamedColumns) {
         final Collection<String> 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<Pair> renamedColumns) {
         // noinspection unchecked
         final Map<String, String> oldDescriptions =
                 (Map<String, String>) getAttribute(Table.COLUMN_DESCRIPTIONS_ATTRIBUTE);
@@ -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());
                 }
             }
         }