asolimando commented on a change in pull request #2230:
URL: https://github.com/apache/calcite/pull/2230#discussion_r512720387



##########
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:
       We support `person[$integer]` for synthetic struct types and  
`person[$field_name]` for UDTs, we should not try to access `person['1']` 
(`'1'` cannot be a field name of the UDT as it's not a valid java identifier). 
I have simplified that part, if the index is a `String` we consider it a field 
name and we let `structAccess` handle it.
   
   I have added a test in 
`core/src/test/java/org/apache/calcite/test/ReflectiveSchemaTest.java` to cover 
for the `person[$field_name]` access pattern (all the existing tests check that 
the query is legal, and that the right type is computed, none of them actually 
try to return the value at runtime).
   
   I would have loved to add it to `testItemOp()` but I am not sure how to 
write a test which loads the bookstore schema (all the tests there target 
simple expressions, and I am not sure it's OK to use `CalciteAssert`).




----------------------------------------------------------------
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]


Reply via email to