Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

GH-45744: [C++] Remove deprecated GetNextSegment #45745

Conversation

AlenkaF
Copy link
Member

@AlenkaF AlenkaF commented Mar 11, 2025

Rationale for this change

GetNextSegment has been deprecated in 18.0.0 and can now be removed.

What changes are included in this PR?

GetNextSegment and related code is removed from compute/row/grouper.cc/.h.

Are these changes tested?

The existing tests should pass.

Are there any user-facing changes?

GetNextSegment is removed in favour of GetSegment

@AlenkaF AlenkaF marked this pull request as draft March 11, 2025 13:45
@AlenkaF AlenkaF marked this pull request as ready for review March 11, 2025 14:30
@AlenkaF AlenkaF requested a review from zanmato1984 March 11, 2025 14:30
Copy link
Contributor

@zanmato1984 zanmato1984 left a comment

Choose a reason for hiding this comment

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

LGTM with only one nit.

@github-actions github-actions bot added awaiting committer review Awaiting committer review and removed awaiting review Awaiting review labels Mar 11, 2025
Co-authored-by: Rossi Sun <zanmato1984@gmail.com>
@zanmato1984
Copy link
Contributor

@github-actions crossbow submit -g cpp

Copy link

<class 'requests.exceptions.ConnectionError'>: A connection-level exception occurred: ('Connection aborted.', RemoteDisconnected('Remote end closed connection without response'))
The Archery job run can be found at: https://github.com/apache/arrow/actions/runs/13805951286

@zanmato1984
Copy link
Contributor

@github-actions crossbow submit -g cpp

Copy link

Revision: 23f880b

Submitted crossbow builds: ursacomputing/crossbow @ actions-23fca30928

Task Status
example-cpp-minimal-build-static GitHub Actions
example-cpp-minimal-build-static-system-dependency GitHub Actions
example-cpp-tutorial GitHub Actions
test-alpine-linux-cpp GitHub Actions
test-build-cpp-fuzz GitHub Actions
test-conda-cpp GitHub Actions
test-conda-cpp-meson GitHub Actions
test-conda-cpp-valgrind GitHub Actions
test-cuda-cpp-ubuntu-22.04-cuda-11.7.1 GitHub Actions
test-debian-12-cpp-amd64 GitHub Actions
test-debian-12-cpp-i386 GitHub Actions
test-fedora-39-cpp GitHub Actions
test-ubuntu-22.04-cpp GitHub Actions
test-ubuntu-22.04-cpp-20 GitHub Actions
test-ubuntu-22.04-cpp-bundled GitHub Actions
test-ubuntu-22.04-cpp-emscripten GitHub Actions
test-ubuntu-22.04-cpp-no-threading GitHub Actions
test-ubuntu-24.04-cpp GitHub Actions
test-ubuntu-24.04-cpp-bundled-offline GitHub Actions
test-ubuntu-24.04-cpp-gcc-13-bundled GitHub Actions
test-ubuntu-24.04-cpp-gcc-14 GitHub Actions
test-ubuntu-24.04-cpp-minimal-with-formats GitHub Actions
test-ubuntu-24.04-cpp-thread-sanitizer GitHub Actions

@zanmato1984
Copy link
Contributor

CI is good. I'm merging. Thank you @AlenkaF !

@zanmato1984 zanmato1984 merged commit 4291dad into apache:main Mar 12, 2025
36 checks passed
@zanmato1984 zanmato1984 removed the awaiting committer review Awaiting committer review label Mar 12, 2025
@AlenkaF AlenkaF deleted the gh-45744-remove-deprecated-functions-GetNextSegment branch March 12, 2025 10:03
Copy link

After merging your PR, Conbench analyzed the 4 benchmarking runs that have been run so far on merge-commit 4291dad.

There were no benchmark performance regressions. 🎉

The full Conbench report has more details. It also includes information about 7 possible false positives for unstable benchmarks that are known to sometimes produce them.

# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants