JoegenUSTC commented on code in PR #11707:
URL: https://github.com/apache/gravitino/pull/11707#discussion_r3432602062
##########
catalogs/catalog-hive/src/main/java/org/apache/gravitino/catalog/hive/HiveCatalogCapability.java:
##########
@@ -42,15 +42,19 @@ public CapabilityResult columnDefaultValue() {
@Override
public CapabilityResult caseSensitiveOnName(Scope scope) {
- switch (scope) {
- case SCHEMA:
- case TABLE:
- case COLUMN:
- // Hive is case insensitive, see
- //
https://cwiki.apache.org/confluence/display/Hive/User+FAQ#UserFAQ-AreHiveSQLidentifiers(e.g.tablenames,columnnames,etc)casesensitive?
- return CapabilityResult.unsupported("Hive is case insensitive.");
- default:
- return CapabilityResult.SUPPORTED;
+ // Use if-else instead of switch-on-enum to avoid the compiler-generated
synthetic class $1.
+ // SchemaNormalizeDispatcher calls this method outside the
IsolatedClassLoader.withClassLoader()
+ // boundary. A switch-on-enum causes the JVM to load $1 via the server
classloader, which
+ // cannot find it, and the JVM permanently caches the failure for the
process lifetime.
+ // if-else with == compiles to if_acmpeq (reference comparison) with no
synthetic class.
+ // Hive is case insensitive, see
+ //
https://cwiki.apache.org/confluence/display/Hive/User+FAQ#UserFAQ-AreHiveSQLidentifiers(e.g.tablenames,columnnames,etc)casesensitive?
+ if (scope == null) {
+ throw new NullPointerException("scope");
}
Review Comment:
Good point, updated to use Preconditions.checkArgument(scope != null, "scope
cannot be null") as per the Gravitino coding convention. Also added a dedicated
test testCaseSensitiveOnNameNullScopeThrows to cover the null scope case.
--
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]