Skip to content

ARROW-16238: [C++] Fix nullptr dereference when pre-buffering IPC reads - #12937

Closed
lidavidm wants to merge 2 commits into
apache:masterfrom
lidavidm:arrow-16238
Closed

ARROW-16238: [C++] Fix nullptr dereference when pre-buffering IPC reads#12937
lidavidm wants to merge 2 commits into
apache:masterfrom
lidavidm:arrow-16238

Conversation

@lidavidm

Copy link
Copy Markdown
Member

This fixes a bug when coalescing is enabled. Also, it changes the conditional so that we skip coalescing for zero-copy files instead of enabling it. Finally, it slightly refactors the tests to ensure this case is hit.

@lidavidmlidavidm changed the title ARROW-14577: [C++] Fix nullptr dereference when pre-buffering IPC readsARROW-16238: [C++] Fix nullptr dereference when pre-buffering IPC readsApr 20, 2022
@github-actions

Copy link
Copy Markdown

@github-actions

Copy link
Copy Markdown

@westonpacewestonpace left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for catching this.

Comment threadcpp/src/arrow/ipc/read_write_test.cc Outdated

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Minor nit: Maybe TestFileFormatGeneratorCoalesced?

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

Done.

@ursabot

Copy link
Copy Markdown

Benchmark runs are scheduled for baseline = 40c1226 and contender = fd5f28e. fd5f28e is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
Conbench compare runs links:
[Finished ⬇️0.0% ⬆️0.0%] ec2-t3-xlarge-us-east-2
[Failed] test-mac-arm
[Finished ⬇️0.36% ⬆️0.0%] ursa-i9-9960x
[Finished ⬇️0.5% ⬆️0.04%] ursa-thinkcentre-m75q
Buildkite builds:
[Finished] <https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ec2-t3-xlarge-us-east-2/builds/609|fd5f28ec ec2-t3-xlarge-us-east-2>
[Failed] <https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-test-mac-arm/builds/597|fd5f28ec test-mac-arm>
[Finished] <https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ursa-i9-9960x/builds/596|fd5f28ec ursa-i9-9960x>
[Finished] <https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ursa-thinkcentre-m75q/builds/607|fd5f28ec ursa-thinkcentre-m75q>
[Finished] <https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ec2-t3-xlarge-us-east-2/builds/608|40c1226f ec2-t3-xlarge-us-east-2>
[Failed] <https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-test-mac-arm/builds/596|40c1226f test-mac-arm>
[Finished] <https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ursa-i9-9960x/builds/595|40c1226f ursa-i9-9960x>
[Finished] <https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ursa-thinkcentre-m75q/builds/606|40c1226f ursa-thinkcentre-m75q>
Supported benchmarks:
ec2-t3-xlarge-us-east-2: Supported benchmark langs: Python, R. Runs only benchmarks with cloud = True
test-mac-arm: Supported benchmark langs: C++, Python, R
ursa-i9-9960x: Supported benchmark langs: Python, R, JavaScript
ursa-thinkcentre-m75q: Supported benchmark langs: C++, Java

Sign up for freeto 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.

3 participants

@lidavidm@ursabot@westonpace