leekeiabstraction commented on code in PR #29356:
URL: https://github.com/apache/flink/pull/29356#discussion_r4164495518
##########
flink-table/flink-table-runtime/src/main/java/org/apache/flink/table/runtime/generated/CompileUtils.java:
##########
@@ -95,8 +107,32 @@ public static <T> Class<T> compile(ClassLoader cl, String
name, String code) {
}
}
+ @SuppressWarnings("unchecked")
private static <T> Class<T> doCompile(ClassLoader cl, String name, String
code) {
checkNotNull(cl, "Classloader must not be null.");
+ // Generated code only references types that resolve the same way
under every classloader,
+ // so the bytecode is cooked once per code and defined into cl.
+ final Map<String, byte[]> byteCodes = sharedByteCode(cl, name, code);
+ try {
+ return (Class<T>) new ByteArrayClassLoader(cl,
byteCodes).loadClass(name);
+ } catch (ClassNotFoundException e) {
+ throw new FlinkRuntimeException("Can not load class " + name, e);
+ }
+ }
+
+ private static Map<String, byte[]> sharedByteCode(ClassLoader cl, String
name, String code) {
+ try {
+ return BYTECODE_CACHE.get(code, () -> cook(cl, name, code));
Review Comment:
I'm not very familiar with Janino compilation but I had a quick exploration
with Claude. Is this a risk when using UDF/user defined pojo changed for
example?
```
The generated code names user types but doesn't describe them. The POJO
converter emits something like this:
((java.lang.Integer) external.getAge())
where external is declared as com.x.Pojo. The string says "call getAge() on
com.x.Pojo". It doesn't say what getAge() returns. That information lives in
the user's jar, and Janino looks it up through the classloader when it compiles
the string.
So with the same string and two different jars:
┌────────────────────────────┬──────────────────────────────────────────────────────┬────────────────────────────────────────────────┐
│ │ Job A (jar v1)
│ Job B (jar v2) │
├────────────────────────────┼──────────────────────────────────────────────────────┼────────────────────────────────────────────────┤
│ User's POJO source │ int getAge()
│ Integer getAge() │
├────────────────────────────┼──────────────────────────────────────────────────────┼────────────────────────────────────────────────┤
│ Generated string │ ((java.lang.Integer) external.getAge())
│ same string │
├────────────────────────────┼──────────────────────────────────────────────────────┼────────────────────────────────────────────────┤
│ Bytecode Janino would emit │ invokevirtual Pojo.getAge()I +
Integer.valueOf (box) │ invokevirtual Pojo.getAge()Ljava/lang/Integer; │
└────────────────────────────┴──────────────────────────────────────────────────────┴────────────────────────────────────────────────┘
The string is the same in both columns because castExpr always wraps the
field in its boxed type (primitiveToWrapper). Both int and Integer become
(java.lang.Integer).
Compiling turns a name into a concrete method descriptor (getAge()I), and
that descriptor depends on which version of com.x.Pojo the compiling
classloader sees. Before this PR, each classloader compiled its own copy, so
job B's bytecode was built against v2. Now the cache key is the string alone.
Job B finds job A's bytecode and loads it into job B's classloader. The JVM
then looks for getAge()I on v2's Pojo, finds only getAge()Ljava/lang/Integer;,
and throws NoSuchMethodError.
```
--
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]
