fix: Prevent infinite float values in documents #3604
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #3604 +/- ##
===========================================
- Coverage 78.67% 78.62% -0.05%
===========================================
Files 405 409 +4
Lines 37063 37295 +232
===========================================
+ Hits 29157 29321 +164
- Misses 6175 6225 +50
- Partials 1731 1749 +18
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 27 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
| }, | ||
| }, | ||
| }, | ||
| ExpectedError: "error merging delta: operation results in a NaN, Inf, or -Inf value.", |
There was a problem hiding this comment.
thought: maybe it should be part of defradb configuration to allow different behavior for such cases.
Users might want to:
- return error
- saturate the value
- allow infinity
There was a problem hiding this comment.
This would be a major change of direction, I think. Let's discuss it in standup, I think.
|
|
||
| newValue := curValue + value | ||
|
|
||
| // Disallow increments that cause overflow, and throw error |
There was a problem hiding this comment.
question: I thought we agreed to store infinitely precise floats, merge them, and then return NaN when queried?
There was a problem hiding this comment.
I misunderstood. I thought the plan was to go ahead and merge this, and that this change would take place on a separate PR in the future. Clearly, I am mistaken. :)
There was a problem hiding this comment.
Also, I see Fred reacted to the message, but I hope the notifications are not interrupting his vacation.
3 participants
Relevant issue(s)
Resolves #2569
Resolves #2566
Description
It was possible to update a field that was a float PNCounter such that it became so large, or so largely negative, that it overflowed and became
Infor-Inf. This resulted in future Updates to silently fail, and future Get requests to fail with an error. Both behaviors are undesirable. The proposed solution in this fix is to explicitly check for an overflow when incrementing or decrementing a PNCounter.There existed a second issue, where PNCounters did not always reach consistency when overflowing. This was because of the way addition and subtraction operations are defined to work on infinite values, and how the order in which those operations was performed, actually mattered. However, the path to store infinite values is now blocked, which should eliminate this issue.
Tasks
How has this been tested?
The tests within
tests/integration/mutation/update/crdt/pncounter_test.gohave been updated. Specifically, the following two tests:TestPNCounterUpdate_FloatKindWithPositiveIncrementOverflow_PositiveInfTestPNCounterUpdate_FloatKindWithDecrementOverflow_NegativeInfFurther, the following two tests have been added:
TestPNCounterCreate_FloatKindWithNegativeInfValueTestPNCounterCreate_FloatKindWithPositiveInfValueSpecify the platform(s) on which this was tested: