mchades commented on code in PR #9589:
URL: https://github.com/apache/gravitino/pull/9589#discussion_r2668497634


##########
lance/lance-rest-server/src/test/java/org/apache/gravitino/lance/integration/test/LanceRESTServiceIT.java:
##########
@@ -446,19 +445,20 @@ void testCreateEmptyTable() throws ApiException {
 
     // Try to create a table with wrong location should fail

Review Comment:
   Why not address this comment?



##########
lance/lance-common/src/main/java/org/apache/gravitino/lance/common/utils/LanceConstants.java:
##########
@@ -37,5 +37,7 @@ public class LanceConstants {
 
   public static final String LANCE_TABLE_REGISTER = "lance.register";
 
+  public static final String LANCE_TABLE_CREATE_EMPTY = "lance.create-empty";

Review Comment:
   Do you need to add this property definition to the 
`GenericTablePropertiesMetadata`?



##########
catalogs/catalog-lakehouse-generic/src/main/java/org/apache/gravitino/catalog/lakehouse/lance/LanceTableOperations.java:
##########
@@ -263,6 +264,18 @@ Table createTableInternal(
           ident, columns, comment, properties, partitions, distribution, 
sortOrders, indexes);
     }
 
+    // Check whether it's a create empty table operation.
+    boolean createEmpty =
+        
Optional.ofNullable(properties.get(LanceConstants.LANCE_TABLE_CREATE_EMPTY))
+            .map(Boolean::parseBoolean)
+            .orElse(false);
+    if (createEmpty) {
+      // For create empty table, we just create the table metadata in 
Gravitino without creating
+      // the underlying Lance dataset.
+      return super.createTable(
+          ident, columns, comment, properties, partitions, distribution, 
sortOrders, indexes);

Review Comment:
   Is `tableLocation` required for `createEmptyTable`?



##########
lance/lance-rest-server/src/main/java/org/apache/gravitino/lance/service/rest/LanceTableOperations.java:
##########
@@ -122,6 +123,10 @@ public Response createTable(
     }
   }
 
+  /**
+   * According to the latest spec, createEmptyTable only stores the table 
metadata including its
+   * location, and will never touch lance storage.

Review Comment:
   You are addressing an issue in a fixed Lance namespace version; therefore, 
you cannot refer to "According to the latest spec."



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