Skip to content

copr: support common handle for index_lookup_executor#19159

Merged
ti-chi-bot[bot] merged 1 commit intotikv:masterfrom
lcwangchao:common_handle_indexlookup_pushdown
Dec 2, 2025
Merged

copr: support common handle for index_lookup_executor#19159
ti-chi-bot[bot] merged 1 commit intotikv:masterfrom
lcwangchao:common_handle_indexlookup_pushdown

Conversation

@lcwangchao
Copy link
Contributor

@lcwangchao lcwangchao commented Dec 2, 2025

What is changed and how it works?

Issue Number: Close #19158

What's Changed:

copr: support common handle for index_lookup_executor

Related changes

  • PR to update pingcap/docs/pingcap/docs-cn:
  • Need to cherry-pick to the release branch

Check List

Tests

  • Unit test
  • Integration test
  • Manual test (add detailed scripts or steps below)
  • No code

Side effects

  • Performance regression: Consumes more CPU
  • Performance regression: Consumes more Memory
  • Breaking backward compatibility

Release note

None

Copilot AI review requested due to automatic review settings December 2, 2025 01:58
@ti-chi-bot ti-chi-bot bot added release-note-none Denotes a PR that doesn't merit a release note. contribution This PR is from a community contributor. labels Dec 2, 2025
@ti-chi-bot
Copy link
Contributor

ti-chi-bot bot commented Dec 2, 2025

Hi @lcwangchao. Thanks for your PR.

I'm waiting for a tikv member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@ti-chi-bot ti-chi-bot bot added needs-ok-to-test Indicates a PR created by contributors and need ORG member send '/ok-to-test' to start testing. dco-signoff: no Indicates the PR's author has not signed dco. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Dec 2, 2025
Copilot AI reviewed Dec 2, 2025
View reviewed changes
Copy link
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 adds support for common handles in the index lookup executor, enabling TiKV to handle tables with composite primary keys during index scan and lookup operations. Previously, the index lookup executor only supported integer handles (single-column primary keys).

Key Changes:

  • Introduced CommonHandle type alongside existing IntHandle to support composite primary keys
  • Added extra_common_handle_keys field to LazyBatchColumnVec for passing handle data between executors
  • Refactored RowHandle::from_lazy_batch_column_vec() to process multiple rows at once

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
components/tidb_query_datatype/src/codec/batch/lazy_column_vec.rs Added extra_common_handle_keys field to store common handle keys for passing between executors
components/tidb_query_datatype/src/codec/table.rs Refactored RowHandle trait to process batches; added CommonHandle implementation; renamed encode_common_handle_for_test to encode_common_handle for production use
components/tidb_query_executors/src/index_scan_executor.rs Added fill_extra_common_handle_key parameter to populate common handle keys during index scans
components/tidb_query_executors/src/index_lookup_executor.rs Refactored to use generic Handle type parameter and process handles in batches
components/tidb_query_executors/src/runner.rs Added is_executor_under_parent_tp() to detect executor hierarchy; updated builder to choose between IntHandle and CommonHandle based on table schema
components/tidb_query_executors/src/projection_executor.rs Added error handling to reject queries with common handle keys (not yet supported)
components/tidb_query_executors/src/util/top_n_heap.rs Added logic to copy common handle keys when building TopN results
components/tidb_query_executors/src/table_scan_executor.rs Updated test utilities to use renamed encode_common_handle function
tests/benches/coprocessor_executors/index_scan/util.rs Updated test executor creation to include new fill_extra_common_handle_key parameter

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

@lcwangchao lcwangchao force-pushed the common_handle_indexlookup_pushdown branch from bcdc47e to 9311a16 Compare December 2, 2025 02:03
@ti-chi-bot ti-chi-bot bot added dco-signoff: yes Indicates the PR's author has signed the dco. and removed dco-signoff: no Indicates the PR's author has not signed dco. labels Dec 2, 2025
@lcwangchao lcwangchao force-pushed the common_handle_indexlookup_pushdown branch from 9311a16 to b08d03c Compare December 2, 2025 02:30
@cfzjywxk
Copy link
Collaborator

cfzjywxk commented Dec 2, 2025

/ok-to-test

@ti-chi-bot ti-chi-bot bot added ok-to-test Indicates a PR is ready to be tested. and removed needs-ok-to-test Indicates a PR created by contributors and need ORG member send '/ok-to-test' to start testing. labels Dec 2, 2025
cfzjywxk
cfzjywxk approved these changes Dec 2, 2025
View reviewed changes
Copy link
Collaborator

