Skip to content

ARROW-14197: [C++][Compute] Fixing wrong buffer size in GrouperFastImpl #11335

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
Closed
michalursa wants to merge 1 commit into from
Closed

ARROW-14197: [C++][Compute] Fixing wrong buffer size in GrouperFastImpl #11335

michalursa wants to merge 1 commit into from

Conversation

@michalursa
Copy link
Contributor

@michalursa michalursa commented Oct 6, 2021

Fixing wrong buffer size in GrouperFastImpl (introduced in recent refactoring of SwissTable that introduced "find" method).
Also adding markers at the start and end of each temporary vector to check buffer overruns in the future.

@github-actions
Copy link

github-actions bot commented Oct 6, 2021

https://issues.apache.org/jira/browse/ARROW-14197

@github-actions
Copy link

github-actions bot commented Oct 6, 2021

⚠️ Ticket has not been started in JIRA, please click 'Start Progress'.

@michalursa michalursa force-pushed the ARROW-14197-tpch-hang branch from 0190eb9 to f429b93 Compare October 6, 2021 05:28
@pitrou
Copy link
Member

pitrou commented Oct 6, 2021

@github-actions crossbow submit -g cpp

@github-actions
Copy link

github-actions bot commented Oct 6, 2021

Revision: f429b93eaf9958fc74af93ebc787254a3fdb16d2

Submitted crossbow builds: ursacomputing/crossbow @ actions-895

Task Status
test-conda-cpp Github Actions
test-conda-cpp-valgrind Azure
test-debian-11-cpp Github Actions
test-fedora-33-cpp Github Actions
test-ubuntu-18.04-cpp Github Actions
test-ubuntu-18.04-cpp-release Github Actions
test-ubuntu-18.04-cpp-static Github Actions
test-ubuntu-20.04-cpp Github Actions
test-ubuntu-20.04-cpp-14 Github Actions
test-ubuntu-20.04-cpp-17 Github Actions
test-ubuntu-20.04-cpp-thread-sanitizer Github Actions

bkietz
bkietz requested changes Oct 7, 2021
View reviewed changes
Copy link
Member

@bkietz bkietz left a comment

Choose a reason for hiding this comment

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

This looks good to me, modulo a naming nit and a comment

@michalursa michalursa force-pushed the ARROW-14197-tpch-hang branch from f429b93 to 1702c85 Compare October 7, 2021 20:51
@michalursa michalursa requested a review from bkietz October 7, 2021 23:06
@ursabot
Copy link

ursabot commented Oct 12, 2021 •
edited
Loading

Benchmark runs are scheduled for baseline = 4f0ac50 and contender = 5f80ddc. 5f80ddc is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
Conbench compare runs links:
[Failed ⬇️0.0% ⬆️0.0%] ec2-t3-xlarge-us-east-2
[Finished ⬇️1.36% ⬆️0.0%] ursa-i9-9960x
[Failed ⬇️2.78% ⬆️0.0%] ursa-thinkcentre-m75q
Supported benchmarks:
ursa-i9-9960x: langs = Python, R, JavaScript
ursa-thinkcentre-m75q: langs = C++, Java
ec2-t3-xlarge-us-east-2: cloud = True

@asfimport asfimport mentioned this pull request Oct 13, 2021
Closed
@felipecrv felipecrv mentioned this pull request May 16, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@bkietz bkietz Awaiting requested review from bkietz

1 more reviewer

@felipeblazing felipeblazing felipeblazing approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

Component: C++

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@michalursa @pitrou @ursabot @bkietz @felipeblazing