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 67b41ce [CALCITE-4436] Use the fields order from the struct type for
'ITEM(STRUCT, INDEX)' access (Alessandro Solimando)
67b41ce is described below
commit 67b41cea21203b1c7541d04dc0ff6ee6b65fbe35
Author: Alessandro Solimando <[email protected]>
AuthorDate: Wed Dec 9 23:53:08 2020 +0100
[CALCITE-4436] Use the fields order from the struct type for 'ITEM(STRUCT,
INDEX)' access (Alessandro Solimando)
Close apache/calcite#2296
---
build.gradle.kts | 2 -
.../calcite/config/CalciteSystemProperty.java | 9 ----
.../org/apache/calcite/runtime/SqlFunctions.java | 16 +-------
.../calcite/sql/fun/SqlStdOperatorTable.java | 3 +-
.../calcite/sql2rel/StandardConvertletTable.java | 40 ++++++++++++++++++
.../java/org/apache/calcite/test/QuidemTest.java | 10 -----
core/src/test/resources/sql/operator.iq | 48 +++++++++++++++++++++-
7 files changed, 90 insertions(+), 38 deletions(-)
diff --git a/build.gradle.kts b/build.gradle.kts
index 9255897..40e799e 100644
--- a/build.gradle.kts
+++ b/build.gradle.kts
@@ -676,8 +676,6 @@ 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 c4b5361..4fd3a75 100644
--- a/core/src/main/java/org/apache/calcite/config/CalciteSystemProperty.java
+++ b/core/src/main/java/org/apache/calcite/config/CalciteSystemProperty.java
@@ -93,15 +93,6 @@ 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/SqlFunctions.java
b/core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java
index 1632e1e..eaab70b 100644
--- a/core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java
+++ b/core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java
@@ -21,7 +21,6 @@ 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;
@@ -145,9 +144,6 @@ 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() {
}
@@ -2996,18 +2992,10 @@ public class SqlFunctions {
} else {
Class<?> beanClass = structObject.getClass();
try {
- Field structField;
if (fieldName == null) {
- 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);
+ throw new IllegalStateException("Field name cannot be null for
struct field access");
}
+ Field structField = beanClass.getDeclaredField(fieldName);
return structField.get(structObject);
} catch (NoSuchFieldException | IllegalAccessException ex) {
throw RESOURCE.failedToAccessField(fieldName, index,
beanClass.getName()).ex(ex);
diff --git
a/core/src/main/java/org/apache/calcite/sql/fun/SqlStdOperatorTable.java
b/core/src/main/java/org/apache/calcite/sql/fun/SqlStdOperatorTable.java
index 1c6e635..a34098e 100644
--- a/core/src/main/java/org/apache/calcite/sql/fun/SqlStdOperatorTable.java
+++ b/core/src/main/java/org/apache/calcite/sql/fun/SqlStdOperatorTable.java
@@ -2079,7 +2079,8 @@ public class SqlStdOperatorTable extends
ReflectiveSqlOperatorTable {
/**
* The item operator {@code [ ... ]}, used to access a given element of an
- * array or map. For example, {@code myArray[3]} or {@code "myMap['foo']"}.
+ * array, map or struct. For example, {@code myArray[3]}, {@code
"myMap['foo']"},
+ * {@code myStruct[2]} or {@code myStruct['fieldName']}.
*
* <p>The SQL standard calls the ARRAY variant a
* <array element reference>. Index is 1-based. The standard says
diff --git
a/core/src/main/java/org/apache/calcite/sql2rel/StandardConvertletTable.java
b/core/src/main/java/org/apache/calcite/sql2rel/StandardConvertletTable.java
index 1d23d8a..0864468 100644
--- a/core/src/main/java/org/apache/calcite/sql2rel/StandardConvertletTable.java
+++ b/core/src/main/java/org/apache/calcite/sql2rel/StandardConvertletTable.java
@@ -22,6 +22,7 @@ import org.apache.calcite.plan.RelOptUtil;
import org.apache.calcite.rel.type.RelDataType;
import org.apache.calcite.rel.type.RelDataTypeFactory;
import org.apache.calcite.rel.type.RelDataTypeFamily;
+import org.apache.calcite.rel.type.RelDataTypeField;
import org.apache.calcite.rex.RexBuilder;
import org.apache.calcite.rex.RexCall;
import org.apache.calcite.rex.RexCallBinding;
@@ -47,6 +48,7 @@ import org.apache.calcite.sql.SqlNode;
import org.apache.calcite.sql.SqlNodeList;
import org.apache.calcite.sql.SqlNumericLiteral;
import org.apache.calcite.sql.SqlOperator;
+import org.apache.calcite.sql.SqlOperatorBinding;
import org.apache.calcite.sql.SqlUtil;
import org.apache.calcite.sql.SqlWindowTableFunction;
import org.apache.calcite.sql.fun.SqlArrayValueConstructor;
@@ -186,6 +188,8 @@ public class StandardConvertletTable extends
ReflectiveConvertletTable {
(cx, call) -> cx.getRexBuilder().makeFieldAccess(
cx.convertExpression(call.operand(0)),
call.operand(1).toString(), false));
+ // "ITEM"
+ registerOp(SqlStdOperatorTable.ITEM, this::convertItem);
// "AS" has no effect, so expand "x AS id" into "x".
registerOp(SqlStdOperatorTable.AS,
(cx, call) -> cx.convertExpression(call.operand(0)));
@@ -837,6 +841,42 @@ public class StandardConvertletTable extends
ReflectiveConvertletTable {
return rexBuilder.makeNewInvocation(type, defaultCasts);
}
+ private RexNode convertItem(
+ @UnknownInitialization StandardConvertletTable this,
+ SqlRexContext cx,
+ SqlCall call) {
+ final RexBuilder rexBuilder = cx.getRexBuilder();
+ final SqlOperator op = call.getOperator();
+ SqlOperandTypeChecker operandTypeChecker = op.getOperandTypeChecker();
+ final SqlOperandTypeChecker.Consistency consistency =
+ operandTypeChecker == null
+ ? SqlOperandTypeChecker.Consistency.NONE
+ : operandTypeChecker.getConsistency();
+ final List<RexNode> exprs = convertOperands(cx, call, consistency);
+
+ final RelDataType collectionType = exprs.get(0).getType();
+ final boolean isRowTypeField = SqlTypeUtil.isRow(collectionType);
+ final boolean isNumericIndex =
SqlTypeUtil.isIntType(exprs.get(1).getType());
+
+ if (isRowTypeField && isNumericIndex) {
+ final SqlOperatorBinding opBinding = new RexCallBinding(
+ cx.getTypeFactory(), op, exprs, ImmutableList.of());
+ final RelDataType operandType = opBinding.getOperandType(0);
+
+ final Integer index = opBinding.getOperandLiteralValue(1, Integer.class);
+ if (index == null || index < 1 || index > operandType.getFieldCount()) {
+ throw new AssertionError("Cannot access field at position "
+ + index + " within ROW type: " + operandType);
+ } else {
+ RelDataTypeField relDataTypeField =
collectionType.getFieldList().get(index - 1);
+ return rexBuilder.makeFieldAccess(
+ exprs.get(0), relDataTypeField.getName(), false);
+ }
+ }
+ RelDataType type = rexBuilder.deriveReturnType(op, exprs);
+ return rexBuilder.makeCall(type, op, RexUtil.flatten(exprs, op));
+ }
+
/**
* Converts a call to an operator into a {@link RexCall} to the same
* operator.
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 9889d5b..e0eb974 100644
--- a/core/src/test/java/org/apache/calcite/test/QuidemTest.java
+++ b/core/src/test/java/org/apache/calcite/test/QuidemTest.java
@@ -19,7 +19,6 @@ 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;
@@ -81,15 +80,6 @@ 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 db4658a..c9b5fd8 100644
--- a/core/src/test/resources/sql/operator.iq
+++ b/core/src/test/resources/sql/operator.iq
@@ -236,6 +236,8 @@ order by 1,2;
select "T"."X"[1] as x1 from (VALUES (ROW(ROW(3, 7), ROW(4, 8)))) as T(x, y);
+X1 INTEGER(10) NOT NULL
+!type
+----+
| X1 |
+----+
@@ -247,6 +249,8 @@ select "T"."X"[1] as x1 from (VALUES (ROW(ROW(3, 7), ROW(4,
8)))) as T(x, y);
select "T"."X"[CAST(2 AS BIGINT)] as x2 from (VALUES (ROW(ROW(3, 7), ROW(4,
8)))) as T(x, y);
+X2 INTEGER(10) NOT NULL
+!type
+----+
| X2 |
+----+
@@ -258,6 +262,8 @@ select "T"."X"[CAST(2 AS BIGINT)] as x2 from (VALUES
(ROW(ROW(3, 7), ROW(4, 8)))
select "T"."Y"[CAST(1 AS TINYINT)] as y1 from (VALUES (ROW(ROW(3, 7), ROW(4,
8)))) as T(x, y);
+Y1 INTEGER(10) NOT NULL
+!type
+----+
| Y1 |
+----+
@@ -269,6 +275,8 @@ select "T"."Y"[CAST(1 AS TINYINT)] as y1 from (VALUES
(ROW(ROW(3, 7), ROW(4, 8))
select "T"."Y"[CAST(2 AS SMALLINT)] as y2 from (VALUES (ROW(ROW(3, 7), ROW(4,
8)))) as T(x, y);
+Y2 INTEGER(10) NOT NULL
+!type
+----+
| Y2 |
+----+
@@ -282,6 +290,8 @@ select "T"."Y"[CAST(2 AS SMALLINT)] as y2 from (VALUES
(ROW(ROW(3, 7), ROW(4, 8)
select au."birthPlace"['city'] as city from "bookstore"."authors" au;
+CITY VARCHAR
+!type
+-----------+
| CITY |
+-----------+
@@ -293,10 +303,45 @@ 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;
+CITY VARCHAR
+!type
++-----------+
+| CITY |
++-----------+
+| Besançon |
+| Heraklion |
+| Ionia |
++-----------+
+(3 rows)
+
+!ok
+
+select au."birthPlace"[CAST(2 AS SMALLINT)] as city from "bookstore"."authors"
au;
+
+CITY VARCHAR
+!type
++-----------+
+| CITY |
++-----------+
+| Besançon |
+| Heraklion |
+| Ionia |
++-----------+
+(3 rows)
+
+!ok
+
+select au."birthPlace"[CAST(NULL AS INTEGER)] as city from
"bookstore"."authors" au;
+Cannot infer type of field at position null within ROW type:
RecordType(RecordType(JavaType(class java.math.BigDecimal) latitude,
JavaType(class java.math.BigDecimal) longtitude) coords, JavaType(class
java.lang.String) city, JavaType(class java.lang.String) country)
+!error
+
+select au."birthPlace"[2] as city from "bookstore"."authors" au;
+
+CITY VARCHAR
+!type
+-----------+
| CITY |
+-----------+
@@ -307,6 +352,5 @@ select au."birthPlace"[2] as city from
"bookstore"."authors" au;
(3 rows)
!ok
-!}
# End operator.iq
