jerryshao commented on code in PR #9127:
URL: https://github.com/apache/gravitino/pull/9127#discussion_r2733346384


##########
lance/lance-common/src/main/java/org/apache/gravitino/lance/common/ops/gravitino/GravitinoLanceTableOperations.java:
##########
@@ -294,6 +308,72 @@ public DropTableResponse dropTable(String tableId, String 
delimiter) {
     return response;
   }
 
+  @Override
+  public AlterTableDropColumnsResponse alterTableDropColumns(
+      String tableId, String delimiter, AlterTableDropColumnsRequest request) {
+    ObjectIdentifier nsId = ObjectIdentifier.of(tableId, 
Pattern.quote(delimiter));
+    Preconditions.checkArgument(
+        nsId.levels() == 3, "Expected at 3-level namespace but got: %s", 
nsId.levels());
+
+    String catalogName = nsId.levelAtListPos(0);
+    Catalog catalog = 
namespaceWrapper.loadAndValidateLakehouseCatalog(catalogName);
+
+    NameIdentifier tableIdentifier =
+        NameIdentifier.of(nsId.levelAtListPos(1), nsId.levelAtListPos(2));
+
+    TableChange[] changes =
+        request.getColumns().stream()
+            .map(colName -> TableChange.deleteColumn(new String[] {colName}, 
false))
+            .toArray(TableChange[]::new);
+
+    Table table = catalog.asTableCatalog().alterTable(tableIdentifier, 
changes);
+    Long version =
+        
Optional.ofNullable(table.properties().get(LanceConstants.LANCE_TABLE_VERSION))
+            .map(Long::valueOf)
+            .orElse(null);
+    AlterTableDropColumnsResponse alterTableDropColumnsResponse =
+        new AlterTableDropColumnsResponse();
+    alterTableDropColumnsResponse.setVersion(version);
+    return alterTableDropColumnsResponse;
+  }
+
+  @Override
+  public AlterTableAlterColumnsResponse alterTableAlterColumns(
+      String tableId, String delimiter, AlterTableAlterColumnsRequest request) 
{
+    ObjectIdentifier nsId = ObjectIdentifier.of(tableId, 
Pattern.quote(delimiter));
+    Preconditions.checkArgument(
+        nsId.levels() == 3, "Expected at 3-level namespace but got: %s", 
nsId.levels());
+
+    String catalogName = nsId.levelAtListPos(0);
+    Catalog catalog = 
namespaceWrapper.loadAndValidateLakehouseCatalog(catalogName);
+
+    NameIdentifier tableIdentifier =
+        NameIdentifier.of(nsId.levelAtListPos(1), nsId.levelAtListPos(2));
+
+    List<TableChange> changes = buildAlterColumnChanges(request);
+    if (changes.isEmpty()) {
+      throw new IllegalArgumentException("No valid alterations found in the 
request.");
+    }
+    Table table =
+        catalog.asTableCatalog().alterTable(tableIdentifier, 
changes.toArray(new TableChange[0]));
+    Long version =
+        
Optional.ofNullable(table.properties().get(LanceConstants.LANCE_TABLE_VERSION))
+            .map(Long::valueOf)
+            .orElse(null);
+    AlterTableAlterColumnsResponse response = new 
AlterTableAlterColumnsResponse();
+    response.setVersion(version);
+    return response;
+  }
+
+  @Override
+  public AlterTableAddColumnsResponse alterTableAddColumns(
+      String tableId, String delimiter, AlterTableAddColumnsRequest request) {
+    // We need to parse NewColumnTransform to Column, however, 
NewColumnTransform only contains
+    // the name and a string expression.
+    // More please see: https://docs.lancedb.com/api-reference/data/add-columns
+    throw new UnsupportedOperationException("Adding columns is not supported 
yet.");
+  }
+

Review Comment:
   The implementation of these 3 methods are basically the same; can they be 
merged together?



-- 
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