[CALCITE-3424] AssertionError thrown for user-defined table function with array argument by ihuzenko · Pull Request #1519 · apache/calcite

ihuzenko · 2019-10-18T10:07:32Z

No description provided.

danny0405 · 2019-10-18T10:20:09Z

core/src/test/java/org/apache/calcite/util/Smalls.java

  }

+  public static QueryableTable generateStrings2(final List<Integer> list) {
+    return generateStrings(list.size());


Can we give it a more meaning name ? not XXX2 ?

good catch, done.

core/src/main/java/org/apache/calcite/jdbc/JavaTypeFactoryImpl.java

danny0405 · 2019-10-18T11:17:02Z

core/src/main/java/org/apache/calcite/jdbc/JavaTypeFactoryImpl.java

+        nonNullType = typeFactory.createArrayType(typeFactory.createSqlType(SqlTypeName.ANY), -1);
+      } else {
+        nonNullType = typeFactory.createSqlType(type.getSqlTypeName());
+      }


Can you reference JavaToSqlTypeConversionRules for the type mapping ?

There is no need to use JavaToSqlTypeConversionRules here because type contains SqlTypeName. Most probably this check just need to be rewritten to switch case based on the enum values and further java class investigation isn't necessary here. To be honest the API of typeFactory which promises to createSqlType(...) for sqlTypeName and implicitly throws assertion errors is really bad.

I didn't mean to use JavaToSqlTypeConversionRules directly, i meant to match all the java collection classes that synced with what JavaToSqlTypeConversionRules has already matched.

yanlin-Lynn · 2019-10-18T11:17:57Z

core/src/main/java/org/apache/calcite/jdbc/JavaTypeFactoryImpl.java

-          type.isNullable());
+      Class javaClass = ((JavaType) type).getJavaClass();
+      final RelDataType nonNullType;
+      if (javaClass.isArray()) {


what about map type?

Issue described in Jira ticket relates to ARRAY assertion inside SqlTypeFactoryImpl's assertBasic method:

private void assertBasic(SqlTypeName typeName) { assert typeName != null; assert typeName != SqlTypeName.MULTISET : "use createMultisetType() instead"; assert typeName != SqlTypeName.ARRAY : "use createArrayType() instead"; assert typeName != SqlTypeName.ROW : "use createStructType() instead"; assert !SqlTypeName.INTERVAL_TYPES.contains(typeName) : "use createSqlIntervalType() instead"; }

For map type assertion isn't fired here and I think it's rare case that someone will accept map arguments (or others from the method body) into custom table function.

I'm not sure is it a bug or is just designed like this.
For me, it's probably a bug.
I create a PR(#1521) trying to fix it.

yanlin-Lynn · 2019-10-18T11:18:20Z

core/src/test/java/org/apache/calcite/test/TableFunctionTest.java

    }
  }

+  @Test public void testTableFunctionWithArrayParameter() throws SQLException {


maybe add a case for map type parameter?

I think array is enough, just like what the commit describes.

jinxing64 · 2019-10-19T04:31:05Z

core/src/main/java/org/apache/calcite/jdbc/JavaTypeFactoryImpl.java

+      final RelDataType relDataType;
+      if (sqlTypeName == SqlTypeName.ARRAY) {
+        RelDataType elementType = type.getComponentType() == null
+            ? typeFactory.createSqlType(SqlTypeName.ANY) : type.getComponentType();


Does it need special consideration when it's a nested array, i.e. the element type of array is also of array

Could you please give an example of table function for which it's necessary to accept nested array as argument ?

You can use SqlTypeUtil#isArray to decide if a type is ARRAY type.

danny0405 · 2019-10-22T02:29:20Z

core/src/test/java/org/apache/calcite/test/TableFunctionTest.java

    }
  }

