snuyanzin commented on code in PR #26474: URL: https://github.com/apache/flink/pull/26474#discussion_r2057965475
########## flink-table/flink-table-planner/src/main/scala/org/apache/flink/table/planner/codegen/JsonGenerateUtils.scala: ########## @@ -227,6 +227,23 @@ object JsonGenerateUtils { } } + /** + * Determines if the parameter index is % 2, which means checking that we only allow JSON calls + * for the VALUE parameter of a JSON_OBJECT call. + */ + def inValuesParamOfJsonObject(i: Int): Boolean = { + (i % 2) == 0 + } + + /** + * Determines whether a JSON function is allowed in the current context. JSON functions are + * allowed as values in JSON_ARRAY calls or as value parameters in JSON_OBJECT calls. + */ + def isSupportedJsonOperand(operand: RexNode, call: RexNode, i: Int): Boolean = { + isJsonFunctionOperand(operand) && + (isJsonArrayOperand(call) || isJsonObjectOperand(call) && inValuesParamOfJsonObject(i)) + } Review Comment: the confusing part for me is 1. the line which is body of function is about 10 symbols length 2. we add a separate method for that and 5 lines of comments for description why not just inline it and add one-line comment describing the intention? -- 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: issues-unsubscr...@flink.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org