[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.
2 participants
JIRA: CALCITE-2901
Changes Proposed