julianhyde commented on code in PR #3167:
URL: https://github.com/apache/calcite/pull/3167#discussion_r1187827849
##########
core/src/main/java/org/apache/calcite/adapter/enumerable/RexImpTable.java:
##########
@@ -3090,6 +3102,13 @@ private static class ItemImplementor extends
AbstractRexCallImplementor {
final RexCall call, final List<Expression> argValueList) {
final MethodImplementor implementor =
getImplementor(call.getOperands().get(0).getType().getSqlTypeName());
+ // If the structure is an array, two additional arguments are added to
the argValueList
+ // to denote what the offset is and what the behavior should be if the
index is out of boudns.
+ if (implementor.method.getName().equals("arrayItemOptional")) {
Review Comment:
Hard-coded method names are too fragile.
Especially strange given that getImplementor returns
BuiltInMethod.ARRAY_ITEM and there's no BuiltInMethod.ARRAY_ITEM_OPTIONAL.
Reading this code, I first thought it was because of the 'optional'. But
actually it's because it's an array.
So, figure out a way to make this code more robust and intuitive. It's not
sufficient that it works.
--
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]
