[GitHub] [calcite] olivrlee commented on a diff in pull request #3391: [CALCITE-5933] Add SAFE_DIVIDE function (enabled in BigQuery library)
olivrlee commented on code in PR #3391:
URL: https://github.com/apache/calcite/pull/3391#discussion_r1316551091
##
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##
@@ -7538,8 +7610,8 @@ private static void checkIf(SqlOperatorFixture f) {
@Test void testSafeNegateFunc() {
final SqlOperatorFixture f0 =
fixture().setFor(SqlLibraryOperators.SAFE_NEGATE);
f0.checkFails("^safe_negate(2)^",
- "No match found for function signature "
-+ "SAFE_NEGATE\\(\\)", false);
+"No match found for function signature "
Review Comment:
nvm discussed offline, these are correct
--
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: commits-unsubscr...@calcite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [calcite] olivrlee commented on a diff in pull request #3391: [CALCITE-5933] Add SAFE_DIVIDE function (enabled in BigQuery library)
olivrlee commented on code in PR #3391:
URL: https://github.com/apache/calcite/pull/3391#discussion_r1316517662
##
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##
@@ -7538,8 +7610,8 @@ private static void checkIf(SqlOperatorFixture f) {
@Test void testSafeNegateFunc() {
final SqlOperatorFixture f0 =
fixture().setFor(SqlLibraryOperators.SAFE_NEGATE);
f0.checkFails("^safe_negate(2)^",
- "No match found for function signature "
-+ "SAFE_NEGATE\\(\\)", false);
+"No match found for function signature "
Review Comment:
unfortunately indents changed here again lol
--
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: commits-unsubscr...@calcite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [calcite] olivrlee commented on a diff in pull request #3391: [CALCITE-5933] Add SAFE_DIVIDE function (enabled in BigQuery library)
olivrlee commented on code in PR #3391:
URL: https://github.com/apache/calcite/pull/3391#discussion_r1312548850
##
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##
@@ -7385,8 +7385,8 @@ private static void checkIf(SqlOperatorFixture f) {
@Test void testSafeAddFunc() {
final SqlOperatorFixture f0 =
fixture().setFor(SqlLibraryOperators.SAFE_ADD);
f0.checkFails("^safe_add(2, 3)^",
-"No match found for function signature "
-+ "SAFE_ADD\\(, \\)", false);
+ "No match found for function signature "
++ "SAFE_ADD\\(, \\)", false);
Review Comment:
is this indent still here or is it outdated?
--
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: commits-unsubscr...@calcite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [calcite] olivrlee commented on a diff in pull request #3391: [CALCITE-5933] Add SAFE_DIVIDE function (enabled in BigQuery library)
olivrlee commented on code in PR #3391:
URL: https://github.com/apache/calcite/pull/3391#discussion_r1304822219
##
core/src/main/java/org/apache/calcite/sql/fun/SqlLibraryOperators.java:
##
@@ -1700,6 +1700,15 @@ private static RelDataType
deriveTypeMapFromEntries(SqlOperatorBinding opBinding
OperandTypes.NUMERIC_NUMERIC,
SqlFunctionCategory.NUMERIC);
+ /** The "SAFE_DIVIDE(numeric1, numeric2)" function; equivalent to the {@code
/} operator but
+ * returns null if an error occurs, such as division by zero. */
Review Comment:
nit: add to comment a part about overflow, since other areas seem to mention
it too
--
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: commits-unsubscr...@calcite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [calcite] olivrlee commented on a diff in pull request #3391: [CALCITE-5933] Add SAFE_DIVIDE function (enabled in BigQuery library)
olivrlee commented on code in PR #3391:
URL: https://github.com/apache/calcite/pull/3391#discussion_r1304821520
##
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##
@@ -7332,81 +7332,153 @@ private static void checkIf(SqlOperatorFixture f) {
@Test void testSafeAddFunc() {
final SqlOperatorFixture f0 =
fixture().setFor(SqlLibraryOperators.SAFE_ADD);
f0.checkFails("^safe_add(2, 3)^",
-"No match found for function signature "
Review Comment:
Are these new indents supposed to be here?
--
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: commits-unsubscr...@calcite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [calcite] olivrlee commented on a diff in pull request #3391: [CALCITE-5933] Add SAFE_DIVIDE function (enabled in BigQuery library)
olivrlee commented on code in PR #3391:
URL: https://github.com/apache/calcite/pull/3391#discussion_r1304820120
##
core/src/main/java/org/apache/calcite/sql/type/ReturnTypes.java:
##
@@ -826,6 +826,28 @@ public static SqlCall stripSeparator(SqlCall call) {
public static final SqlReturnTypeInference DECIMAL_QUOTIENT_NULLABLE =
DECIMAL_QUOTIENT.andThen(SqlTypeTransforms.TO_NULLABLE);
+ /**
+ * Type-inference stratey whereby the result type of a call is
+ * {@link #DOUBLE} if both operands are integer types.
+ */
+ public static final SqlReturnTypeInference DOUBLE_IF_INTEGER = opBinding -> {
Review Comment:
Maybe plural `DOUBLE_IF_INTEGERS` or `DOUBLE_IF_BOTH_INTEGERS`
--
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: commits-unsubscr...@calcite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
[GitHub] [calcite] olivrlee commented on a diff in pull request #3391: [CALCITE-5933] Add SAFE_DIVIDE function (enabled in BigQuery library)
olivrlee commented on code in PR #3391:
URL: https://github.com/apache/calcite/pull/3391#discussion_r1304819311
##
core/src/main/java/org/apache/calcite/sql/type/ReturnTypes.java:
##
@@ -826,6 +826,28 @@ public static SqlCall stripSeparator(SqlCall call) {
public static final SqlReturnTypeInference DECIMAL_QUOTIENT_NULLABLE =
DECIMAL_QUOTIENT.andThen(SqlTypeTransforms.TO_NULLABLE);
+ /**
+ * Type-inference stratey whereby the result type of a call is
Review Comment:
`strategy`
--
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: commits-unsubscr...@calcite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
