Fix replication of large decimals #4521
Conversation
cf89838 to
d80deb1
Compare
d80deb1 to
ffcbfb8
Compare
| constexpr int bits_per_byte = 7; | ||
| using uchar = unsigned char; | ||
| char* ptr = buffer; | ||
| while (value_0 >> (bits_per_byte - 1)) { |
There was a problem hiding this comment.
Seems wrong. 1) why the "bits_per_byte-1", 2) isn't there a risk for too early out since value_1 is not part of the condition.
There was a problem hiding this comment.
- "bits_per_byte-1" because there should be room for the sign bit in the last byte (always 0 in this case).
- An error - well spotted.
There was a problem hiding this comment.
perhaps add a comment :-)
There was a problem hiding this comment.
Has tests been updated to catch the error Finn spotted?
There was a problem hiding this comment.
@jedelbo Yes, but I was more wondering if you added tests that would have found this error instead of Finn :-)
|
@finnschiermer it would be a change in the protocol if we changed the way we serialize the values. |
ffcbfb8 to
e0d0e5e
Compare
|
@jedelbo what was the root-cause for having this issue in the first place? Not creating an issue to fix the "FIXME"? |
|
@bmunkholm yes, the root cause was me forgetting the FIXME. |
What, How & Why?
Fixes #4519
☑️ ToDos