+  @Test public void testTableFunctionWithArrayParameter() throws SQLException {


I think array is enough, just like what the commit describes.

danny0405 · 2019-10-22T02:31:39Z

core/src/main/java/org/apache/calcite/jdbc/JavaTypeFactoryImpl.java

+      final RelDataType relDataType;
+      if (sqlTypeName == SqlTypeName.ARRAY) {
+        RelDataType elementType = type.getComponentType() == null
+            ? typeFactory.createSqlType(SqlTypeName.ANY) : type.getComponentType();


You can use SqlTypeUtil#isArray to decide if a type is ARRAY type.

…with array argument

ihuzenko · 2019-10-22T11:52:41Z

Hi @danny0405 , I've updated JavaTypeFactoryImpl according to your suggestion, rebased on latest master and squashed commits. Please take a look again.

…with array argument (Igor Guzenko) close apache#1519

…with array argument (Igor Guzenko) close apache#1519 Change-Id: I1e974383412a2731ab601d98b6f633bb92e859df

danny0405 force-pushed the master branch from a39cab3 to cd24cae Compare October 18, 2019 10:16

danny0405 reviewed Oct 18, 2019

View reviewed changes

core/src/main/java/org/apache/calcite/jdbc/JavaTypeFactoryImpl.java Outdated Show resolved Hide resolved

danny0405 reviewed Oct 18, 2019

View reviewed changes

yanlin-Lynn reviewed Oct 18, 2019

View reviewed changes

jinxing64 reviewed Oct 19, 2019

View reviewed changes

danny0405 requested changes Oct 22, 2019

View reviewed changes

[CALCITE-3424] AssertionError thrown for user-defined table function …

050c368

…with array argument

ihuzenko force-pushed the CALCITE-3424 branch from cda24dc to 050c368 Compare October 22, 2019 11:49

danny0405 added the LGTM-will-merge-soon Overall PR looks OK. Only minor things left. label Oct 22, 2019

danny0405 closed this in 8ba2d85 Oct 23, 2019

yanlin-Lynn mentioned this pull request Oct 29, 2019

[CALCITE-3429] AssertionError for user-defined table function with map argument #1521

Closed

XuQianJin-Stars pushed a commit to XuQianJin-Stars/calcite that referenced this pull request Nov 17, 2019

[CALCITE-3424] AssertionError thrown for user-defined table function …

3afd999

…with array argument (Igor Guzenko) close apache#1519

XuQianJin-Stars pushed a commit to XuQianJin-Stars/calcite that referenced this pull request Nov 20, 2019

[CALCITE-3424] AssertionError thrown for user-defined table function …

3cb06cf

…with array argument (Igor Guzenko) close apache#1519

XuQianJin-Stars pushed a commit to XuQianJin-Stars/calcite that referenced this pull request Nov 20, 2019

[CALCITE-3424] AssertionError thrown for user-defined table function …

7ad85c2

…with array argument (Igor Guzenko) close apache#1519

wangxlong pushed a commit to wangxlong/calcite that referenced this pull request Feb 13, 2020

[CALCITE-3424] AssertionError thrown for user-defined table function …

bec7256

…with array argument (Igor Guzenko) close apache#1519

jamesstarr pushed a commit to jamesstarr/calcite that referenced this pull request Aug 28, 2025

[CALCITE-3424] AssertionError thrown for user-defined table function …

e6c0593

…with array argument (Igor Guzenko) close apache#1519 Change-Id: I1e974383412a2731ab601d98b6f633bb92e859df

jamesstarr pushed a commit to jamesstarr/calcite that referenced this pull request Mar 16, 2026

[CALCITE-3424] AssertionError thrown for user-defined table function …

558b809

…with array argument (Igor Guzenko) close apache#1519 Change-Id: I1e974383412a2731ab601d98b6f633bb92e859df

Conversation

ihuzenko commented Oct 18, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ihuzenko Oct 18, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yanlin-Lynn Oct 18, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yanlin-Lynn Oct 18, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ihuzenko commented Oct 22, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

ihuzenko Oct 18, 2019 •

edited

Loading

yanlin-Lynn Oct 18, 2019 •

edited

Loading

yanlin-Lynn Oct 18, 2019 •

edited

Loading