rubenada commented on code in PR #5370:
URL: https://github.com/apache/calcite/pull/5370#discussion_r4535290068
##########
core/src/main/java/org/apache/calcite/sql/SqlWithItem.java:
##########
@@ -131,10 +146,11 @@ private static class SqlWithItemOperator extends
SqlSpecialOperator {
@Override public SqlCall createCall(@Nullable SqlLiteral functionQualifier,
SqlParserPos pos, @Nullable SqlNode... operands) {
assert functionQualifier == null;
- assert operands.length == 4 || operands.length == 5;
+ assert operands.length >= 4 && operands.length <= 6;
return new SqlWithItem(pos, (SqlIdentifier) operands[0],
(SqlNodeList) operands[1], operands[2], (SqlLiteral) operands[3],
- operands.length == 5 ? (SqlCycleClause) operands[4] : null);
+ operands.length >= 5 ? (SqlSearchClause) operands[4] : null,
Review Comment:
Since the last two operands are "optional", it can be a bit risky to allow
createCall with 5 operands, because a caller might be tempted to pass a
SqlCycleClause as last operand, leading to a ClassCastException here. IMO it'd
be safer to allow only either 4 or 6 operands, so that optional parameters are
either both omitted or both present. I think that will be less error-prone.
--
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]
