-
Notifications
You must be signed in to change notification settings - Fork 13.8k
[FLINK-20539][table] Type mismatch when using computed ROW column #27158
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
Co-authored-by: xuyang <[email protected]>
| "ROW<f0 INT NOT NULL, f1 BOOLEAN>", | ||
| DataTypes.ROW( | ||
| DataTypes.FIELD("f0", DataTypes.INT()), | ||
| DataTypes.FIELD("f0", DataTypes.INT().notNull()), |
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.
Now align with what is mentioned in comment
Seems it is a very old bug also mentioned at https://issues.apache.org/jira/browse/FLINK-13604
| new RexShuttle() { | ||
| @Override | ||
| public RexNode visitCall(final RexCall call) { | ||
| if (call.getKind() == SqlKind.CAST |
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.
A basic question - I am curious why we are doing cast processing when the reported problem was for a computed row - are they related?
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.
yes, if you look at the comment for makeFieldAccess in this class, for instance
Lines 34 to 41 in ed0ae9d
| /** | |
| * Compared to the original method we adjust the nullability of the nested column based on the | |
| * nullability of the enclosing type. | |
| * | |
| * <p>If the fields type is NOT NULL, but the enclosing ROW is nullable we still can produce | |
| * nulls. | |
| */ | |
| @Override |
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.
so we adjust nullability, however with such approach for heavily nested columns it might happen that several casts might appear after that. The idea here is to keep only zero or one if needed and not more since anyway one cast is enough for nullability adjustment
When using cast, Flink uses ExtendedSqlRowTypeNameSpec to derive the row type, and we should change the StructKind from FULLY_QUALIFIED in Calcite to PEEK_FIELDS_NO_EXPAND in Flink.
Also if extra cast for nested field happened because of mismatched type with enclosing row it checks whether previous cast should be dropped.
This will resolve the issue with nested column casts
for instance
and then
gives
while in fact it should be simplified to
I noticed that part of my changes are very similar to the PR #24994 (about PEEK_FIELDS_NO_EXPAND) done by @xuyangzhong,
however cherry-picking leads to multiple conflicts, so I just added him as a co-author.
Brief change log
Return a Row type with PEEK_FIELDS_NO_EXPAND as the target type in CAST
Drop extra casts for rexFieldAccess
Verifying this change
A number of tests in CalcTest and CalcITTest
Does this pull request potentially affect one of the following parts:
@Public(Evolving): ( no)Documentation