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 side effect in decimal addition operator #8973

Merged
merged 1 commit into from
Aug 26, 2021

Conversation

martint
Copy link
Member

@martint martint commented Aug 25, 2021

addLongShortLong performs an in-place addition that mutates the value of
the Slice associated with left. This is based on the assumption that
rescale always returns a new Slice, which would avoid modifications
to the input arguments to the operator. However, when the rescale factor
is 0, rescale just returns its input argument.

Fixes #8968

addLongShortLong performs an in-place addition that mutates the value of
the Slice associated with "left". This is based on the assumption that
"rescale" always returns a new Slice, which would avoid modifications
to the input arguments to the operator. However, when the rescale factor
is 0, rescale just returns its input argument.
@cla-bot cla-bot bot added the cla-signed label Aug 25, 2021
Slice value = unscaledDecimal(2);

addShortLongLong(1, value, 0, false);
assertThat(value.getBytes())
Copy link
Member Author

Choose a reason for hiding this comment

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

getBytes() so that the contents of the array are rendered properly in case of an error and the difference is clear.

@sopel39
Copy link
Member

sopel39 commented Aug 25, 2021

@skrzypo987 do you recall any similar places?

@skrzypo987
Copy link
Member

I've looked through my commits regarding 128 ints and it looks like this is the only change like that

@sopel39 sopel39 merged commit 040d857 into trinodb:master Aug 26, 2021
@sopel39 sopel39 mentioned this pull request Aug 26, 2021
10 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

Incorrect result for addition between long decimals
4 participants