jaystarshot commented on a change in pull request #2598:
URL: https://github.com/apache/calcite/pull/2598#discussion_r744423629
##########
File path:
core/src/main/java/org/apache/calcite/sql2rel/StandardConvertletTable.java
##########
@@ -308,12 +308,20 @@ private static RexNode convertNvl(SqlRexContext cx,
SqlCall call) {
cx.convertExpression(call.getOperandList().get(1));
final RelDataType type =
cx.getValidator().getValidatedNodeType(call);
+ // Preserve Operand Nullability
return rexBuilder.makeCall(type, SqlStdOperatorTable.CASE,
ImmutableList.of(
rexBuilder.makeCall(SqlStdOperatorTable.IS_NOT_NULL,
operand0),
- rexBuilder.makeCast(type, operand0),
- rexBuilder.makeCast(type, operand1)));
+ rexBuilder.makeCast(
+ cx.getTypeFactory()
+ .createTypeWithNullability(type,
operand0.getType().isNullable()),
+ operand0),
+ rexBuilder.makeCast(
+ cx.getTypeFactory()
Review comment:
@chunweilei
1) Consider If that operand depends on an input reference(which is nullable)
2) If in future that operand pushed down like in
[ProjectJoinTransposeRule](https://github.com/apache/calcite/blob/bebe473fab2e242736614659ed6e5d04eeeb8bf5/core/src/main/java/org/apache/calcite/rel/rules/ProjectJoinTransposeRule.java),
there will be an issue as the operand and its reference have different
nullability.
3) The output of the NVL operation is Not Null but there is no need to
change the nullability of the operands as in calcite rules we might push the
operands down in some rules causing errors ( operand referenced input type is
nullable whereas we forced the operand to be not Null)
Do you think hence it makes sense to let the operand nullability what it was
as the output is anyway not nullable?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
