JoegenUSTC commented on code in PR #11355:
URL: https://github.com/apache/gravitino/pull/11355#discussion_r3353873669


##########
catalogs/hive-metastore-common/src/main/java/org/apache/gravitino/hive/client/HiveClientFactory.java:
##########
@@ -162,10 +164,18 @@ public static HiveClient createHiveClientImpl(
       HiveClientClassLoader.HiveVersion version, Properties properties, 
ClassLoader classloader)
       throws Exception {
     Class<?> hiveClientImplClass = 
classloader.loadClass(HiveClientImpl.class.getName());
+    // HiveClientImpl is a barrier class loaded via defineClass inside the 
isolated classloader.
+    // Its constructor expects the HiveVersion enum type from the *isolated* 
classloader scope.
+    // Passing the system classloader's HiveVersion.class causes 
NoSuchMethodException because
+    // the two Class objects are not identical even though they have the same 
name.
+    Class<?> hiveVersionInIsolated =
+        
classloader.loadClass(HiveClientClassLoader.HiveVersion.class.getName());
+    @SuppressWarnings({"unchecked", "rawtypes"})
+    Object isolatedVersion =
+        Enum.valueOf((Class<? extends Enum>) hiveVersionInIsolated, 
version.name());
     Constructor<?> hiveClientImplCtor =
-        hiveClientImplClass.getConstructor(
-            HiveClientClassLoader.HiveVersion.class, Properties.class);
-    return (HiveClient) hiveClientImplCtor.newInstance(version, properties);

Review Comment:
   You are absolutely right, thank you for the correction!
   
   I re-read `HiveClientClassLoader.isSharedClass()` carefully and found the 
rule:
   
       if (name.startsWith("org.apache.gravitino.")) { return true; }
   
   `HiveVersion` is 
`org.apache.gravitino.hive.client.HiveClientClassLoader$HiveVersion`, 
   which falls under this rule. The isolated classloader delegates it to the 
base 
   classloader, so both sides hold the same `Class` object. 
`getConstructor(HiveVersion.class, 
   Properties.class)` works correctly without any workaround.
   
   I've simplified `createHiveClientImpl()` to pass 
`HiveClientClassLoader.HiveVersion.class` 
   directly and removed the unnecessary `Enum.valueOf` reflection code.



-- 
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]

Reply via email to