gustavodemorais commented on code in PR #28991:
URL: https://github.com/apache/flink/pull/28991#discussion_r3806202667
##########
flink-table/flink-table-planner/src/test/java/org/apache/flink/table/planner/functions/JsonFunctionsITCase.java:
##########
@@ -1034,7 +1037,35 @@ private static List<TestSetSpec> parseJsonSpec() {
jsonString(call("TRY_PARSE_JSON", $("f1"))),
"JSON_STRING(TRY_PARSE_JSON(f1))",
null,
- STRING()));
+ STRING()),
+ // Regression test for FLINK-40410: PARSE_JSON(f0) (VARIANT)
precedes
+ // JSON_STRING(PARSE_JSON(f0)) (STRING) in a constant-folded
projection. VARIANT
+ // is excluded from folding in ExpressionReducer, like
ROW/ARRAY/MAP, and
+ // evaluated normally at runtime.
Review Comment:
nit: this comment narrates the bug/ticket story rather than stating the
timeless invariant. The test name already establishes it's a regression test,
so the JIRA reference and blow-by-blow of what the old bug looked like can move
to the commit message. Something tighter reads better long-term, e.g.:
```suggestion
// VARIANT must be excluded from constant folding, like
ROW/ARRAY/MAP,
// otherwise a constant-folded projection reads a stale slot
for it.
```
--
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]
