Following [CALCITE-4354], added 'calcite.experimental.allow.field.ind…

[asolimando]

…ex.access' feature flag

Index-based field access for struct is experimental as it relies on field order which is JVM-dependent (see CALCITE-2489) and is disabled by default, it can be activated via 'calcite.experimental.allow.field.index.access=true'.

[zabetak]

Thanks for the PR @asolimando !

LGTM

One minor comment would be to see if we can enable the property by default during tests so that we have better coverage. I guess it should be doable to pass this to the gradle scripts somewhere. I can check it out when I will merge the PR.

[asolimando]

One minor comment would be to see if we can enable the property by default during tests so that we have better coverage. I guess it should be doable to pass this to the gradle scripts somewhere. I can check it out when I will merge the PR.

Thanks, that's indeed a good idea. So far none of the CI-profiles were failing, so I guess we can activate the feature flag for all of them, and disable it in the feature we add a JVM with the problematic symptoms.

Not sure where the best place to activate the flag is, but systemProperty("calcite.enable.enumerable.fieldIndexAccess", true) looks like the way to do it in Calcite's gradle scripts.

[asolimando]

There is a test failure for the last commit: https://ci.appveyor.com/project/ApacheSoftwareFoundation/calcite/builds/36376813/job/5k2lubas8qbehk8o

But it looks unrelated to me:

GeodeBookstoreTest STANDARD_ERROR
FATAL LoggingUncaughtExceptionHandler Uncaught exception in thread Thread[ServerLauncherStopper,5,main]
java.lang.NullPointerException
at org.apache.geode.distributed.ServerLauncher.doStopInProcess(ServerLauncher.java:1253)
at java.base/java.lang.Thread.run(Thread.java:830)
FAILURE 0.0sec, org.apache.calcite.adapter.geode.rel.GeodeBookstoreTest > initializationError
java.lang.IllegalStateException: Expected state not responding but got online
at com.google.common.base.Preconditions.checkState(Preconditions.java:824)
at org.apache.calcite.adapter.geode.rel.GeodeEmbeddedPolicy.requireStatus(GeodeEmbeddedPolicy.java:94)
at org.apache.calcite.adapter.geode.rel.GeodeEmbeddedPolicy.beforeAll(GeodeEmbeddedPolicy.java:52)
at org.apache.calcite.adapter.geode.rel.GeodeEmbeddedPolicy$RefCountPolicy.beforeAll(GeodeEmbeddedPolicy.java:138)
at org.junit.jupiter.engine.descriptor.ClassBasedTestDescriptor.lambda$invokeBeforeAllCallbacks$7(ClassBasedTestDescriptor.java:355)
at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
at org.junit.jupiter.engine.descriptor.ClassBasedTestDescriptor.invokeBeforeAllCallbacks(ClassBasedTestDescriptor.java:355)
at org.junit.jupiter.engine.descriptor.ClassBasedTestDescriptor.before(ClassBasedTestDescriptor.java:189)
at org.junit.jupiter.engine.descriptor.ClassBasedTestDescriptor.before(ClassBasedTestDescriptor.java:77)
at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$5(NodeTestTask.java:132)
at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$7(NodeTestTask.java:125)
at org.junit.platform.engine.support.hierarchical.Node.around(Node.java:135)
at org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$8(NodeTestTask.java:123)
at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
at org.junit.platform.engine.support.hierarchical.NodeTestTask.executeRecursively(NodeTestTask.java:122)
at org.junit.platform.engine.support.hierarchical.NodeTestTask.execute(NodeTestTask.java:80)
FAILURE 1.4sec, 1 completed, 1 failed, 0 skipped, org.apache.calcite.adapter.geode.rel.GeodeBookstoreTest
WARNING 47.4sec, 14 completed, 0 failed, 3 skipped, org.apache.calcite.adapter.geode.rel.GeodeZipsTest
FAILURE 57.4sec, 27 completed, 1 failed, 3 skipped, Gradle Test Run :geode:test
27 tests completed, 1 failed, 3 skipped

I am pretty sure I saw a "relaunch test" button somewhere sparing the need for an empty commit to retrigger tests, but I can't find it anymore.