amaliujia commented on a change in pull request #13835:
URL: https://github.com/apache/beam/pull/13835#discussion_r566494713
##########
File path:
sdks/java/extensions/sql/src/main/java/org/apache/beam/sdk/extensions/sql/impl/JavaUdfLoader.java
##########
@@ -57,7 +58,10 @@
* Maps the external jar location to the functions the jar defines. Static
so it can persist
* across multiple SQL transforms.
*/
- private static final Map<String, FunctionDefinitions> cache = new
HashMap<>();
+ private static final Map<String, FunctionDefinitions> functionCache = new
HashMap<>();
+
+ /** Maps potentially remote jar paths to their local file copies. */
+ private static final Map<String, File> jarCache = new HashMap<>();
Review comment:
To confirm: for the created random files, as I recall they will live
while the JVM is alive?
Just to confirm that there will not be the case the there is a entry in this
jarCache but the jar file does not exist.
##########
File path:
sdks/java/extensions/sql/src/main/java/org/apache/beam/sdk/extensions/sql/impl/rel/BeamCalcRel.java
##########
@@ -232,15 +245,22 @@ public Calc copy(RelTraitSet traitSet, RelNode input,
RexProgram program) {
private static class CalcFn extends DoFn<Row, Row> {
private final String processElementBlock;
private final Schema outputSchema;
+ private final List<String> jarPaths;
private transient @Nullable ScriptEvaluator se = null;
- public CalcFn(String processElementBlock, Schema outputSchema) {
+ public CalcFn(String processElementBlock, Schema outputSchema,
List<String> jarPaths) {
this.processElementBlock = processElementBlock;
this.outputSchema = outputSchema;
+ this.jarPaths = jarPaths;
}
- ScriptEvaluator compile() {
+ ScriptEvaluator compile() throws IOException {
ScriptEvaluator se = new ScriptEvaluator();
+ if (!jarPaths.isEmpty()) {
+ JavaUdfLoader udfLoader = new JavaUdfLoader();
+ ClassLoader classLoader = udfLoader.createClassLoader(jarPaths);
+ se.setParentClassLoader(classLoader);
Review comment:
From the java doc I cannot really tell what this `setParentClassLoader`
behave:
https://janino-compiler.github.io/janino/apidocs/org/codehaus/janino/ScriptEvaluator.html#setParentClassLoader(java.lang.ClassLoader)
Does it use `classLoader` to replace the default one or combine with the
default one? Basically I guess to make `ScriptEvaluator` work functionally, it
needs to know both Beam and UDF implementation.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
