julianhyde commented on code in PR #3489:
URL: https://github.com/apache/calcite/pull/3489#discussion_r1375652661
##########
core/src/main/java/org/apache/calcite/adapter/enumerable/RexImpTable.java:
##########
@@ -664,6 +665,7 @@ Builder populate() {
defineMethod(SINH, BuiltInMethod.SINH.method, NullPolicy.STRICT);
defineMethod(TAN, BuiltInMethod.TAN.method, NullPolicy.STRICT);
defineMethod(TANH, BuiltInMethod.TANH.method, NullPolicy.STRICT);
+ defineMethod(HYPOT, BuiltInMethod.HYPOT.method, NullPolicy.ANY);
defineMethod(TRUNC, BuiltInMethod.STRUNCATE.method, NullPolicy.STRICT);
Review Comment:
Move HYPOT up so that it is after DEGREES and before IS_INF. The list is
approximately ordered, so let's keep it that way.
##########
core/src/main/java/org/apache/calcite/adapter/enumerable/RexImpTable.java:
##########
@@ -664,6 +665,7 @@ Builder populate() {
defineMethod(SINH, BuiltInMethod.SINH.method, NullPolicy.STRICT);
defineMethod(TAN, BuiltInMethod.TAN.method, NullPolicy.STRICT);
defineMethod(TANH, BuiltInMethod.TANH.method, NullPolicy.STRICT);
+ defineMethod(HYPOT, BuiltInMethod.HYPOT.method, NullPolicy.ANY);
Review Comment:
NullPolicy should be STRICT. ANY means 'returns null only if all arguments
are null'.
In my opinion, HYPOT(3, NULL) and HYPOT(NULL, 4) should return null, which
is consistent with STRICT.
Also test that the type is `DOUBLE` (not `DOUBLE NOT NULL`) when 1 argument
is nullable. (Maybe `checkNull` could do that checking?)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
