Skip to content

[branch-51] Backport Fix for Creating Null FixedSizeBinary Arrays to DF 51Branch #19017

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.

Sign up for GitHub

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

Jump to bottom
Merged
alamb merged 3 commits into from
Dec 2, 2025
Merged

[branch-51] Backport Fix for Creating Null FixedSizeBinary Arrays to DF 51Branch #19017

alamb merged 3 commits into from
Dec 2, 2025

Conversation

@tobixdev
Copy link
Contributor

@tobixdev tobixdev commented Dec 1, 2025 •
edited
Loading

Which issue does this PR close?

Backports #18903 to DF 51 branch

Rationale for this change

Fix the bug in a future 51.1.0 release (#18843)

What changes are included in this PR?

Same as in #18903

Are these changes tested?

Yes.

Are there any user-facing changes?

Yes, fix the original bug and create a non-zero sized values buffer in the case that exhibits the bug.

@github-actions github-actions bot added the common Related to common crate label Dec 1, 2025
@tobixdev tobixdev changed the title [branch-51] Back ports Fix for Creating Null FixedSizeBinary Arrays to DF 51Branch [branch-51] Backport Fix for Creating Null FixedSizeBinary Arrays to DF 51Branch Dec 1, 2025
@tobixdev
Copy link
Contributor Author

tobixdev commented Dec 1, 2025

Apparently, the test failed with System.IO.IOException: No space left on device. Should we restart the CI?

@alamb
Copy link
Contributor

alamb commented Dec 1, 2025

Apparently, the test failed with System.IO.IOException: No space left on device. Should we restart the CI?

Done

alamb
alamb approved these changes Dec 1, 2025
View reviewed changes
Copy link
Contributor

@alamb alamb left a comment

Choose a reason for hiding this comment

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

Thank you @tobixdev

@tobixdev
Copy link
Contributor Author

tobixdev commented Dec 1, 2025

Thanks @alamb ! Any idea what I could do to make the space error go away? 🤔

@alamb
Copy link
Contributor

alamb commented Dec 1, 2025

Thanks @alamb ! Any idea what I could do to make the space error go away? 🤔

There is code to clean up pre-installed nonsense on main:

datafusion/.github/workflows/rust.yml

Lines 277 to 286 in 6425e40

echo "Disk space before cleanup:"
df -h
# remove tool cache: about 8.5GB (github has host /opt/hostedtoolcache mounted as /__t)
rm -rf /__t/* || true
# remove Haskell runtime: about 6.3GB (host /usr/local/.ghcup)
rm -rf /host/usr/local/.ghcup || true
# remove Android library: about 7.8GB (host /usr/local/lib/android)
rm -rf /host/usr/local/lib/android || true
echo "Disk space after cleanup:"
df -h

That got added in #18709 by @Jefffrey

@tobixdev would you mind making a backport to branch-51 with that PR ? I can do it as well, but I can't approve my own PRs so if you make one I can approve/merge it.

@tobixdev
Copy link
Contributor Author

tobixdev commented Dec 1, 2025

Thanks for the info! I'll do it tomorrow. 👍

@alamb alamb mentioned this pull request Dec 1, 2025
Merged
@alamb
Copy link
Contributor

alamb commented Dec 1, 2025

That got added in #18709 by @Jefffrey

No worries -- I made it this afternoon (my time) in case another committer has time to review it over the night

xudong963 pushed a commit that referenced this pull request Dec 2, 2025
@alamb @Jefffrey
[branch-51] CI: try free up space in Rust / cargo test (amd64) acti…
388ed32 
…on (#18709) (#19037)

## Which issue does this PR close?

- part of #18843 
- Backports #18709 from
@Jefffrey

## Rationale for this change

@tobixdev is hitting a CI failure due to out of space on a different PR:
-
#19017 (comment)


@Jefffrey  fixed this on main with
-  #18709 

So let's backport that change to branch-51
## What changes are included in this PR?

- Backport #18709 to branch-51

## Are these changes tested?
by ci
<!--
We typically require tests for all PRs in order to:
1. Prevent the code from being accidentally broken by subsequent changes
2. Serve as another way to document the expected behavior of the code

If tests are not included in your PR, please explain why (for example,
are they covered by existing tests)?
-->

## Are there any user-facing changes?

no

Co-authored-by: Jeffrey Vo <jeffrey.vo.australia@gmail.com>
@tobixdev
Copy link
Contributor Author

tobixdev commented Dec 2, 2025

Thanks @alamb ! I've updated the branch. The build should pass now.

@alamb alamb merged commit 37c00ab into apache:branch-51 Dec 2, 2025
28 checks passed
@alamb
Copy link
Contributor

alamb commented Dec 2, 2025

Thanks @tobixdev !

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

Reviewers

@alamb alamb alamb approved these changes

Assignees

No one assigned

Labels

common Related to common crate

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@tobixdev @alamb