[FLINK-39999][table] Make Flink Sql calls unparsable as it is required by Calcite 1.42.0#28551
[FLINK-39999][table] Make Flink Sql calls unparsable as it is required by Calcite 1.42.0#28551snuyanzin wants to merge 5 commits into
Conversation
…d by Calcite 1.42.0
… Calcite 1.42.0 upgrade
AHeise
left a comment
AHeise
left a comment
There was a problem hiding this comment.
Looks very good. Thank you for fixing.
I have NOT looked at all the different classes but rather a good portion of it. However the fixes to SqlSpecialOperator#createCall are rather mechanical.
What I'm a bit more worried about are the intermittent fixes to #getOperandList. These changes are more subtle with a higher blast radius. Can you do an additional pass over them? I'm also trying to catch them all but they also require some knowledge about the SqlNode.
| } | ||
|
|
||
| /** Decodes a {@code boolean} field encoded as a {@link SqlLiteral}. */ | ||
| protected static boolean operandToBoolean(SqlNode operand) { |
There was a problem hiding this comment.
Any value in doing some assertion on type for better error message? I think it's so deep in the stack that classcast is just fine.
| for (String detail : explainDetails) { | ||
| details.add(SqlLiteral.createCharString(detail, SqlParserPos.ZERO)); | ||
| } | ||
| return List.of(statement, details); |
There was a problem hiding this comment.
Is this an unrelated fix? (okay to keep)
3 participants
What is the purpose of the change
The main reason is CALCITE-7301(https://issues.apache.org/jira/browse/CALCITE-7301) issue which introduced tests for every sql operator to check whether it is unparsable
The PR introduces similar fix and a similar test (in a separate commit to easily revert it once after upgrade Calcite's test could be reused)
Brief change log
parse sql operators
Verifying this change
test introduced
Does this pull request potentially affect one of the following parts:
@Public(Evolving): (no)Documentation
Was generative AI tooling used to co-author this PR?