korlov42 commented on code in PR #2690:
URL: https://github.com/apache/ignite-3/pull/2690#discussion_r1363959297
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/exp/RexImpTable.java:
##########
@@ -632,7 +632,7 @@ Builder populate() {
defineMethod(DEGREES, "degrees", NullPolicy.STRICT);
defineMethod(POW, "power", NullPolicy.STRICT);
defineMethod(RADIANS, "radians", NullPolicy.STRICT);
- defineMethod(ROUND, "sround", NullPolicy.STRICT);
+ defineIgniteMethod(ROUND, "sround", NullPolicy.STRICT);
Review Comment:
all ignite's overrides better to place to `populateIgnite()`
##########
modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/exec/exp/IgniteSqlFunctionsTest.java:
##########
@@ -242,4 +245,57 @@ public void testConvertDecimal(String input, int
precision, int scale, String re
assertThrowsSqlException(Sql.RUNTIME_ERR, "Numeric field
overflow", convert::get);
}
}
+
+ /** Tests for ROUND(x) function. */
+ @Test
+ public void testRound() {
Review Comment:
let's add all possible numeric types to the test cases. The same for another
variant of this function
##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/exec/exp/RexImpTable.java:
##########
@@ -4359,6 +4364,26 @@ private static class DefaultImplementor extends
AbstractRexCallImplementor {
}
}
+ private static class IgniteMethodNameImplementor extends
AbstractRexCallImplementor {
Review Comment:
why do we need new implementor?
##########
modules/runner/src/integrationTest/sql/types/decimal/test_decimal_ops.test:
##########
Review Comment:
BTW, we have `sql/function/numeric/test_round.test`. Let's add test cases to
this file to cover all possible types
##########
modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/exec/exp/IgniteSqlFunctionsTest.java:
##########
@@ -242,4 +245,57 @@ public void testConvertDecimal(String input, int
precision, int scale, String re
assertThrowsSqlException(Sql.RUNTIME_ERR, "Numeric field
overflow", convert::get);
}
}
+
+ /** Tests for ROUND(x) function. */
+ @Test
+ public void testRound() {
+ assertEquals(new BigDecimal("1"), IgniteSqlFunctions.sround(new
BigDecimal("1.000")));
+ assertEquals(1, IgniteSqlFunctions.sround(1), "int");
+ assertEquals(1L, IgniteSqlFunctions.sround(1L), "long");
+ assertEquals(1.0d, IgniteSqlFunctions.sround(1.123d), "double");
+ }
+
+ /** Tests for ROUND(x, s) function, where x is a BigDecimal value. */
+ @ParameterizedTest
+ @CsvSource({
+ "1.123, 0, 1.000",
Review Comment:
can we have similar set of tests for every type? LIke, tests with negative
`s`
--
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]
