Uploaded image for project: 'Calcite'
  1. Calcite
  2. CALCITE-3562

Unify function's operands type check logic in validation and behavior in runtime

    XMLWordPrintableJSON

Details

    • Task
    • Status: Open
    • Major
    • Resolution: Unresolved
    • None
    • None
    • core
    • None

    Description

      Current now, there are many issues caused by inconsistency between validator and runtime phase. To summarize:

      (1)Validation phase allows a wide range of operand types, but the runtime implementation does not cover all cases.
      For example, SqlFunction(MOD) adopts OperandTypes.EXACT_NUMERIC_EXACT_NUMERIC.

      @Test public void test0() {
       final String sql = "SELECT mod(12.5, cast(1 as bigint))";
       CalciteAssert.that()
       .query(sql)
       .returns("EXPR$0=0.5\n");
       }

      We will get:

      java.lang.RuntimeException: while resolving method 'mod[class java.math.BigDecimal, long]' in class class org.apache.calcite.runtime.SqlFunctions
       at org.apache.calcite.linq4j.tree.Types.lookupMethod(Types.java:323)
       at org.apache.calcite.linq4j.tree.Expressions.call(Expressions.java:445)
       at org.apache.calcite.adapter.enumerable.RexImpTable$MethodNameImplementor.implement(RexImpTable.java:2253)
       at org.apache.calcite.adapter.enumerable.RexImpTable.implementCall(RexImpTable.java:1195)

       

      (2)Type is assignable conceptually, but in the runtime phase, explicite cast is still required.
      For example, according to SqlTypeAssignmentRules, ST_MakePoint(Decimal, Decimal) also accepts operands with (Integer, Decimal) types, because Decimal is assignable from Integer.

        @Test public void test1() {
          final String sql = "SELECT ST_MakePoint(1, 2.1)";
          CalciteAssert.that()
              .with(CalciteAssert.Config.GEO)
              .query(sql)
              .returns("EXPR$0={\"x\":1,\"y\":2.1}\n");
        }
      

      We will get:

      org.codehaus.commons.compiler.CompileException: Line 22, Column 124: No applicable constructor/method found for actual parameters "int, java.math.BigDecimal"; candidates are: "public static org.apache.calcite.runtime.GeoFunctions$Geom org.apache.calcite.runtime.GeoFunctions.ST_MakePoint(java.math.BigDecimal, java.math.BigDecimal, java.math.BigDecimal)", "public static org.apache.calcite.runtime.GeoFunctions$Geom org.apache.calcite.runtime.GeoFunctions.ST_MakePoint(java.math.BigDecimal, java.math.BigDecimal)"
       at org.codehaus.janino.UnitCompiler.compileError(UnitCompiler.java:12211)
       at org.codehaus.janino.UnitCompiler.findMostSpecificIInvocable(UnitCompiler.java:9263)
       at org.codehaus.janino.UnitCompiler.findIMethod(UnitCompiler.java:9123)
       at org.codehaus.janino.UnitCompiler.findIMethod(UnitCompiler.java:9025)
       at org.codehaus.janino.UnitCompiler.compileGet2(UnitCompiler.java:5062)
       at org.codehaus.janino.UnitCompiler.access$9100(UnitCompiler.java:215)

       

      (3)For some functions, it is too late to fail the query in runtime phase.
      For example: RAND_INTEGER adopts OperandTypes.or(OperandTypes.NUMERIC, OperandTypes.NUMERIC_NUMERIC)

      @Test public void test2() {
       final String sql = "SELECT rand_integer(1.1, 2)";
       CalciteAssert.that()
       .query(sql)
       .planContains("xyxyx")
       .returns("EXPR$0={\"x\":1,\"y\":2.1}\n");
       }

      We will get:

      org.codehaus.commons.compiler.CompileException: Line 22, Column 100: No applicable constructor/method found for actual parameters "java.math.BigDecimal, int"; candidates are: "public int org.apache.calcite.runtime.RandomFunction.randIntegerSeed(int, int)"
       at org.codehaus.janino.UnitCompiler.compileError(UnitCompiler.java:12211)
       at org.codehaus.janino.UnitCompiler.findMostSpecificIInvocable(UnitCompiler.java:9263)
       at org.codehaus.janino.UnitCompiler.findIMethod(UnitCompiler.java:9123)

       

      How to fix?

      From my personal view, for case (1) and (2), we need to fix it in runtime layer with a "try-best" mechanism to convert operand type to match the implementation.

      The difference between them: case(1) is builtin function, we cannot get exact argument types, while case(2) is udf.

      For case(3), it seems more suitable to fix in Validation phase.

      Attachments

        Activity

          People

            Unassigned Unassigned
            donnyzone Feng Zhu
            Votes:
            0 Vote for this issue
            Watchers:
            4 Start watching this issue

            Dates

              Created:
              Updated:

              Time Tracking

                Estimated:
                Original Estimate - Not Specified
                Not Specified
                Remaining:
                Remaining Estimate - 0h
                0h
                Logged:
                Time Spent - 2h 20m
                2h 20m