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`.
##########
File path: docs/layouts/shortcodes/generated/table_config_configuration.html
##########
@@ -22,7 +22,7 @@
</tr>
<tr>
<td><h5>table.generated-code.max-length</h5><br> <span
class="label label-primary">Batch</span> <span class="label
label-primary">Streaming</span></td>
- <td style="word-wrap: break-word;">64000</td>
+ <td style="word-wrap: break-word;">4000</td>
<td>Integer</td>
<td>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.</td>
Review comment:
You need update `Java has a maximum method length of 64 KB` too. Why not
64K?
Add some description for `JIT` limitation?
##########
File path:
flink-table/flink-table-code-splitter/src/main/resources/META-INF/NOTICE
##########
@@ -1,4 +1,4 @@
-flink-table-code-splitter
+Flink : Table : Code Splitter
Review comment:
Why not `flink-table-code-splitter`?
##########
File path:
flink-table/flink-table-code-splitter/src/main/resources/META-INF/NOTICE
##########
@@ -7,7 +7,7 @@ The Apache Software Foundation (http://www.apache.org/).
This project bundles the following dependencies under the BSD 3-clause license.
See bundled license files for details.
-- antlr:4.7
+- org.antlr:antlr4-runtime:4.7
Review comment:
I took a look to
https://github.com/antlr/grammars-v4/blob/master/java/java/pom.xml#L4 for
`JavaLexer.g4`.
There should be two.
##########
File path:
flink-table/flink-table-runtime-blink/src/main/java/org/apache/flink/table/runtime/generated/GeneratedTableAggsHandleFunction.java
##########
@@ -18,12 +18,15 @@
package org.apache.flink.table.runtime.generated;
+import org.apache.flink.configuration.Configuration;
+
/** Describes a generated {@link TableAggsHandleFunction}. */
public class GeneratedTableAggsHandleFunction extends
GeneratedClass<TableAggsHandleFunction> {
private static final long serialVersionUID = 1L;
Review comment:
Maybe you should increase `serialVersionUID`?
--
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]
