You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@calcite.apache.org by da...@apache.org on 2020/07/29 03:23:50 UTC

[calcite] branch master updated: [CALCITE-4145] Exception when query from UDF field with structured type

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

danny0405 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 c52f0e5  [CALCITE-4145] Exception when query from UDF field with structured type
c52f0e5 is described below

commit c52f0e527c86f6a6b18e138310359ac0c72ca529
Author: yuzhao.cyz <yu...@gmail.com>
AuthorDate: Wed Jul 29 10:49:50 2020 +0800

    [CALCITE-4145] Exception when query from UDF field with structured type
---
 .../org/apache/calcite/rex/RexFieldAccess.java     | 13 ++++++++-
 .../org/apache/calcite/sql/type/SqlTypeUtil.java   |  8 ++++++
 .../apache/calcite/test/MockSqlOperatorTable.java  | 32 ++++++++++++++++++----
 .../apache/calcite/test/SqlToRelConverterTest.java | 11 ++++++++
 .../apache/calcite/test/SqlToRelConverterTest.xml  | 12 ++++++++
 5 files changed, 70 insertions(+), 6 deletions(-)

diff --git a/core/src/main/java/org/apache/calcite/rex/RexFieldAccess.java b/core/src/main/java/org/apache/calcite/rex/RexFieldAccess.java
index a7b318e..bd05202 100644
--- a/core/src/main/java/org/apache/calcite/rex/RexFieldAccess.java
+++ b/core/src/main/java/org/apache/calcite/rex/RexFieldAccess.java
@@ -20,6 +20,8 @@ import org.apache.calcite.rel.type.RelDataType;
 import org.apache.calcite.rel.type.RelDataTypeField;
 import org.apache.calcite.sql.SqlKind;
 
