zabetak commented on code in PR #3326:
URL: https://github.com/apache/calcite/pull/3326#discussion_r1328640825
##########
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##########
@@ -6422,6 +6422,10 @@ private static void checkIf(SqlOperatorFixture f) {
f.checkScalarApprox("atanh(0.76159416)", "DOUBLE NOT NULL",
isWithin(1d, 0.0001d));
f.checkScalarApprox("atanh(cast(-0.1 as decimal))", "DOUBLE NOT NULL",
+ isWithin(-0.0d, 0.0001d));
Review Comment:
The `SqlOperatorTest` class contains many tests casting various types to
decimals. These tests are all disabled since `SqlOperatorTest#DECIMAL` is
`false`.
Since in this PR you are basically fixing CAST to DECIMAL type, I was
thinking that it makes sense to enable those that are affected by this fix. If
the existing tests are sufficient to test the changes in this PR then it is not
really necessary to add new ones (using `atanh` etc.).
Basically, another way to put my question is the following.
* Can we set SqlOperatorTest#DECIMAL to true ?
* Can we remove some `if (DECIMAL)`, `if (!DECIMAL)` statements?
* Does this PR fix any of the above?
--
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]
