This is an automated email from the ASF dual-hosted git repository.

zabetak pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/calcite.git


The following commit(s) were added to refs/heads/master by this push:
     new 3f0353c  Following [CALCITE-4354], control field index access at 
runtime via feature flag (Alessandro Solimando)
3f0353c is described below

commit 3f0353cecfbdfa3b85e6a4fe11946df912ebd2b1
Author: Alessandro Solimando <[email protected]>
AuthorDate: Tue Nov 17 23:30:57 2020 +0100

    Following [CALCITE-4354], control field index access at runtime via feature 
flag (Alessandro Solimando)
    
    The runtime implementation for index based access is experimental since it 
is
    JVM-dependent (see CALCITE-2489), thus it is disabled by default.
    
    Nevertheless, the property is enabled by default during tests for better
    coverage.
    
    Close apache/calcite#2267
---
 build.gradle.kts                                           |  2 ++
 .../org/apache/calcite/config/CalciteSystemProperty.java   |  9 +++++++++
 .../java/org/apache/calcite/runtime/CalciteResource.java   |  5 +++--
 .../main/java/org/apache/calcite/runtime/SqlFunctions.java | 14 ++++++++++++--
 .../org/apache/calcite/runtime/CalciteResource.properties  |  2 +-
 core/src/test/java/org/apache/calcite/test/QuidemTest.java | 10 ++++++++++
 core/src/test/resources/sql/operator.iq                    |  2 ++
 7 files changed, 39 insertions(+), 5 deletions(-)

