-
Notifications
You must be signed in to change notification settings - Fork 13.5k
[FLINK-37695][table] Fix parsing for built-in function JSON in JSON_OBJECT for all positions #26474
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
if isJsonFunctionOperand(operand) && | ||
(isJsonArrayOperand(call) || i == 2 && isJsonObjectOperand(call)) => | ||
(isJsonArrayOperand(call) || isJsonObjectOperand(call) && inValuesParam(i)) => |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i wonder whether we can extract the whole if
condition into method something like isJsonOperand
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Created a helper function called isSupportedJsonOperand
* 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 inValuesParam(i: Int): Boolean = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we can drop this method if we extract the whole if
condition into a method.
Also about naming: it might make confused some people since in SQL there is also VALUES
and this method has nothing to do with it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I renamed it to inValuesParamOfJsonObject? I think the code is more readable if you read inValuesParamOfJsonObject
as part of the if condition than i%2
What is the purpose of the change
Parsing for a JSON() function call fails if its position is not the first param of JSON_OBJECT.
SELECT JSON_OBJECT('key' VALUE JSON('{}'), 'key_2' VALUE JSON('{}'));
Outputs:
This is incorrect, we were too strict with the limitations. We're strictly allowing json to be called only as the second param in the function, when it should be allowed for every second.
Brief change log
(for example:)
JSON
call is for any of the VALUES param inside aJSON_OBJECT
and not only the first oneVerifying this change
This change added tests and can be verified as follows:
Does this pull request potentially affect one of the following parts:
@Public(Evolving)
: (no)Documentation