Skip to content

Conversation

@martin-g
Copy link
Member

Which issue does this PR close?

Related-to: #8625

Rationale for this change

63fe99a introduced the new unit test.
I believe the index=3 is a typo. It should be =2.

What changes are included in this PR?

Fixed the array index.

Are these changes tested?

The fix is in a unit test.

Are there any user-facing changes?

No.

apache@63fe99a
introduced the new unit test.
I believe the index=3 is a typo. It should be =2.

Signed-off-by: Martin Tzvetanov Grigorov <mgrigorov@apache.org>
@github-actions github-actions bot added the parquet-variant parquet-variant* crates label Oct 27, 2025
@martin-g
Copy link
Member Author

CC @friendlymatthew

Copy link
Contributor

@friendlymatthew friendlymatthew 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 @martin-g

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 @martin-g and @friendlymatthew

I wonder why calling is_null(3)didn't panic 🤔 Maybe it is because the bitmap is within bounds ...

@alamb alamb merged commit 0745bb4 into apache:main Oct 28, 2025
17 checks passed
@martin-g martin-g deleted the fix-variant-array-item-index-in-test branch October 28, 2025 20:30
@martin-g
Copy link
Member Author

I wonder why calling is_null(3)didn't panic

It does not panic because it does not use the index at all. .nulls() returns None and thus it does not use the index. The assert() passes because of the negation.

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

Labels

parquet-variant parquet-variant* crates

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants