asolimando commented on a change in pull request #2230:
URL: https://github.com/apache/calcite/pull/2230#discussion_r525509730
##########
File path: core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java
##########
@@ -2956,7 +2963,12 @@ public static Object structAccess(Object structObject,
int index, String fieldNa
} else {
Class<?> beanClass = structObject.getClass();
try {
- Field structField = beanClass.getDeclaredField(fieldName);
+ Field structField;
+ if (fieldName == null) {
+ structField = beanClass.getDeclaredFields()[index];
Review comment:
@vlsi thanks for taking a look, I agree on the whole line.
@zabetak thanks for asking, I can take care of implementing (b) for a start,
I just have few additional questions:
1. ~~Do we need to file a new Jira ticket for this? If not, which are the
conventions for PR name and branch name in such cases (PR closed, a fix is
needed)?~~ never mind, just found an example
[here](https://github.com/apache/calcite/pull/2265)
2. What happens to the unit-tests relying on the feature? (I had a look at
`CalciteSystemProperty.java` but there are no tests relying on enabling
properties).
If they were JUnit tests, I'd conditionally enable them based on the
`calcite.experimental.allow.field.index.access` property, but I am not sure how
to achieve that for quidem tests.
~~In addition, test `testItemOp()` in
`core/src/test/java/org/apache/calcite/sql/test/SqlOperatorBaseTest.java` has
been extended to cover the new functionality and should not be disabled
entirely IMO. Would wrapping the part relying on the extended feature with a:
```java
if (CalciteSystemProperty.FIELD_INDEX_ACCESS.value()) {
...
}
```
be OK?~~ the test does not rely on the experimental part of the feature
----------------------------------------------------------------
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]
