You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@calcite.apache.org by Joshua Maurice <jo...@gmail.com> on 2022/06/22 07:11:47 UTC
CALCITE-4382: "CompileException on table with array of struct column"
Hi all,
Sorry if this is the wrong list.
I have a question regarding CALCITE-4382: "CompileException on table
with array of struct column". I've been playing with the code, and
I've managed to make nested arrays and structs of all kinds work. I'd
like to merge the work to github, but I'm pretty sure it's not in a
state that is merge ready, and I'd like to ask this group for some
help on that.
The primary change is to EnumerationTableScan.fieldExpression(). The
important part of the diff:
- final MethodCallExpression e2 =
- Expressions.call(BuiltInMethod.AS_ENUMERABLE2.method, e);
- final Expression e3 = elementPhysType.convertTo(e2,
JavaRowFormat.LIST);
- return Expressions.call(e3, BuiltInMethod.ENUMERABLE_TO_LIST.method);
+ final Expression e2 = toEnumerable(e);
+ return Expressions.call(e2, BuiltInMethod.ENUMERABLE_TO_LIST.method);
My changes in brief:
First, the value in the array is List, but the (static) type of the
Expression is Object, and so I had to enhance this code. I used
toEnumerable and enhanced it to do the right thing.
Second, I entirely removed all reference to the physical-type and the
physical-type convert. It works for my simple test case, but this
makes me very worried that I'm doing something wrong. However, I've
looked over some of the history of the file (including the comment in
this function), and it seems like there was a conscious effort to
change all sql Arrays to java.util.List, and so maybe the conversion
from physical type to List has already happened somewhere else(?).
Including full diff against head at end of message.
PS: I notice that nested values for nested arrays returned from
AvaticaResultSet are sometimes java.util.List and sometimes
java.sql.Array. I barely looked into this, and it might be a bug in
Avatica.
$ git diff | cat -
diff --git a/core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableTableScan.java
b/core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableTableScan.java
index 3f3d95f..cfeb7c3 100644
--- a/core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableTableScan.java
+++ b/core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableTableScan.java
@@ -16,7 +16,6 @@
*/
package org.apache.calcite.adapter.enumerable;
-import org.apache.calcite.adapter.java.JavaTypeFactory;
import org.apache.calcite.config.CalciteSystemProperty;
import org.apache.calcite.interpreter.Row;
import org.apache.calcite.linq4j.Enumerable;
@@ -25,7 +24,6 @@
import org.apache.calcite.linq4j.tree.Blocks;
import org.apache.calcite.linq4j.tree.Expression;
import org.apache.calcite.linq4j.tree.Expressions;
-import org.apache.calcite.linq4j.tree.MethodCallExpression;
import org.apache.calcite.linq4j.tree.ParameterExpression;
import org.apache.calcite.linq4j.tree.Primitive;
import org.apache.calcite.linq4j.tree.Types;
@@ -211,7 +209,9 @@ private Expression getExpression(PhysType physType) {
private static Expression toEnumerable(Expression expression) {
final Type type = expression.getType();
- if (Types.isArray(type)) {
+ if (Types.isAssignableFrom(Enumerable.class, type)) {
+ return expression;
+ } else if (Types.isArray(type)) {
if (requireNonNull(toClass(type).getComponentType()).isPrimitive()) {
expression =
Expressions.call(BuiltInMethod.AS_LIST.method, expression);
@@ -227,8 +227,9 @@ private static Expression toEnumerable(Expression
expression) {
// evaluated directly.
return Expressions.call(expression,
BuiltInMethod.QUERYABLE_AS_ENUMERABLE.method);
+ } else {
+ return Expressions.call(BuiltInMethod.TO_ENUMERABLE.method, expression);
}
- return expression;
}
private Expression toRows(PhysType physType, Expression expression) {
@@ -273,14 +274,8 @@ private Expression
fieldExpression(ParameterExpression row_, int i,
// the consumer does not know the element type.
// The standard element type is List.
// We need to convert to a List<List>.
- final JavaTypeFactory typeFactory =
- (JavaTypeFactory) getCluster().getTypeFactory();
- final PhysType elementPhysType = PhysTypeImpl.of(
- typeFactory, fieldType, JavaRowFormat.CUSTOM);
- final MethodCallExpression e2 =
- Expressions.call(BuiltInMethod.AS_ENUMERABLE2.method, e);
- final Expression e3 = elementPhysType.convertTo(e2,
JavaRowFormat.LIST);
- return Expressions.call(e3, BuiltInMethod.ENUMERABLE_TO_LIST.method);
+ final Expression e2 = toEnumerable(e);
+ return Expressions.call(e2, BuiltInMethod.ENUMERABLE_TO_LIST.method);
} else {
return e;
}
diff --git a/core/src/main/java/org/apache/calcite/runtime/Utilities.java
b/core/src/main/java/org/apache/calcite/runtime/Utilities.java
index 458a07c..360e75e 100644
--- a/core/src/main/java/org/apache/calcite/runtime/Utilities.java
+++ b/core/src/main/java/org/apache/calcite/runtime/Utilities.java
@@ -16,6 +16,9 @@
*/
package org.apache.calcite.runtime;
+import org.apache.calcite.linq4j.Enumerable;
+import org.apache.calcite.linq4j.Linq4j;
+
import org.checkerframework.checker.nullness.qual.Nullable;
import java.text.Collator;
@@ -260,4 +263,23 @@ public static Collator generateCollator(Locale
locale, int strength) {
collator.setStrength(strength);
return collator;
}
+
+ public static Enumerable toEnumerable(@Nullable Object v0) {
+ if (v0 == null) {
+ return null;
+ } else if (v0 instanceof Enumerable) {
+ return (Enumerable) v0;
+ } else if (v0 instanceof java.util.List) {
+ return Linq4j.asEnumerable((java.util.List) v0);
+ } else if (v0 instanceof java.util.Collection) {
+ return Linq4j.asEnumerable((java.util.Collection) v0);
+ } else if (v0 instanceof Iterable) {
+ return Linq4j.asEnumerable((Iterable) v0);
+ } else if (v0 instanceof Object[]) {
+ return Linq4j.asEnumerable((Object[]) v0);
+ } else {
+ throw new RuntimeException("Utilities.asEnumerable: Unsupported type: "
+ + v0.getClass().getName());
+ }
+ }
}
diff --git a/core/src/main/java/org/apache/calcite/util/BuiltInMethod.java
b/core/src/main/java/org/apache/calcite/util/BuiltInMethod.java
index e1089a8..1c3d952 100644
--- a/core/src/main/java/org/apache/calcite/util/BuiltInMethod.java
+++ b/core/src/main/java/org/apache/calcite/util/BuiltInMethod.java
@@ -280,6 +280,7 @@
IDENTITY_SELECTOR(Functions.class, "identitySelector"),
AS_ENUMERABLE(Linq4j.class, "asEnumerable", Object[].class),
AS_ENUMERABLE2(Linq4j.class, "asEnumerable", Iterable.class),
+ TO_ENUMERABLE(Utilities.class, "toEnumerable", Object.class),
ENUMERABLE_TO_LIST(ExtendedEnumerable.class, "toList"),
AS_LIST(Primitive.class, "asList", Object.class),
MEMORY_GET0(MemoryFactory.Memory.class, "get"),
Re: CALCITE-4382: "CompileException on table with array of struct column"
Posted by Julian Hyde <jh...@gmail.com>.
Joshua,
The usual way we work is to create a GitHub pull request. If it’s not complete, you can mark it ‘draft’, so no one would merge it to main, but people could at least look at your work. Link the pull request to https://issues.apache.org/jira/browse/CALCITE-4382 <https://issues.apache.org/jira/browse/CALCITE-4382>. Then we can have a conversation in the comments of that case.
Can someone please volunteer to review Joshua’s work and help him out?
Julian
> On Jun 22, 2022, at 12:11 AM, Joshua Maurice <jo...@gmail.com> wrote:
>
> Hi all,
> Sorry if this is the wrong list.
>
> I have a question regarding CALCITE-4382: "CompileException on table
> with array of struct column". I've been playing with the code, and
> I've managed to make nested arrays and structs of all kinds work. I'd
> like to merge the work to github, but I'm pretty sure it's not in a
> state that is merge ready, and I'd like to ask this group for some
> help on that.
>
> The primary change is to EnumerationTableScan.fieldExpression(). The
> important part of the diff:
> - final MethodCallExpression e2 =
> - Expressions.call(BuiltInMethod.AS_ENUMERABLE2.method, e);
> - final Expression e3 = elementPhysType.convertTo(e2,
> JavaRowFormat.LIST);
> - return Expressions.call(e3, BuiltInMethod.ENUMERABLE_TO_LIST.method);
> + final Expression e2 = toEnumerable(e);
> + return Expressions.call(e2, BuiltInMethod.ENUMERABLE_TO_LIST.method);
>
> My changes in brief:
>
> First, the value in the array is List, but the (static) type of the
> Expression is Object, and so I had to enhance this code. I used
> toEnumerable and enhanced it to do the right thing.
>
> Second, I entirely removed all reference to the physical-type and the
> physical-type convert. It works for my simple test case, but this
> makes me very worried that I'm doing something wrong. However, I've
> looked over some of the history of the file (including the comment in
> this function), and it seems like there was a conscious effort to
> change all sql Arrays to java.util.List, and so maybe the conversion
> from physical type to List has already happened somewhere else(?).
>
> Including full diff against head at end of message.
>
> PS: I notice that nested values for nested arrays returned from
> AvaticaResultSet are sometimes java.util.List and sometimes
> java.sql.Array. I barely looked into this, and it might be a bug in
> Avatica.
>
> $ git diff | cat -
> diff --git a/core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableTableScan.java
> b/core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableTableScan.java
> index 3f3d95f..cfeb7c3 100644
> --- a/core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableTableScan.java
> +++ b/core/src/main/java/org/apache/calcite/adapter/enumerable/EnumerableTableScan.java
> @@ -16,7 +16,6 @@
> */
> package org.apache.calcite.adapter.enumerable;
>
> -import org.apache.calcite.adapter.java.JavaTypeFactory;
> import org.apache.calcite.config.CalciteSystemProperty;
> import org.apache.calcite.interpreter.Row;
> import org.apache.calcite.linq4j.Enumerable;
> @@ -25,7 +24,6 @@
> import org.apache.calcite.linq4j.tree.Blocks;
> import org.apache.calcite.linq4j.tree.Expression;
> import org.apache.calcite.linq4j.tree.Expressions;
> -import org.apache.calcite.linq4j.tree.MethodCallExpression;
> import org.apache.calcite.linq4j.tree.ParameterExpression;
> import org.apache.calcite.linq4j.tree.Primitive;
> import org.apache.calcite.linq4j.tree.Types;
> @@ -211,7 +209,9 @@ private Expression getExpression(PhysType physType) {
>
> private static Expression toEnumerable(Expression expression) {
> final Type type = expression.getType();
> - if (Types.isArray(type)) {
> + if (Types.isAssignableFrom(Enumerable.class, type)) {
> + return expression;
> + } else if (Types.isArray(type)) {
> if (requireNonNull(toClass(type).getComponentType()).isPrimitive()) {
> expression =
> Expressions.call(BuiltInMethod.AS_LIST.method, expression);
> @@ -227,8 +227,9 @@ private static Expression toEnumerable(Expression
> expression) {
> // evaluated directly.
> return Expressions.call(expression,
> BuiltInMethod.QUERYABLE_AS_ENUMERABLE.method);
> + } else {
> + return Expressions.call(BuiltInMethod.TO_ENUMERABLE.method, expression);
> }
> - return expression;
> }
>
> private Expression toRows(PhysType physType, Expression expression) {
> @@ -273,14 +274,8 @@ private Expression
> fieldExpression(ParameterExpression row_, int i,
> // the consumer does not know the element type.
> // The standard element type is List.
> // We need to convert to a List<List>.
> - final JavaTypeFactory typeFactory =
> - (JavaTypeFactory) getCluster().getTypeFactory();
> - final PhysType elementPhysType = PhysTypeImpl.of(
> - typeFactory, fieldType, JavaRowFormat.CUSTOM);
> - final MethodCallExpression e2 =
> - Expressions.call(BuiltInMethod.AS_ENUMERABLE2.method, e);
> - final Expression e3 = elementPhysType.convertTo(e2,
> JavaRowFormat.LIST);
> - return Expressions.call(e3, BuiltInMethod.ENUMERABLE_TO_LIST.method);
> + final Expression e2 = toEnumerable(e);
> + return Expressions.call(e2, BuiltInMethod.ENUMERABLE_TO_LIST.method);
> } else {
> return e;
> }
> diff --git a/core/src/main/java/org/apache/calcite/runtime/Utilities.java
> b/core/src/main/java/org/apache/calcite/runtime/Utilities.java
> index 458a07c..360e75e 100644
> --- a/core/src/main/java/org/apache/calcite/runtime/Utilities.java
> +++ b/core/src/main/java/org/apache/calcite/runtime/Utilities.java
> @@ -16,6 +16,9 @@
> */
> package org.apache.calcite.runtime;
>
> +import org.apache.calcite.linq4j.Enumerable;
> +import org.apache.calcite.linq4j.Linq4j;
> +
> import org.checkerframework.checker.nullness.qual.Nullable;
>
> import java.text.Collator;
> @@ -260,4 +263,23 @@ public static Collator generateCollator(Locale
> locale, int strength) {
> collator.setStrength(strength);
> return collator;
> }
> +
> + public static Enumerable toEnumerable(@Nullable Object v0) {
> + if (v0 == null) {
> + return null;
> + } else if (v0 instanceof Enumerable) {
> + return (Enumerable) v0;
> + } else if (v0 instanceof java.util.List) {
> + return Linq4j.asEnumerable((java.util.List) v0);
> + } else if (v0 instanceof java.util.Collection) {
> + return Linq4j.asEnumerable((java.util.Collection) v0);
> + } else if (v0 instanceof Iterable) {
> + return Linq4j.asEnumerable((Iterable) v0);
> + } else if (v0 instanceof Object[]) {
> + return Linq4j.asEnumerable((Object[]) v0);
> + } else {
> + throw new RuntimeException("Utilities.asEnumerable: Unsupported type: "
> + + v0.getClass().getName());
> + }
> + }
> }
> diff --git a/core/src/main/java/org/apache/calcite/util/BuiltInMethod.java
> b/core/src/main/java/org/apache/calcite/util/BuiltInMethod.java
> index e1089a8..1c3d952 100644
> --- a/core/src/main/java/org/apache/calcite/util/BuiltInMethod.java
> +++ b/core/src/main/java/org/apache/calcite/util/BuiltInMethod.java
> @@ -280,6 +280,7 @@
> IDENTITY_SELECTOR(Functions.class, "identitySelector"),
> AS_ENUMERABLE(Linq4j.class, "asEnumerable", Object[].class),
> AS_ENUMERABLE2(Linq4j.class, "asEnumerable", Iterable.class),
> + TO_ENUMERABLE(Utilities.class, "toEnumerable", Object.class),
> ENUMERABLE_TO_LIST(ExtendedEnumerable.class, "toList"),
> AS_LIST(Primitive.class, "asList", Object.class),
> MEMORY_GET0(MemoryFactory.Memory.class, "get"),