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