Skip to content

BytesInput.copy(BytesInput) should return a copy not a reference - #3729

Open
dossett wants to merge 1 commit into
apache:masterfrom
dossett:test-dictionary-page-copy-isolation
Open

BytesInput.copy(BytesInput) should return a copy not a reference#3729
dossett wants to merge 1 commit into
apache:masterfrom
dossett:test-dictionary-page-copy-isolation

Conversation

@dossett

@dossett dossett commented Aug 17, 2026

Copy link
Copy Markdown
Contributor

#3565 introduced several optimizations, but I think it also introduced an issue that PR seeks to address. Basically BytesInput.copy(BytesInput) has a public contract to return a copy, but a change to BytesInput.toByteArray() led to returning a reference in some cases. In addition to breaking the public contract, this can lead to data corruption.

See also #3717

* Zero-copy override: returns the backing array directly when fully used,
* skipping the base-class BAOS allocation + copy on every decompressor call.
* Returning the mutable array is safe — the base class already exposes a
* mutable {@code BAOS.getBuf()}.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I think Returning the mutable array is safe is the key. The array was mutable before, but it was a mutable copy of the original array.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant