Skip to content

GH-39309: [Go][Parquet] handle nil bitWriter for DeltaBinaryPacked#39347

Merged
zeroshade merged 1 commit intoapache:mainfrom
zeroshade:empty-delta-binary-packed
Jan 8, 2024
Merged

GH-39309: [Go][Parquet] handle nil bitWriter for DeltaBinaryPacked#39347
zeroshade merged 1 commit intoapache:mainfrom
zeroshade:empty-delta-binary-packed

Conversation

@zeroshade
Copy link
Copy Markdown
Member

@zeroshade zeroshade commented Dec 21, 2023

Rationale for this change

If using the DeltaBinaryPacked encoding, we end up with a nil pointer dereference if we end up with an empty column.

What changes are included in this PR?

Add a nil check in EstimatedDataEncodedSize for the base deltaBitPackEncoder. This should only ever occur if we have an empty column with this encoding when closing a row group.

Are these changes tested?

Yes a unit test was added to verify the fix.

@github-actions
Copy link
Copy Markdown

⚠️ GitHub issue #39309 has been automatically assigned in GitHub to PR creator.

@mapleFU
Copy link
Copy Markdown
Member

mapleFU commented Jan 3, 2024

Do we need to merge this @zeroshade ?

@zeroshade zeroshade merged commit 0aadd5a into apache:main Jan 8, 2024
@zeroshade zeroshade removed the awaiting committer review Awaiting committer review label Jan 8, 2024
@conbench-apache-arrow
Copy link
Copy Markdown

After merging your PR, Conbench analyzed the 4 benchmarking runs that have been run so far on merge-commit 0aadd5a.

There were no benchmark performance regressions. 🎉

The full Conbench report has more details.

@zeroshade zeroshade deleted the empty-delta-binary-packed branch January 9, 2024 22:15
dgreiss pushed a commit to dgreiss/arrow that referenced this pull request Feb 19, 2024
…ked (apache#39347)

### Rationale for this change
If using the DeltaBinaryPacked encoding, we end up with a nil pointer dereference if we end up with an empty column.

### What changes are included in this PR?
Add a nil check in `EstimatedDataEncodedSize` for the base `deltaBitPackEncoder`. This should only ever occur if we have an empty column with this encoding when closing a row group.

### Are these changes tested?
Yes a unit test was added to verify the fix.

* Closes: apache#39309

Authored-by: Matt Topol <zotthewizard@gmail.com>
Signed-off-by: Matt Topol <zotthewizard@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Go][Parquet] Panic when writing a ListOf(DeltaBinaryPacked field) with no data

3 participants