Skip to content

perf: DH-22420: Skip Unnecessary Single Value Columns when Sorting#7957

Merged
cpwright merged 4 commits intodeephaven:mainfrom
cpwright:coverage/cpw/skip-unnecessary-sorts
Apr 29, 2026
Merged

perf: DH-22420: Skip Unnecessary Single Value Columns when Sorting#7957
cpwright merged 4 commits intodeephaven:mainfrom
cpwright:coverage/cpw/skip-unnecessary-sorts

Conversation

@cpwright
Copy link
Copy Markdown
Contributor

No description provided.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 28, 2026

No docs changes detected for 262afb0

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR implements a sorting optimization in the Deephaven engine to avoid doing sort work for sort keys that cannot affect ordering (single-value / row-key-agnostic columns), including a no-op “identity” mapping when all sort keys are constant.

Changes:

  • Add logic in SortHelpers.getSortedKeys to drop row-key-agnostic sort columns (when the comparator respects equality) and return an identity mapping when all keys are constant.
  • Refactor row-redirection utilities by extracting StaticWrappedRowSetRowRedirection and reusing it from WrappedRowSetRowRedirection.
  • Add test coverage to validate behavior for constant, null-value, and non-equality-respecting comparator cases.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
engine/table/src/test/java/io/deephaven/engine/table/impl/MultiColumnSortTest.java Adds tests for skipping constant / null sort columns and identity-sort behavior.
engine/table/src/main/java/io/deephaven/engine/table/impl/util/WrappedRowSetRowRedirection.java Refactors to extend StaticWrappedRowSetRowRedirection and updates prev-chunk behavior.
engine/table/src/main/java/io/deephaven/engine/table/impl/util/StaticWrappedRowSetRowRedirection.java New reusable base implementation for wrapped-RowSet redirection.
engine/table/src/main/java/io/deephaven/engine/table/impl/SortOperation.java Treats identity mapping as already-sorted and generalizes historical redirection type.
engine/table/src/main/java/io/deephaven/engine/table/impl/SortHelpers.java Implements constant-column skipping and introduces IdentitySortMapping.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

cpwright and others added 3 commits April 28, 2026 20:42
…iColumnSortTest.java

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…/WrappedRowSetRowRedirection.java

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@cpwright cpwright requested a review from lbooker42 April 29, 2026 00:45
@cpwright cpwright enabled auto-merge (squash) April 29, 2026 00:45
Copy link
Copy Markdown
Contributor

@lbooker42 lbooker42 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 and is rather clever.

@cpwright cpwright merged commit adaaf78 into deephaven:main Apr 29, 2026
23 checks passed
@github-actions github-actions Bot locked and limited conversation to collaborators Apr 29, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants