[CALCITE-2901] Preserve scalar RexSubQuery type with copyType - #5027
[CALCITE-2901] Preserve scalar RexSubQuery type with copyType#5027Soumyadeep-github wants to merge 1 commit into
Conversation
Preserve scalar RexSubQuery type with copyType; use copyType for scalar sub-query Rex/SQL types instead of forcing nullable. Cast scalar sub-query Rex to the validated type in SqlToRelConverter. Cast after SubQueryRemoveRule and AggregateMinMaxToLimitRule when types drift. Update tests (IQ, XML, JUnit).
|
mihaibudiu
left a comment
mihaibudiu
left a comment
There was a problem hiding this comment.
Other optimizations will remove useless cast, and some builder methods will not insert useless casts, but maybe you can check if all the casts you insert are necessary. There are quite a few changes.
| + "SELECT NULL) END AS `$f0`\n" | ||
| + "FROM `foodmart`.`product`) AS `t0` ON TRUE\n" | ||
| + "WHERE `product`.`net_weight` > CAST(`t0`.`$f0` AS DOUBLE)"; | ||
| + "WHERE `product`.`net_weight` > CAST(CAST(`t0`.`$f0` AS SIGNED) AS DOUBLE)"; |
There was a problem hiding this comment.
Is this case necessary?
If not, can you avoid inserting it?
There was a problem hiding this comment.
Thanks for taking the time to review my changes! Agreed that we should not rely on later cast cleanup. I’ll go through the changed casts and drop any that aren’t needed for correctness.
For the CAST(CAST($f0 AS …) AS DOUBLE) case in RelToSqlConverterTest: I’ll trace where each cast is introduced and try to simplify to a single cast (or align types earlier) if semantics stay the same; if two steps are still required I’ll note why in a short comment or reply here.
| | 20 | 5 | | ||
| | 30 | 6 | | ||
| | 40 | | | ||
| | 40 | 0 | |
| | 10 | 0 | | ||
| | 20 | 0 | | ||
| | 30 | 0 | | ||
| | 40 | 0 | |
iwanttobepowerful
added
the
returned-with-feedback
|
This pull request has been marked as stale due to 30 days of inactivity. It will be closed in 90 days if no further activity occurs. If you think that’s incorrect or this pull request requires a review, please simply write any comment. If closed, you can revive the PR at any time and @mention a reviewer or discuss it on the dev@calcite.apache.org list. Thank you for your contributions. |
3 participants
JIRA: CALCITE-2901
Changes Proposed