yuqi1129 commented on code in PR #14682:
URL: https://github.com/apache/gravitino/pull/14682#discussion_r4490673891
##########
core/src/main/java/org/apache/gravitino/tag/TagManager.java:
##########
@@ -482,6 +482,42 @@ private String[] associateTagValuesForMetadataObject(
}
}
+ private static NameIdentifier checkAndNormalizeMetadataObject(
+ String metalake, MetadataObject metadataObject) {
+ NameIdentifier ident = MetadataObjectUtil.toEntityIdent(metalake,
metadataObject);
+ // Existence checks use normalizing dispatchers and must see the original
request. Applying
+ // normalization first would fold quoted names again after their quotes
have been removed.
+ MetadataObjectUtil.checkMetadataObject(metalake, metadataObject);
+ if
(!SUPPORTED_METADATA_OBJECT_TYPES_FOR_TAGS.contains(metadataObject.type())
+ || metadataObject.type() == MetadataObject.Type.CATALOG) {
+ return ident;
+ }
+
+ CatalogManager catalogManager =
GravitinoEnv.getInstance().catalogManager();
+ Capability capability = CapabilityHelpers.getCapability(ident,
catalogManager);
+ Capability.Scope scope =
Capability.Scope.valueOf(metadataObject.type().name());
+ if (scope == Capability.Scope.COLUMN) {
+ NameIdentifier table =
+ CapabilityHelpers.applyCaseSensitive(
+ NameIdentifier.of(ident.namespace().levels()),
Capability.Scope.TABLE, capability);
+ table = CapabilityHelpers.resolvePhysicalTableName(table,
catalogManager);
+ return NameIdentifier.of(
+ Namespace.of(ObjectArrays.concat(table.namespace().levels(),
table.name())),
+ CapabilityHelpers.applyCaseSensitiveOnName(scope, ident.name(),
capability));
Review Comment:
Fixed in 514c761531. Built-in catalogs were not affected (Hive folds columns
and HMS stores them lowercase; Glue and JDBC keep column case), but a custom
`normalizeName` could break an exact name, as you said.
`CapabilityHelpers.normalizeMetadataIdentifier` now resolves the column's
parent table to its physical name (the `resolvePhysicalTableName` part). When
folding changes the column name, it reads the stored table once and keeps the
requested name if a stored column matches it exactly; otherwise it uses the
folded name. When folding changes nothing, there is no store read. Tests:
`TestTagManager.testTagOperationsKeepExactStoredColumnName`,
`TestCapabilityHelpers.testNormalizeMetadataIdentifierKeepsExactStoredColumnName`,
and
`TestMetadataIdConverter.testColumnAuthorizationKeepsExactStoredColumnName`
(86ba83be04).
##########
core/src/main/java/org/apache/gravitino/tag/TagManager.java:
##########
@@ -482,6 +482,42 @@ private String[] associateTagValuesForMetadataObject(
}
}
+ private static NameIdentifier checkAndNormalizeMetadataObject(
+ String metalake, MetadataObject metadataObject) {
+ NameIdentifier ident = MetadataObjectUtil.toEntityIdent(metalake,
metadataObject);
+ // Existence checks use normalizing dispatchers and must see the original
request. Applying
+ // normalization first would fold quoted names again after their quotes
have been removed.
+ MetadataObjectUtil.checkMetadataObject(metalake, metadataObject);
+ if
(!SUPPORTED_METADATA_OBJECT_TYPES_FOR_TAGS.contains(metadataObject.type())
+ || metadataObject.type() == MetadataObject.Type.CATALOG) {
+ return ident;
+ }
+
+ CatalogManager catalogManager =
GravitinoEnv.getInstance().catalogManager();
+ Capability capability = CapabilityHelpers.getCapability(ident,
catalogManager);
+ Capability.Scope scope =
Capability.Scope.valueOf(metadataObject.type().name());
+ if (scope == Capability.Scope.COLUMN) {
Review Comment:
Fixed in 514c761531. The rules now live in
`CapabilityHelpers.normalizeMetadataIdentifier`, and both `TagManager` and
`MetadataIdConverter` call it. The `COLUMN` difference is resolved the
TagManager way: a column's parent table now resolves to its physical name in
authorization too, as tables already did
(`TestMetadataIdConverter.testColumnAuthorizationResolvesParentPhysicalName`).
The entity store is passed as a supplier, so it is only read for a column whose
name folding changes. #14719 and #14720 can reuse the helper.
##########
core/src/test/java/org/apache/gravitino/tag/TestTagManager.java:
##########
@@ -381,6 +421,285 @@ public void cleanUp() {
Arrays.stream(tagManager.listTags(METALAKE)).forEach(n ->
tagManager.deleteTag(METALAKE, n));
}
+ @ParameterizedTest
+ @EnumSource(
+ value = Entity.EntityType.class,
+ names = {"SCHEMA", "TABLE", "COLUMN", "VIEW", "FUNCTION"})
+ void testTagOperationsNormalizeCaseInsensitiveNames(Entity.EntityType type) {
Review Comment:
Added in 514c761531:
- `testTagOperationsFollowGlueStyleColumnCase`: schema and table fold, the
column keeps `MixedCol`.
- `testFailedTableNameResolutionLeavesTagsUnchanged` (TABLE and COLUMN): the
resolver throws; add, remove and value assignment all fail with the resolver
error as the cause, and the existing tag is still the only one listed.
--
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]