+import com.google.common.base.Preconditions;
+
 /**
  * Access to a field of a row-expression.
  *
@@ -57,14 +59,23 @@ public class RexFieldAccess extends RexNode {
   RexFieldAccess(
       RexNode expr,
       RelDataTypeField field) {
+    checkValid(expr, field);
     this.expr = expr;
     this.field = field;
     this.digest = expr + "." + field.getName();
-    assert expr.getType().getFieldList().get(field.getIndex()) == field;
   }
 
   //~ Methods ----------------------------------------------------------------
 
+  private static void checkValid(RexNode expr, RelDataTypeField field) {
+    RelDataType exprType = expr.getType();
+    int fieldIdx = field.getIndex();
+    Preconditions.checkArgument(
+        fieldIdx < exprType.getFieldList().size()
+            && exprType.getFieldList().get(fieldIdx).equals(field),
+        "Field " + field + " does not exist for expression " + expr);
+  }
+
   public RelDataTypeField getField() {
     return field;
   }
diff --git a/core/src/main/java/org/apache/calcite/sql/type/SqlTypeUtil.java b/core/src/main/java/org/apache/calcite/sql/type/SqlTypeUtil.java
index eb1aafd..3ba355c 100644
--- a/core/src/main/java/org/apache/calcite/sql/type/SqlTypeUtil.java
+++ b/core/src/main/java/org/apache/calcite/sql/type/SqlTypeUtil.java
@@ -1261,6 +1261,10 @@ public abstract class SqlTypeUtil {
    * Returns the ordinal of a given field in a record type, or -1 if the field
    * is not found.
    *
+   * <p>The {@code fieldName} is always simple, if the field is nested within a record field,
+   * returns index of the outer field instead. i.g. for row type
+   * (a int, b (b1 bigint, b2 varchar(20) not null)), returns 1 for both simple name "b1" and "b2".
+   *
    * @param type      Record type
    * @param fieldName Name of field
    * @return Ordinal of field
@@ -1272,6 +1276,10 @@ public abstract class SqlTypeUtil {
       if (field.getName().equals(fieldName)) {
         return i;
       }
+      final RelDataType fieldType = field.getType();
+      if (fieldType.isStruct() && findField(fieldType, fieldName) != -1) {
+        return i;
+      }
     }
     return -1;
   }
diff --git a/core/src/test/java/org/apache/calcite/test/MockSqlOperatorTable.java b/core/src/test/java/org/apache/calcite/test/MockSqlOperatorTable.java
index 3cced47..8ebcd19 100644
--- a/core/src/test/java/org/apache/calcite/test/MockSqlOperatorTable.java
+++ b/core/src/test/java/org/apache/calcite/test/MockSqlOperatorTable.java
@@ -69,9 +69,10 @@ public class MockSqlOperatorTable extends ChainedSqlOperatorTable {
     opTab.addOperator(new RowFunction());
     opTab.addOperator(new NotATableFunction());
     opTab.addOperator(new BadTableFunction());
+    opTab.addOperator(new StructuredFunction());
   }
 
-  /** "RAMP" user-defined function. */
+  /** "RAMP" user-defined table function. */
   public static class RampFunction extends SqlFunction
       implements SqlTableFunction {
     public RampFunction() {
@@ -128,7 +129,7 @@ public class MockSqlOperatorTable extends ChainedSqlOperatorTable {
     }
   }
 
-  /** "DEDUP" user-defined function. */
+  /** "DEDUP" user-defined table function. */
   public static class DedupFunction extends SqlFunction
       implements SqlTableFunction {
     public DedupFunction() {
@@ -211,13 +212,13 @@ public class MockSqlOperatorTable extends ChainedSqlOperatorTable {
     }
   }
 
-  /** "ROW_FUNC" user-defined function whose return type is
-   * nullable row type with non-nullable fields. */
+  /** "ROW_FUNC" user-defined table function whose return type is
+   * row type with nullable and non-nullable fields. */
   public static class RowFunction extends SqlFunction
       implements SqlTableFunction {
     RowFunction() {
       super("ROW_FUNC", SqlKind.OTHER_FUNCTION, ReturnTypes.CURSOR, null,
-          OperandTypes.NILADIC, SqlFunctionCategory.USER_DEFINED_FUNCTION);
+          OperandTypes.NILADIC, SqlFunctionCategory.USER_DEFINED_TABLE_FUNCTION);
     }
 
     private static RelDataType inferRowType(SqlOperatorBinding opBinding) {
@@ -234,4 +235,25 @@ public class MockSqlOperatorTable extends ChainedSqlOperatorTable {
       return RowFunction::inferRowType;
     }
   }
+
+  /** "STRUCTURED_FUNC" user-defined function whose return type is structured type. */
+  public static class StructuredFunction extends SqlFunction {
+    StructuredFunction() {
+      super("STRUCTURED_FUNC", new SqlIdentifier("STRUCTURED_FUNC", SqlParserPos.ZERO),
+          SqlKind.OTHER_FUNCTION, null, null, OperandTypes.NILADIC, null,
+          SqlFunctionCategory.USER_DEFINED_FUNCTION);
+    }
+
+    @Override public RelDataType inferReturnType(SqlOperatorBinding opBinding) {
+      final RelDataTypeFactory typeFactory = opBinding.getTypeFactory();
+      final RelDataType bigintType =
+          typeFactory.createSqlType(SqlTypeName.BIGINT);
+      final RelDataType varcharType =
+          typeFactory.createSqlType(SqlTypeName.VARCHAR, 20);
+      return typeFactory.builder()
+          .add("F0", bigintType)
+          .add("F1", varcharType)
+          .build();
+    }
+  }
 }
diff --git a/core/src/test/java/org/apache/calcite/test/SqlToRelConverterTest.java b/core/src/test/java/org/apache/calcite/test/SqlToRelConverterTest.java
index 4bb6ed0..d560dc1 100644
--- a/core/src/test/java/org/apache/calcite/test/SqlToRelConverterTest.java
+++ b/core/src/test/java/org/apache/calcite/test/SqlToRelConverterTest.java
@@ -3825,6 +3825,17 @@ class SqlToRelConverterTest extends SqlToRelTestBase {
 
   /**
    * Test case for
+   * <a href="https://issues.apache.org/jira/browse/CALCITE-4145">[CALCITE-4145]
+   * Exception when query from UDF field with structured type</a>.
+   */
+  @Test void testUdfWithStructuredReturnType() {
+    final String sql = "SELECT deptno, tmp.r.f0, tmp.r.f1 FROM\n"
+        + "(SELECT deptno, STRUCTURED_FUNC() AS r from dept)tmp";
+    sql(sql).ok();
+  }
+
+  /**
+   * Test case for
    * <a href="https://issues.apache.org/jira/browse/CALCITE-3826">[CALCITE-3826]
    * UPDATE assigns wrong type to bind variables</a>.
    */
diff --git a/core/src/test/resources/org/apache/calcite/test/SqlToRelConverterTest.xml b/core/src/test/resources/org/apache/calcite/test/SqlToRelConverterTest.xml
index 178e2e6..3b45f50 100644
--- a/core/src/test/resources/org/apache/calcite/test/SqlToRelConverterTest.xml
+++ b/core/src/test/resources/org/apache/calcite/test/SqlToRelConverterTest.xml
@@ -6864,4 +6864,16 @@ LogicalProject(EXPR$0=[CASE(IS NOT NULL($3), CAST($3):INTEGER NOT NULL, 0)])
 ]]>
         </Resource>
     </TestCase>
+    <TestCase name="testUdfWithStructuredReturnType">
+        <Resource name="sql">
+            <![CDATA[SELECT deptno, tmp.r.f0, tmp.r.f1 FROM
+(SELECT deptno, STRUCTURED_FUNC() AS r from dept)tmp]]>
+        </Resource>
+        <Resource name="plan">
+            <![CDATA[
+LogicalProject(DEPTNO=[$0], F0=[STRUCTURED_FUNC().F0], F1=[STRUCTURED_FUNC().F1])
+  LogicalTableScan(table=[[CATALOG, SALES, DEPT]])
+]]>
+        </Resource>
+    </TestCase>
 </Root>