JingsongLi commented on a change in pull request #16349:
URL: https://github.com/apache/flink/pull/16349#discussion_r664210234
##########
File path:
flink-table/flink-table-api-java/src/main/java/org/apache/flink/table/api/config/TableConfigOptions.java
##########
@@ -94,8 +94,16 @@ private TableConfigOptions() {}
public static final ConfigOption<Integer> MAX_LENGTH_GENERATED_CODE =
key("table.generated-code.max-length")
.intType()
- .defaultValue(64000)
+ .defaultValue(4000)
.withDescription(
"Specifies a threshold where generated code will
be split into sub-function calls. "
+ "Java has a maximum method length of 64
KB. This setting allows for finer granularity if necessary.");
+
+ @Documentation.TableOption(execMode =
Documentation.ExecMode.BATCH_STREAMING)
+ public static final ConfigOption<Integer> MAX_MEMBERS_GENERATED_CODE =
Review comment:
Can this one be internal config option? I don't think it is a
frequently-used option.
##########
File path: flink-table/flink-table-planner-blink/pom.xml
##########
@@ -378,6 +378,8 @@ under the License.
<!--
flink-table-runtime-blink dependencies -->
<include>org.codehaus.janino:*</include>
+
<include>org.apache.flink:flink-table-code-splitter</include>
Review comment:
Can include in runtime?
##########
File path:
flink-table/flink-table-runtime-blink/src/main/java/org/apache/flink/table/runtime/generated/CompileUtils.java
##########
@@ -71,23 +72,32 @@
.weakKeys()
.softValues()
.build());
- return compiledClasses.get(cl, () -> doCompile(cl, name, code));
+ return compiledClasses.get(cl, () -> doCompile(cl, name, code,
splitCode));
} catch (Exception e) {
throw new FlinkRuntimeException(e.getMessage(), e);
}
}
- private static <T> Class<T> doCompile(ClassLoader cl, String name, String
code) {
+ private static <T> Class<T> doCompile(
+ ClassLoader cl, String name, String code, String splitCode) {
checkNotNull(cl, "Classloader must not be null.");
CODE_LOG.debug("Compiling: {} \n\n Code:\n{}", name, code);
+
SimpleCompiler compiler = new SimpleCompiler();
compiler.setParentClassLoader(cl);
try {
- compiler.cook(code);
- } catch (Throwable t) {
- System.out.println(addLineNumber(code));
- throw new InvalidProgramException(
- "Table program cannot be compiled. This is a bug. Please
file an issue.", t);
+ compiler.cook(splitCode);
+ } catch (Throwable throwable) {
+ throwable.printStackTrace();
Review comment:
LOG.error
##########
File path: flink-table/flink-table-planner-blink/pom.xml
##########
@@ -378,6 +378,8 @@ under the License.
<!--
flink-table-runtime-blink dependencies -->
<include>org.codehaus.janino:*</include>
+
<include>org.apache.flink:flink-table-code-splitter</include>
+
<include>org.antlr:antlr4-runtime</include>
Review comment:
Can we just include antlr:antlr4 in `flink-table-code-splitter`? (And
modify NOTICE too)
##########
File path:
flink-table/flink-table-planner-blink/src/main/java/org/apache/flink/table/planner/plan/abilities/source/WatermarkPushDownSpec.java
##########
@@ -127,10 +129,18 @@ public DefaultWatermarkGeneratorSupplier(
new
ArrayList<>(Arrays.asList(generatedWatermarkGenerator.getReferences()));
references.add(context);
+ String code = generatedWatermarkGenerator.getCode();
+ String splitCode =
+ JavaCodeSplitter.split(
Review comment:
Can we take this into `GeneratedClass`? We have 20+
`JavaCodeSplitter.split`... We can just pass a TableConfig to `GeneratedClass`.
--
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]
