-
Notifications
You must be signed in to change notification settings - Fork 421
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
PARQUET-2215: [Format] Document overflow handling in DELTA_BINARY_PACKED #187
Conversation
cc @rok |
Encodings.md
Outdated
Subtractions in steps 1) and 2) may incur signed arithmetic overflow. Overflow | ||
should be allowed and handled as wrapping around in 2's complement notation. | ||
This may require explicit care in some programming languages (for example by | ||
doing all arithmetic in the unsigned domain). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps we should note that this produces the correct result on decoding?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't that implied?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is, but maybe if it were less subtle it would save readers having to reason through it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, I elaborated a bit. Does it help?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It does! :)
@ksuarez1423 @wjones127 Would you like to take a look at the wording? |
Also word-wrapping for better source readability and editability.