This is an automated email from the ASF dual-hosted git repository.
jshao pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/gravitino.git
The following commit(s) were added to refs/heads/main by this push:
new f4b7bce73d [#9298] improvement(lance-rest): Add more tests in Lance
REST and lakehouse generic catalog. (#9299)
f4b7bce73d is described below
commit f4b7bce73d30cc6b85f0ef94eb77ded8e71ce2af
Author: Mini Yu <[email protected]>
AuthorDate: Tue Dec 2 19:19:56 2025 +0800
[#9298] improvement(lance-rest): Add more tests in Lance REST and
lakehouse generic catalog. (#9299)
### What changes were proposed in this pull request?
- Change the key from `register` to `lance.register` in register a lance
table.
- set `external` to true for a registered table.
### Why are the changes needed?
It's a bug.
Fix: #9298
### Does this PR introduce _any_ user-facing change?
N/A
### How was this patch tested?
IT and existing tests.
---
.../test/CatalogGenericCatalogLanceIT.java | 37 +++++++++++++++++++
.../lance/integration/test/LanceRESTServiceIT.java | 42 ++++++++++++++++++++++
2 files changed, 79 insertions(+)
diff --git
a/catalogs/catalog-lakehouse-generic/src/test/java/org/apache/gravitino/catalog/lakehouse/lance/integration/test/CatalogGenericCatalogLanceIT.java
b/catalogs/catalog-lakehouse-generic/src/test/java/org/apache/gravitino/catalog/lakehouse/lance/integration/test/CatalogGenericCatalogLanceIT.java
index 1988a2d56d..fae235818b 100644
---
a/catalogs/catalog-lakehouse-generic/src/test/java/org/apache/gravitino/catalog/lakehouse/lance/integration/test/CatalogGenericCatalogLanceIT.java
+++
b/catalogs/catalog-lakehouse-generic/src/test/java/org/apache/gravitino/catalog/lakehouse/lance/integration/test/CatalogGenericCatalogLanceIT.java
@@ -897,4 +897,41 @@ public class CatalogGenericCatalogLanceIT extends BaseIT {
Distributions.NONE,
new SortOrder[0]));
}
+
+ @Test
+ void testRegisterWithNonExistLocation() {
+ // Now try to register a table with non-existing location with CREATE mode
- should succeed
+ String newTableName = GravitinoITUtils.genRandomName(TABLE_PREFIX);
+ NameIdentifier newTableIdentifier = NameIdentifier.of(schemaName,
newTableName);
+ Map<String, String> newCreateProperties = createProperties();
+ String newLocation = String.format("%s/%s/%s/", tempDirectory, schemaName,
newTableName);
+ boolean dirExists = new File(newLocation).exists();
+ Assertions.assertFalse(dirExists);
+
+ newCreateProperties.put(Table.PROPERTY_LOCATION, newLocation);
+ newCreateProperties.put(Table.PROPERTY_TABLE_FORMAT, LANCE_TABLE_FORMAT);
+ newCreateProperties.put(Table.PROPERTY_EXTERNAL, "true");
+ newCreateProperties.put(LANCE_TABLE_REGISTER, "true");
+
+ Table nonExistingTable =
+ catalog
+ .asTableCatalog()
+ .createTable(
+ newTableIdentifier,
+ new Column[0],
+ "Updated comment",
+ newCreateProperties,
+ Transforms.EMPTY_TRANSFORM,
+ Distributions.NONE,
+ new SortOrder[0]);
+
+ Assertions.assertNotNull(nonExistingTable);
+ Assertions.assertEquals(
+ newLocation,
nonExistingTable.properties().get(Table.PROPERTY_LOCATION));
+ Assertions.assertEquals(dirExists, new File(newLocation).exists());
+
+ // Now try to drop table, there should no problem here
+ boolean dropSuccess =
catalog.asTableCatalog().dropTable(newTableIdentifier);
+ Assertions.assertTrue(dropSuccess);
+ }
}
diff --git
a/lance/lance-rest-server/src/test/java/org/apache/gravitino/lance/integration/test/LanceRESTServiceIT.java
b/lance/lance-rest-server/src/test/java/org/apache/gravitino/lance/integration/test/LanceRESTServiceIT.java
index 9dab836811..3cdb773626 100644
---
a/lance/lance-rest-server/src/test/java/org/apache/gravitino/lance/integration/test/LanceRESTServiceIT.java
+++
b/lance/lance-rest-server/src/test/java/org/apache/gravitino/lance/integration/test/LanceRESTServiceIT.java
@@ -608,6 +608,9 @@ public class LanceRESTServiceIT extends BaseIT {
RegisterTableResponse response = ns.registerTable(registerTableRequest);
Assertions.assertNotNull(response);
+ // The location should not exist yet as we do not create it in advanced.
+ Assertions.assertEquals(location, response.getLocation());
+ Assertions.assertFalse(new File(location).exists());
DescribeTableRequest describeTableRequest = new DescribeTableRequest();
describeTableRequest.setId(ids);
@@ -623,6 +626,35 @@ public class LanceRESTServiceIT extends BaseIT {
response = Assertions.assertDoesNotThrow(() ->
ns.registerTable(registerTableRequest));
Assertions.assertNotNull(response);
Assertions.assertEquals(newLocation, response.getLocation());
+ // The location should not exist yet as we do not create it in advanced.
+ Assertions.assertFalse(new File(newLocation).exists());
+
+ // Register a new table with location exists
+ String existingLocation = tempDir + "/" + "existing_location/";
+ new File(existingLocation).mkdirs();
+ Assertions.assertTrue(new File(existingLocation).exists());
+ registerTableRequest.setMode(ModeEnum.CREATE);
+ registerTableRequest.setLocation(existingLocation);
+ registerTableRequest.setId(List.of(CATALOG_NAME, SCHEMA_NAME,
"table_with_existing_location"));
+ RegisterTableResponse existingLocationResponse =
+ Assertions.assertDoesNotThrow(() ->
ns.registerTable(registerTableRequest));
+ Assertions.assertNotNull(existingLocationResponse);
+ Assertions.assertEquals(existingLocation,
existingLocationResponse.getLocation());
+ Assertions.assertTrue(new File(existingLocation).exists());
+
+ // Deregister the table with existing location
+ DeregisterTableRequest deregisterExistingLocationRequest = new
DeregisterTableRequest();
+ deregisterExistingLocationRequest.setId(
+ List.of(CATALOG_NAME, SCHEMA_NAME, "table_with_existing_location"));
+ DeregisterTableResponse deregisterExistingLocationResponse =
+ ns.deregisterTable(deregisterExistingLocationRequest);
+ Assertions.assertNotNull(deregisterExistingLocationResponse);
+ Assertions.assertEquals(existingLocation,
deregisterExistingLocationResponse.getLocation());
+ // Assert the location has not been dropped.
+ Assertions.assertTrue(
+ new
File(Objects.requireNonNull(deregisterExistingLocationResponse.getLocation()))
+ .exists());
+ new File(existingLocation).deleteOnExit();
// Test deregister table
DeregisterTableRequest deregisterTableRequest = new
DeregisterTableRequest();
@@ -630,6 +662,16 @@ public class LanceRESTServiceIT extends BaseIT {
DeregisterTableResponse deregisterTableResponse =
ns.deregisterTable(deregisterTableRequest);
Assertions.assertNotNull(deregisterTableResponse);
Assertions.assertEquals(newLocation,
deregisterTableResponse.getLocation());
+
+ // Test Overwrite again after deregister
+ String nonExistingLocation = tempDir + "/" + "non_existing_location/";
+ registerTableRequest.setMode(ModeEnum.OVERWRITE);
+ registerTableRequest.setId(ids);
+ registerTableRequest.setLocation(nonExistingLocation);
+ response = Assertions.assertDoesNotThrow(() ->
ns.registerTable(registerTableRequest));
+ Assertions.assertNotNull(response);
+ Assertions.assertEquals(nonExistingLocation, response.getLocation());
+ Assertions.assertFalse(new File(nonExistingLocation).exists());
}
@Test
