yuqi1129 commented on code in PR #13722:
URL: https://github.com/apache/gravitino/pull/13722#discussion_r4399231114
##########
catalogs/catalog-lakehouse-generic/src/main/java/org/apache/gravitino/catalog/lakehouse/lance/LanceTableOperations.java:
##########
@@ -953,4 +958,50 @@ private IndexParams getIndexParamsByIndexType(IndexType
indexType) {
throw new IllegalArgumentException("Unsupported index type: " +
indexType);
}
}
+
+ private void dropExternalTableForRegistration(NameIdentifier ident) {
+ try {
+ Table existing = super.loadTable(ident);
+ validateLanceTable(ident, existing);
+ Preconditions.checkArgument(
+ LancePropertiesUtils.isExternalTable(existing.properties()),
+ "Cannot overwrite managed Lance table %s with a registration; "
+ + "explicitly drop or purge the managed table before registering
a replacement",
+ ident);
+ // Registration replaces external metadata only. Calling dropTable could
delete a managed
+ // dataset, including the same dataset that the incoming registration
just validated.
+ super.dropTable(ident);
+ } catch (NoSuchTableException e) {
+ // No existing registration needs replacing.
+ }
+ }
+
+ /**
+ * Validates that a caller-supplied location resolves to an existing, valid
Lance dataset before
+ * metadata changes adopt it. An explicit purge can later delete the
registered location, so a
+ * plain directory must not be adopted as if it were a Lance dataset (issue
#13696).
+ *
+ * <p>This checks dataset existence, not operator-defined storage scope or
purge authorization.
+ *
+ * @param ident the table being registered, used only for the error message
+ * @param location the caller-supplied dataset location
+ * @param properties the table properties, used to resolve storage options
+ * @throws IllegalArgumentException if dataset validation cannot complete
successfully
+ */
+ private void validateExistingDatasetLocation(
+ NameIdentifier ident, String location, Map<String, String> properties) {
+ Map<String, String> storageOptions =
+ LancePropertiesUtils.resolveLanceStorageOptions(catalogProperties,
properties);
+ try (Dataset ignored = openDataset(location, storageOptions)) {
+ // Opening a non-Lance location throws; reaching here confirms a valid
dataset.
+ LOG.debug("Validated Lance dataset at location {} for table {}",
location, ident);
+ } catch (Exception e) {
Review Comment:
Fixed in dc3913f099. Validation now preserves the Lance 6.0.0 SDK
distinction: dataset/input errors remain IllegalArgumentException (400), while
I/O and unexpected failures use ExceptionMessages.wrap (500), retaining context
and cause. Dataset-close failures are handled separately as server errors,
including IllegalArgumentException from cleanup. Added regressions for
I/O/runtime failures in both CREATE and OVERWRITE, cleanup failures, and real
missing/plain-directory locations.
--
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]
