diqiu50 commented on code in PR #12098:
URL: https://github.com/apache/gravitino/pull/12098#discussion_r3613887539
##########
core/src/main/java/org/apache/gravitino/catalog/SchemaNormalizeDispatcher.java:
##########
@@ -57,10 +56,9 @@ public NameIdentifier[] listSchemas(Namespace namespace)
throws NoSuchCatalogExc
NameIdentifier.of(namespace.levels())));
}
- NameIdentifier[] identifiers = dispatcher.listSchemas(namespace);
- // The constraints of the name spec may be more strict than underlying
catalog,
- // and for compatibility reasons, we only apply case-sensitive
capabilities here.
- return normalizeCaseSensitive(identifiers);
+ // listSchemas() is assumed to already return names in their canonical,
legal form, so no
+ // re-normalization is applied here.
+ return dispatcher.listSchemas(namespace);
Review Comment:
Skipping re-normalization in listSchemas()/listTables() is intentional here,
because the name returned by the backend catalog is already correct.
For example, when creating a table with the quoted name "Aaa", Oracle stores
the physical table name as Aaa (case preserved due to quoting). listTables()
then returns Aaa, which is already correct. If we re-apply
normalizeCaseSensitive() on top of that, Oracle's folding rule (unquoted names
are upper-cased) would turn Aaa into AAA — silently corrupting a name that was
already right.
This only breaks for catalogs like Oracle where folding depends on the
name's original form (quoted vs. unquoted), so normalizeName isn't idempotent.
For catalogs with simple, context-independent folding (always
upper/lower-case), re-normalizing is harmless since it's already idempotent —
but we can't tell the two cases apart here, so the safer fix is to trust the
catalog's result as-is.
--
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]
