[ 
https://issues.apache.org/jira/browse/CALCITE-3562?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16988170#comment-16988170
 ] 

Julian Hyde commented on CALCITE-3562:
--------------------------------------

Too long for me to read all of this, but I think you are confusing user-defined 
functions and internal functions.
 * User-defined functions have argument types such as BigDecimal, Timestamp, 
Date. They aim to be convenient, not necessarily performant.
 * Internal functions, which occur in {{class SqlFunctions}}, and are only used 
by code generation, and operate on internal types. DATE is represented as 
{{int}}. TIMESTAMP is represented as {{long}}. DECIMAL is represented as 
{{long}}.

> Unify function's operands type check logic in validation and behavior in 
> runtime
> --------------------------------------------------------------------------------
>
>                 Key: CALCITE-3562
>                 URL: https://issues.apache.org/jira/browse/CALCITE-3562
>             Project: Calcite
>          Issue Type: Task
>          Components: core
>            Reporter: Feng Zhu
>            Priority: Major
>
> 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*_.
> {code:java}
> @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");
>  }{code}
> We will get:
> {code:java}
> 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){code}
>  
> (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.
> {code:java}
>   @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");
>   }
> {code}
> We will get:
> {code:java}
> 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){code}
>  
> (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)*_
> {code:java}
> @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");
>  }{code}
> We will get:
> {code:java}
> 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){code}
>  
> *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.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

Reply via email to