@cfzjywxk cfzjywxk left a comment

Choose a reason for hiding this comment

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

LGTM

@ti-chi-bot ti-chi-bot bot added needs-1-more-lgtm Indicates a PR needs 1 more LGTM. approved labels Dec 2, 2025
@cfzjywxk
Copy link
Collaborator

cfzjywxk commented Dec 2, 2025

@lcwangchao Please fix the lint error

@lcwangchao
copr: support common handle for index_lookup_executor
3653123 
Signed-off-by: Chao Wang <cclcwangchao@hotmail.com>
@lcwangchao lcwangchao force-pushed the common_handle_indexlookup_pushdown branch from b08d03c to 3653123 Compare December 2, 2025 05:08
@ti-chi-bot ti-chi-bot bot added the lgtm label Dec 2, 2025
@ti-chi-bot
Copy link
Contributor

ti-chi-bot bot commented Dec 2, 2025

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cfzjywxk, you06

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@ti-chi-bot ti-chi-bot bot removed the needs-1-more-lgtm Indicates a PR needs 1 more LGTM. label Dec 2, 2025
@ti-chi-bot
Copy link
Contributor

ti-chi-bot bot commented Dec 2, 2025

[LGTM Timeline notifier]

Timeline:

  • 2025-12-02 03:42:51.695243682 +0000 UTC m=+321316.509021255: ☑️ agreed by cfzjywxk.
  • 2025-12-02 06:02:48.23913593 +0000 UTC m=+329713.052913502: ☑️ agreed by you06.

@ti-chi-bot ti-chi-bot bot merged commit 95cb136 into tikv:master Dec 2, 2025
9 checks passed
@ti-chi-bot ti-chi-bot bot added this to the Pool milestone Dec 2, 2025
@lcwangchao lcwangchao deleted the common_handle_indexlookup_pushdown branch December 2, 2025 06:11
lcwangchao added a commit to lcwangchao/tikv that referenced this pull request Dec 2, 2025
@lcwangchao
copr: support common handle for index_lookup_executor (tikv#19159)
0e4c610 
close tikv#19158

copr: support common handle for index_lookup_executor

Signed-off-by: Chao Wang <cclcwangchao@hotmail.com>
@lcwangchao lcwangchao mentioned this pull request Dec 2, 2025
Merged
9 tasks
ti-chi-bot bot pushed a commit that referenced this pull request Dec 2, 2025
@lcwangchao
copr: support common handle for index_lookup_executor (#19159) (#19160)
029a892 
ref #18736, close #19158

support common handle for index_lookup_executor

Signed-off-by: Chao Wang <cclcwangchao@hotmail.com>
bufferflies pushed a commit to bufferflies/tikv that referenced this pull request Dec 26, 2025
@crazycs520 @lcwangchao
copr: support common handle for index_lookup_executor (tikv#19159) (t…
c85a029 
…ikv#4088)

* copr: support common handle for index_lookup_executor (tikv#19159)

close tikv#19158

copr: support common handle for index_lookup_executor

Signed-off-by: Chao Wang <cclcwangchao@hotmail.com>

* copr: fix TopN can not return the right extra common handle keys (tikv#19173)

ref tikv#18736, close pingcap/tidb#64866

- Fix TopN can not return the right extra common handle keys
- Also make BatchProjectionExecutor to support extra common handle keys
- Added more tests to test extra common handle keys

Signed-off-by: Chao Wang <cclcwangchao@hotmail.com>

* address comment

Signed-off-by: crazycs520 <crazycs520@gmail.com>

---------

Signed-off-by: Chao Wang <cclcwangchao@hotmail.com>
Signed-off-by: crazycs520 <crazycs520@gmail.com>
Co-authored-by: Chao Wang <cclcwangchao@hotmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

@cfzjywxk cfzjywxk cfzjywxk approved these changes

@you06 you06 you06 approved these changes

@zyguan zyguan Awaiting requested review from zyguan

@ekexium ekexium Awaiting requested review from ekexium

Assignees

No one assigned

Labels

approved contribution This PR is from a community contributor. dco-signoff: yes Indicates the PR's author has signed the dco. lgtm ok-to-test Indicates a PR is ready to be tested. release-note-none Denotes a PR that doesn't merit a release note. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.

Projects

None yet

Milestone

Pool

Development

Successfully merging this pull request may close these issues.

Support common handle in IndexLookUp push-down

4 participants

@lcwangchao @cfzjywxk @you06