julianhyde commented on a change in pull request #2230:
URL: https://github.com/apache/calcite/pull/2230#discussion_r514463762
##########
File path: core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java
##########
@@ -2517,6 +2517,18 @@ public static Object item(Object object, Object index) {
if (object instanceof List && index instanceof Number) {
return arrayItem((List) object, ((Number) index).intValue());
}
+ if (index instanceof Number) {
+ return structAccess(object, ((Number) index).intValue() - 1, ""); // 1
indexed
+ }
+ if (index instanceof String) {
Review comment:
Glad you agree, @zabetak. `ReflectiveSchemaTest` should test the
functionality of reflective schemas, not be a test that tests something else
but uses reflective schemas because they are convenient. (We have long abused
`JdbcTest` in that way; *mea culpa*.) `SqlOperatorBaseTest` and `operator.iq`
are the appropriate places for testing operators (possibly also
`SqlValidatorTest` and `SqlParserTest`.)
The litmus test is as follows: would someone else implementing the same
feature put the test in the same place? (That has saved me a few times: I
started to implement a feature and then noticed there was already a test,
because the feature is already implemented.)
----------------------------------------------------------------
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]
