fix: relax validation for missing validity buffer when null_count is -1 - #942
Merged
paleolimbot merged 1 commit intoSep 15, 2026
Merged
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #942 +/- ##
=======================================
Coverage 78.19% 78.19%
=======================================
Files 106 106
Lines 16843 16843
Branches 1985 1985
=======================================
Hits 13170 13170
Misses 2439 2439
Partials 1234 1234 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
paleolimbot
reviewed
Sep 15, 2026
paleolimbot
left a comment
paleolimbot
left a comment
Member
There was a problem hiding this comment.
Thank you!
One note on the test here...I think you're testing two things but we only need to update the null count to ensure your change is covered.
Comment on lines
+2484
to
+2505
| // Same for a string array (["a", "bb", "ccc"]) | ||
| int32_t offsets[] = {0, 1, 3, 6}; | ||
| const void* string_buffers[] = {nullptr, offsets, "abbccc"}; | ||
| array->null_count = -1; | ||
| array->n_buffers = 3; | ||
| array->buffers = string_buffers; | ||
|
|
||
| nanoarrow::UniqueArrayView string_view; | ||
| ArrowArrayViewInitFromType(string_view.get(), NANOARROW_TYPE_STRING); | ||
| ASSERT_EQ(ArrowArrayViewSetArray(string_view.get(), array.get(), &error), NANOARROW_OK) | ||
| << error.message; | ||
| EXPECT_EQ( | ||
| ArrowArrayViewValidate(string_view.get(), NANOARROW_VALIDATION_LEVEL_FULL, &error), | ||
| NANOARROW_OK) | ||
| << error.message; | ||
| EXPECT_EQ(ArrowArrayViewGetBufferView(string_view.get(), 0).size_bytes, 0); | ||
| EXPECT_EQ(ArrowArrayViewComputeNullCount(string_view.get()), 0); | ||
| EXPECT_EQ(ArrowArrayViewGetStringUnsafe(string_view.get(), 0), "bb"_asv); | ||
| EXPECT_EQ(ArrowArrayViewGetStringUnsafe(string_view.get(), 1), "ccc"_asv); | ||
|
|
||
| array->null_count = 1; | ||
| EXPECT_EQ(ArrowArrayViewSetArray(string_view.get(), array.get(), &error), EINVAL); |
Member
There was a problem hiding this comment.
Can you to use the exact same array as above and just modify the null count? I don't think we need to use a different type here to cover the changed lines.
Contributor
Author
There was a problem hiding this comment.
I've removed the string-array case, and the test now uses the same int32 array and only changes the null count.
Validation currently errors if the validity buffer is missing and null_count != 0. Arrow C++ only errors if null_count > 0, and arrays like this show up in the wild. Relax the check to match. Fixes apache#855
and2049
force-pushed
the
fix-855-null-count-unknown-no-validity
branch
from
September 15, 2026 17:43
c0eb1d9 to
275705f
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Allow a missing validity buffer when the null count is unknown, matching
Arrow C++’s import behavior. Arrays reporting a positive null count still
require a validity buffer.
Add regression coverage for unknown null counts with a missing validity
buffer, including sliced arrays and positive-null-count rejection.
Fixes #855