gustavodemorais commented on code in PR #28836:
URL: https://github.com/apache/flink/pull/28836#discussion_r3675065120
##########
flink-table/flink-table-planner/src/test/java/org/apache/flink/table/planner/codegen/JsonParseReuseTest.java:
##########
@@ -149,9 +154,81 @@ void testThreeJsonFunctionCalls() {
.containsExactlyInAnyOrder(
Row.of("account", "42", "{\"city\":\"Munich\"}"),
Row.of("admin", "30", "{\"city\":\"Berlin\"}"));
- assertThat(countJsonParse(extractGeneratedCode(sql)))
+ final String code = extractGeneratedCode(sql);
+ assertThat(countJsonParse(code))
.as("Three JSON function calls on the same input should parse
once")
- .isEqualTo(1);
+ .isOne();
+ }
+
+ @Test
+ void testReuseSurvivesCodeSplitting() {
Review Comment:
Are you sure this is testing code splitting?
extractGeneratedCode reads GeneratedClass.getCode(), which is the original
source; splitting is applied in the constructor into the separate splitCode
field and only that one is compiled (GeneratedClass.java:55-62, :97). The
config only affects the collect(sql) half. I think it needs some tweaks in the
test to test splitting
But to be honest is not the most important thing to test in the PR, so I'd
not say this is a blocker for me and dropping the test would also be ok
--
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]
