Skip to content

Fix append method on compressed datachunks #3387

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

rolnico
Copy link
Member

@rolnico rolnico commented Mar 27, 2025

Please check if the PR fulfills these requirements

  • The commit message follows our guidelines
  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)
  • A PR or issue has been opened in all impacted repositories (if any)

Does this PR already have an issue describing the problem?
No

What kind of change does this PR introduce?
Bug fix

What is the current behavior?
When a compressed datachunk is append to another one, if the last value of the first datachunk is equal to the first value of the second datachunk, the corresponding length should be added together.
However, in the current version, the last length of the first datachunk was added to the first length of the new datachunk, which was initialized with the first datachunk so the wrong length was added.

Tests compressedMergeTest() in StringDataChunkTest and in DoubleDataChunkTest used the same length value in the two datachunks so the bug wasn't apparent.

What is the new behavior (if this is a feature change)?
The last length of the first datachunk is now added to the first length of the second datachunk to create the length in the new datachunk.

Tests compressedMergeTest() in StringDataChunkTest and in DoubleDataChunkTest have been modified to test this.

Does this PR introduce a breaking change or deprecate an API?

  • Yes
  • No

If yes, please check if the following requirements are fulfilled

  • The Breaking Change or Deprecated label has been added
  • The migration steps are described in the following section

What changes might users need to make in their application due to this PR? (migration steps)

Other information:

@rolnico rolnico self-assigned this Mar 27, 2025
@alicecaron alicecaron added the bug label Mar 27, 2025
@alicecaron alicecaron moved this from TODO to In Progress in Release 03/2025 Mar 27, 2025
@marifunf marifunf self-requested a review March 27, 2025 14:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Approved
Development

Successfully merging this pull request may close these issues.

3 participants