Skip to content
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

Fix signed decimal e-notation parsing #6729

Merged
merged 1 commit into from
Nov 14, 2024

Conversation

gruuya
Copy link
Contributor

@gruuya gruuya commented Nov 13, 2024

Which issue does this PR close?

Closes #6728.

Rationale for this change

When parsing signed decimal numbers the leading sign is stripped from the raw value bytes, and the e index is passed relative to the stripped slice.

However, the full value is passed into parse_e_notation, leading to a off-by-one error of the index for the signed numbers.

What changes are included in this PR?

Do not strip the sign when parsing, and make the e index relative to the full value, thus aligning parse_e_notation and parse_decimal.

Are there any user-facing changes?

None

@github-actions github-actions bot added the arrow Changes to the arrow crate label Nov 13, 2024
if truthy_is_scalar && truthy.len() != 1 {
if falsy_is_scalar && falsy.len() != 1 {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is just a drive-by validation fix.

Copy link
Member

@waynexia waynexia left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a clear fix, LGTM 👍

@waynexia waynexia merged commit 1d580ec into apache:master Nov 14, 2024
26 checks passed
@gruuya gruuya deleted the negative-decimal-e-fix branch November 14, 2024 07:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
arrow Changes to the arrow crate
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Signed decimal e-notation parsing bug
2 participants