diff --git a/build.gradle.kts b/build.gradle.kts
index 764c4ef..e6e3742 100644
--- a/build.gradle.kts
+++ b/build.gradle.kts
@@ -611,6 +611,8 @@ allprojects {
                 passProperty("junit.jupiter.execution.timeout.default", "5 m")
                 passProperty("user.language", "TR")
                 passProperty("user.country", "tr")
+                // For better test coverage field index access should be 
enabled.
+                passProperty("calcite.enable.enumerable.fieldIndexAccess", 
"true")
                 val props = System.getProperties()
                 for (e in props.propertyNames() as 
`java.util`.Enumeration<String>) {
                     if (e.startsWith("calcite.") || e.startsWith("avatica.")) {
diff --git 
a/core/src/main/java/org/apache/calcite/config/CalciteSystemProperty.java 
b/core/src/main/java/org/apache/calcite/config/CalciteSystemProperty.java
index ac27b01..01f8cf9 100644
--- a/core/src/main/java/org/apache/calcite/config/CalciteSystemProperty.java
+++ b/core/src/main/java/org/apache/calcite/config/CalciteSystemProperty.java
@@ -91,6 +91,15 @@ public final class CalciteSystemProperty<T> {
   public static final CalciteSystemProperty<Boolean> 
ENUMERABLE_ENABLE_TABLESCAN_MULTISET =
       booleanProperty("calcite.enable.enumerable.tablescan.multiset", false);
 
+  /**
+   * Whether to enable index-based access for struct fields.
+   *
+   * <p>Note: the feature is experimental as it relies on field order which is 
JVM-dependent
+   * (see CALCITE-2489).</p>
+   */
+  public static final CalciteSystemProperty<Boolean> ALLOW_FIELD_INDEX_ACCESS =
+      booleanProperty("calcite.enable.enumerable.fieldIndexAccess", false);
+
   /** Whether streaming is enabled in the default planner configuration. */
   public static final CalciteSystemProperty<Boolean> ENABLE_STREAM =
       booleanProperty("calcite.enable.stream", true);
diff --git a/core/src/main/java/org/apache/calcite/runtime/CalciteResource.java 
b/core/src/main/java/org/apache/calcite/runtime/CalciteResource.java
index 8f4cc89..2577c65 100644
--- a/core/src/main/java/org/apache/calcite/runtime/CalciteResource.java
+++ b/core/src/main/java/org/apache/calcite/runtime/CalciteResource.java
@@ -839,8 +839,9 @@ public interface CalciteResource {
   @BaseMessage("More than one value in list: {0}")
   ExInst<CalciteException> moreThanOneValueInList(String list);
 
-  @BaseMessage("Failed to access field ''{0}'' of object of type {1}")
-  ExInstWithCause<CalciteException> failedToAccessField(String fieldName, 
String typeName);
+  @BaseMessage("Failed to access field ''{0}'', index {1,number,#} of object 
of type {2}")
+  ExInstWithCause<CalciteException> failedToAccessField(
+      String fieldName, int fieldIndex, String typeName);
 
   @BaseMessage("Illegal jsonpath spec ''{0}'', format of the spec should be: 
''<lax|strict> $'{'expr'}'''")
   ExInst<CalciteException> illegalJsonPathSpec(String pathSpec);
diff --git a/core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java 
b/core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java
index 7af05e1..9776037 100644
--- a/core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java
+++ b/core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java
@@ -21,6 +21,7 @@ import org.apache.calcite.avatica.util.ByteString;
 import org.apache.calcite.avatica.util.DateTimeUtils;
 import org.apache.calcite.avatica.util.Spaces;
 import org.apache.calcite.avatica.util.TimeUnitRange;
+import org.apache.calcite.config.CalciteSystemProperty;
 import org.apache.calcite.interpreter.Row;
 import org.apache.calcite.linq4j.AbstractEnumerable;
 import org.apache.calcite.linq4j.CartesianProductEnumerator;
@@ -146,6 +147,9 @@ public class SqlFunctions {
 
   private static final Pattern PATTERN_0_STAR_E = Pattern.compile("0*E");
 
+  private static final boolean ALLOW_FIELD_INDEX_ACCESS =
+      CalciteSystemProperty.ALLOW_FIELD_INDEX_ACCESS.value();
+
   private SqlFunctions() {
   }
 
@@ -2965,13 +2969,19 @@ public class SqlFunctions {
       try {
         Field structField;
         if (fieldName == null) {
-          structField = beanClass.getDeclaredFields()[index];
+          if (ALLOW_FIELD_INDEX_ACCESS) {
+            structField = beanClass.getDeclaredFields()[index];
+          } else {
+            throw new IllegalArgumentException("fieldName is null, fieldIndex 
is " + (index + 1)
+                + ", you might add 
'calcite.enable.enumerable.fieldIndexAccess=true' to allow "
+                + "index-based field access");
+          }
         } else {
           structField = beanClass.getDeclaredField(fieldName);
         }
         return structField.get(structObject);
       } catch (NoSuchFieldException | IllegalAccessException ex) {
-        throw RESOURCE.failedToAccessField(fieldName, 
beanClass.getName()).ex(ex);
+        throw RESOURCE.failedToAccessField(fieldName, index, 
beanClass.getName()).ex(ex);
       }
     }
   }
diff --git 
a/core/src/main/resources/org/apache/calcite/runtime/CalciteResource.properties 
b/core/src/main/resources/org/apache/calcite/runtime/CalciteResource.properties
index 1ab2b11..e62b1555 100644
--- 
a/core/src/main/resources/org/apache/calcite/runtime/CalciteResource.properties
+++ 
b/core/src/main/resources/org/apache/calcite/runtime/CalciteResource.properties
@@ -274,7 +274,7 @@ InvalidTypesForComparison=Invalid types for comparison: {0} 
{1} {2}
 CannotConvert=Cannot convert {0} to {1}
 InvalidCharacterForCast=Invalid character for cast: {0}
 MoreThanOneValueInList=More than one value in list: {0}
-FailedToAccessField=Failed to access field ''{0}'' of object of type {1}
+FailedToAccessField=Failed to access field ''{0}'', index {1,number,#} of 
object of type {2}
 IllegalJsonPathSpec=Illegal jsonpath spec ''{0}'', format of the spec should 
be: ''<lax|strict> $'{'expr'}'''
 IllegalJsonPathMode=Illegal jsonpath mode ''{0}''
 IllegalJsonPathModeInPathSpec=Illegal jsonpath mode ''{0}'' in jsonpath spec: 
''{1}''
diff --git a/core/src/test/java/org/apache/calcite/test/QuidemTest.java 
b/core/src/test/java/org/apache/calcite/test/QuidemTest.java
index e0eb974..9889d5b 100644
--- a/core/src/test/java/org/apache/calcite/test/QuidemTest.java
+++ b/core/src/test/java/org/apache/calcite/test/QuidemTest.java
@@ -19,6 +19,7 @@ package org.apache.calcite.test;
 import org.apache.calcite.adapter.java.ReflectiveSchema;
 import org.apache.calcite.avatica.AvaticaUtils;
 import org.apache.calcite.config.CalciteConnectionProperty;
+import org.apache.calcite.config.CalciteSystemProperty;
 import org.apache.calcite.jdbc.CalciteConnection;
 import org.apache.calcite.prepare.Prepare;
 import org.apache.calcite.rel.type.RelDataType;
@@ -80,6 +81,15 @@ public abstract class QuidemTest {
         }
         return null;
       };
+    case "allow":
+      // Quidem requires a Java 8 function
+      return (Function<String, Object>) v -> {
+        switch (v) {
+        case "fieldIndexAccess":
+          return CalciteSystemProperty.ALLOW_FIELD_INDEX_ACCESS.value();
+        }
+        return null;
+      };
     default:
       return null;
     }
diff --git a/core/src/test/resources/sql/operator.iq 
b/core/src/test/resources/sql/operator.iq
index 904f100..db4658a 100644
--- a/core/src/test/resources/sql/operator.iq
+++ b/core/src/test/resources/sql/operator.iq
@@ -293,6 +293,7 @@ select au."birthPlace"['city'] as city from 
"bookstore"."authors" au;
 
 !ok
 
+!if (allow.fieldIndexAccess) {
 # we have "birthPlace(coords, city, country)", so city has index 2
 select au."birthPlace"[2] as city from "bookstore"."authors" au;
 
@@ -306,5 +307,6 @@ select au."birthPlace"[2] as city from 
"bookstore"."authors" au;
 (3 rows)
 
 !ok
+!}
 
 # End operator.iq

Reply via email to