shaoyu-li commented on code in PR #13058:
URL: https://github.com/apache/gravitino/pull/13058#discussion_r3985375617
##########
catalogs/catalog-lakehouse-generic/src/main/java/org/apache/gravitino/catalog/lakehouse/generic/GenericCatalogOperations.java:
##########
@@ -233,7 +250,16 @@ public Table createTable(
Index[] indexes)
throws NoSuchSchemaException, TableAlreadyExistsException {
Schema schema = loadSchema(NameIdentifier.of(ident.namespace().levels()));
- String tableLocation = calculateTableLocation(schema, ident, properties);
+ String tableLocation =
+ validateProvisionedLocation(
+ tableLocationProvider.provisionTableLocation(
Review Comment:
**External.** Agreed on the problem, but `external` turns out to be the
wrong test: a Lance
registration supplies a location without setting `external`, so that check
would miss it. Keying on
whether the request carries its own non-blank location instead — such a
request has already decided
where the data lives, and consulting an allocator would repoint the table at
a fresh empty path and
orphan the caller's data while still reporting success. Adding
`isExternal()` to the context as you
asked, so a provider can still see it.
**Leak on failed creation.** Agreed — moving the format check and the
`tableOpsCache` lookup above
provisioning, those are free. Not adding a compensating unprovision on
failure, though: a format
that fails partway through creation may already have written to the
location, and unprovisioning
would tell the provider it is free to reclaim a path that has data on it.
Leaking an unused path is
the safer of the two failures, and doing better needs the format to report
whether it touched
storage before failing, which this interface can't express. Documenting it
as a known limitation.
--
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]
