vlsi commented on a change in pull request #2296:
URL: https://github.com/apache/calcite/pull/2296#discussion_r543699372
##########
File path:
core/src/main/java/org/apache/calcite/sql2rel/StandardConvertletTable.java
##########
@@ -837,6 +841,44 @@ private static RexNode makeConstructorCall(
return rexBuilder.makeNewInvocation(type, defaultCasts);
}
+ private RexNode convertItem(
+ @UnknownInitialization StandardConvertletTable this,
+ SqlRexContext cx,
+ SqlCall call) {
+ assert call.operandCount() == 2;
Review comment:
Should the assert be removed then?
I think assert should have a corresponding clarification (e.g. `"ITEM should
have 2 arguments, but given " + call.opearndCount() + ", operands: " +
call.operands`) or it should be removed.
Suppose the assertion fails. Who would analyze the nature of the failure?
What if the assertion was not really needed in the first place? Currently, it
is not that easy to tell why exactly 2 operands expected here.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]