diqiu50 commented on code in PR #12997:
URL: https://github.com/apache/gravitino/pull/12997#discussion_r3958202167
##########
catalogs/catalog-glue/src/main/java/org/apache/gravitino/catalog/glue/GlueCatalogOperations.java:
##########
@@ -824,16 +827,23 @@ private software.amazon.awssdk.services.glue.model.Column
toGlueColumn(Column co
.build();
}
+ /**
+ * Resolves the storage location of a table, in order of precedence: the
explicit {@code location}
+ * property, the {@code LocationUri} the Glue database declares, and finally
the catalog warehouse
+ * path. A database location already identifies the database, so the table
name is appended
+ * directly to it, whereas the warehouse path is shared by all databases and
needs the database
+ * name in between.
+ */
private String resolveTableLocation(String explicitLocation, String dbName,
String tableName) {
if (explicitLocation != null) {
return explicitLocation;
}
+ String databaseLocation = databaseLocationUri(dbName);
+ if (StringUtils.isNotBlank(databaseLocation)) {
+ return trimTrailingSlash(databaseLocation) + "/" + tableName;
+ }
Review Comment:
Thanks for the suggestion. The GetDatabase call is required to resolve the
database LocationUri, and the per-create overhead is acceptable for now.
Caching also requires an invalidation policy to avoid using a stale LocationUri
after the Glue database is updated, so we will leave it as a follow-up
optimization if request latency or throttling becomes an issue.
--
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]
