LuciferYang commented on code in PR #13448:
URL: https://github.com/apache/gravitino/pull/13448#discussion_r4082455655


##########
core/src/main/java/org/apache/gravitino/catalog/ManagedTableOperations.java:
##########
@@ -407,7 +407,14 @@ private List<ColumnEntity> applyColumnChanges(
                 newPosition,
                 newNullable,
                 newAutoIncrement);
-        newColumns.add(newColumn.position(), newColumn);
+        if (newPosition.isPresent()) {
+          newColumns.add(newColumn.position(), newColumn);
+        } else {
+          // Stored positions go stale as sibling changes in the same alter 
add or
+          // remove columns, so without an explicit position change the column 
goes
+          // back to the list index it was removed from.
+          newColumns.add(i, newColumn);
+        }

Review Comment:
   Done. Renamed the loop index to `removedIndex` (declared at the search loop 
and used at the re-insertion), so its meaning at the 
`newColumns.add(removedIndex, newColumn)` site is explicit.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to