[FLINK-15635][table] EnvironmentSettings supports to accept the user ClassLoader #19845
Conversation
lsyldliu
changed the title
|
Hi @lsyldliu , it seems that the spotless is failed. Could you fix it? |
|
There was a problem hiding this comment.
Thanks @slinkydeveloper , @matriv , @lsyldliu , the pull request looks good to me in general. I only left 2 comments.
| /** | ||
| * 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. |
There was a problem hiding this comment.
This sounds problematic? Why not use userClassloader here?
| val objCopy: AnyRef = | ||
| InstantiationUtil.deserializeObject(byteArray, Thread.currentThread().getContextClassLoader) | ||
| references += objCopy | ||
| references += obj |
There was a problem hiding this 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?
|
Besides, there are many cases are failed, could you take a look? |
Signed-off-by: slinkydeveloper <francescoguard@gmail.com>
…Context Signed-off-by: slinkydeveloper <francescoguard@gmail.com>
Signed-off-by: slinkydeveloper <francescoguard@gmail.com>
Signed-off-by: slinkydeveloper <francescoguard@gmail.com>
|
There are too many commits. I will squash and tidy the commits. |
Signed-off-by: slinkydeveloper <francescoguard@gmail.com>
|
@lsyldliu could you help to have a look at the test? It seems {{SplitAggregateITCase.testMinMaxWithRetraction}} is failed. |
|
@flinkbot run azure |
|
Merging... |
This PR solves the issue of allowing the user to provide its own classloader for UDFs and at other places where it's more appropriate than the current Thread context classloader.
The PR also contains a couple of hotfixes (mergeable separately):
Deprecate CatalogFunction#isGeneric
Remove CodeGeneratorContext#apply which makes the code unnecessarily more complicated
Fix singleton config in EnvironmentSettings, which among various issues, it's also causing in this case to select the wrong user class loader