wuchong commented on code in PR #19845:
URL: https://github.com/apache/flink/pull/19845#discussion_r886303859
##########
flink-table/flink-table-planner/src/main/scala/org/apache/flink/table/planner/codegen/calls/ScalarOperatorGens.scala:
##########
@@ -1763,6 +1763,11 @@ object ScalarOperatorGens {
}
}
+ /**
+ * Note that this cast context is going to use the thread context
classloader. This is fine when
+ * the context will be used to generate casting for primitive types, but it
might create problems
+ * when dealing with user provided types.
Review Comment:
This sounds problematic? Why not use `userClassloader` here?
##########
flink-table/flink-table-planner/src/main/scala/org/apache/flink/table/planner/codegen/CodeGeneratorContext.scala:
##########
@@ -701,11 +701,7 @@ class CodeGeneratorContext(val tableConfig:
ReadableConfig) {
fieldTerm: String,
fieldTypeTerm: String): Unit = {
val idx = references.length
- // make a deep copy of the object
- val byteArray = InstantiationUtil.serializeObject(obj)
- val objCopy: AnyRef =
- InstantiationUtil.deserializeObject(byteArray,
Thread.currentThread().getContextClassLoader)
- references += objCopy
+ references += obj
Review Comment:
Hi @slinkydeveloper , I think the `CodeGeneratorContext` doesn't know
whether the `obj` would be mutated or not, so it would be safer to clone the
`obj` here. Do you have any concerns to copy the object?
--
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]
