Skip to content

Test: add alignment test for arrow-flight - #10428

Merged
alamb merged 3 commits into
apache:mainfrom
Rich-T-kid:rich-t-kid/arrow-flight-decode-alignment-test
Jul 28, 2026
Merged

Test: add alignment test for arrow-flight #10428
alamb merged 3 commits into
apache:mainfrom
Rich-T-kid:rich-t-kid/arrow-flight-decode-alignment-test

Conversation

@Rich-T-kid

Copy link
Copy Markdown
Contributor

Which issue does this PR close?

Rationale for this change

see #10418

What changes are included in this PR?

Are these changes tested?

the changes are test

Are there any user-facing changes?

no

@github-actions github-actions Bot added arrow Changes to the arrow crate arrow-flight Changes to the arrow-flight crate labels Jul 24, 2026
@Rich-T-kid Rich-T-kid changed the title add alighment test for arrow-flight Test: add alighment test for arrow-flight Jul 25, 2026
@Rich-T-kid Rich-T-kid changed the title Test: add alighment test for arrow-flight Test: add alignment test for arrow-flight Jul 25, 2026
Comment thread arrow-flight/tests/encode_decode.rs Outdated
}

#[test]
#[should_panic(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit: we can probably assert on the error instead of relying on should_panic here (since we explicitly call unwrap in the helper)?

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.

good point!

@alamb alamb left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This looks good other than the should_panic thing

Comment thread arrow-flight/tests/encode_decode.rs Outdated
@Rich-T-kid
Rich-T-kid force-pushed the rich-t-kid/arrow-flight-decode-alignment-test branch from c3f145b to baac7e4 Compare July 28, 2026 15:03

@Rich-T-kid Rich-T-kid left a comment

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.

should be good now

}

#[test]
#[should_panic(expected = "Memory pointer is not aligned with the specified scalar type")]

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.

The dense union panic originates inside arrow-buffer's ScalarBuffer code. Not from decode_misaligned's own unwrap()

@alamb
alamb merged commit 12091af into apache:main Jul 28, 2026
12 checks passed
@alamb

alamb commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Thanks @Rich-T-kid and @Jefffrey

@Jefffrey Jefffrey added the development-process Related to development process of arrow-rs label Jul 28, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

arrow Changes to the arrow crate arrow-flight Changes to the arrow-flight crate development-process Related to development process of arrow-rs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add test for mis-aligned buffers in arrow flight

3 participants