Widen returned types for UpdateBy floating point operations.
#5371
Conversation
lbooker42
added
bug
engine/table/src/test/java/io/deephaven/engine/table/impl/updateby/TestCumSum.java
Show resolved
Hide resolved
| @@ -81,7 +84,7 @@ public void pop(int count) { | |||
| @Override | |||
| public void writeToOutputChunk(int outIdx) { | |||
| if (aggSum.size() == nullCount) { | |||
| outputValues.set(outIdx, NULL_FLOAT); | |||
| outputValues.set(outIdx, NULL_DOUBLE); | |||
There was a problem hiding this comment.
This doesn't match sum(DoubleVector) or sum(FloatVector) in Numeric. Which is right, @chipkent ?
There was a problem hiding this comment.
I think you are looking at something old. The latest Numeric.sum does return NULL_DOUBLE.
There was a problem hiding this comment.
I may have commented on a differnt case. Looking.
There was a problem hiding this comment.
If they are all null, it would make sense to return NULL_DOUBLE. I think Numeric.ftl needs to be changed.
There was a problem hiding this comment.
Do you want me to change it in this PR?
There was a problem hiding this comment.
OK. In that case, I think we can merge Larry's PR, and have follow-up work for the numerics.
engine/table/src/test/java/io/deephaven/engine/table/impl/updateby/TestCumSum.java
Show resolved
Hide resolved
|
Labels indicate documentation is required. Issues for documentation have been opened: Community: deephaven/deephaven-docs-community#208 |
The following operations returned
Floattypes when providedFloatinput and are changed by this PR to returnDouble:Companion to #5368 and will close #4023
Also close #5459 (test failures issue)