Skip to content
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

*: new secondary index value format #20220

Merged
merged 39 commits into from
Mar 3, 2021
Merged

*: new secondary index value format #20220

merged 39 commits into from
Mar 3, 2021

Conversation

wjhuang2016
Copy link
Member

@wjhuang2016 wjhuang2016 commented Sep 25, 2020 •
edited
Loading

Signed-off-by: wjhuang2016 huangwenjun1997@gmail.com

What problem does this PR solve?

  1. Optimized _bin collation.
  2. Only write the restored data for the new collation column in composed indexes.
  3. Write restored data for the new collation column in the common handle.

Fix #19942 completely.

What is changed and how it works?

  1. When creating the clustered index, set the CommonHandleVersion to 1.
  2. Add a method TryGetHandleRestoredDataWrapper to get the restored data for the clustered index. Ignore it if the version is 0.
  3. Add a parameter(handleRestoreData) for index.create.
  4. Add rsData(restored data for handle) to struct indexRecord and recoverRows.
  5. IndexLookUpExecutor.getHandle(), for previous logic, add condition e.table.Meta().CommonHandleVersion == 0.
  6. DataSource.isCoveringIndex(), for previous logic, add condition e.table.Meta().CommonHandleVersion == 0.
  7. Remove interface and the implementation of GenIndexValue, use GenIndexValuePortal instead.
  8. Change containNonBinaryString to needRestoredData. NeedRestoredData() is more accurate than ContainsNonBinaryString().
  9. Add IndexVersionFlag 125.
  10. Implement decodeIndexKvForClusteredIndexVersion1()
  11. Implement getIndexVersion()
  12. In DecodeHandleInUniqueIndexValue(), handle case ClusteredIndexVersion1
  13. In decodeHandleInIndexValue(), handle case ClusteredIndexVersion1

Related changes

Check List

Tests

  • Unit test

Side effects

Release note

  • No release note

@wjhuang2016
1
10a6926 
Signed-off-by: wjhuang2016 <huangwenjun1997@gmail.com>
@wjhuang2016 wjhuang2016 requested review from a team as code owners September 25, 2020 09:35
@wjhuang2016 wjhuang2016 requested review from SunRunAway, tangenta and coocood and removed request for a team September 25, 2020 09:35
@wjhuang2016
merge master
4cbf9e0 
Signed-off-by: wjhuang2016 <huangwenjun1997@gmail.com>
tangenta
tangenta reviewed Sep 27, 2020
View reviewed changes
table/tables/index_test.go Show resolved Hide resolved
tablecodec/tablecodec.go Show resolved Hide resolved
tablecodec/tablecodec.go Outdated Show resolved Hide resolved
tablecodec/tablecodec.go Outdated Show resolved Hide resolved
@wjhuang2016
address comment
56bf441 
Signed-off-by: wjhuang2016 <huangwenjun1997@gmail.com>
@wjhuang2016
Merge branch 'master' of https://github.com/pingcap/tidb into new_ind…
381f49b 
…ex_encode_V5
tangenta
tangenta reviewed Oct 13, 2020
View reviewed changes
table/tables/index.go Show resolved Hide resolved
table/tables/tables.go Show resolved Hide resolved
tablecodec/tablecodec.go Outdated Show resolved Hide resolved
coocood
coocood reviewed Oct 16, 2020
View reviewed changes
tablecodec/tablecodec.go Outdated Show resolved Hide resolved
coocood
coocood reviewed Oct 16, 2020
View reviewed changes
tablecodec/tablecodec.go Outdated Show resolved Hide resolved
coocood
coocood reviewed Oct 16, 2020
View reviewed changes
tablecodec/tablecodec.go Outdated Show resolved Hide resolved
@wjhuang2016
Merge branch 'master' of https://github.com/pingcap/tidb into new_ind…
c05c23c 
…ex_encode_V5
@wjhuang2016
1
e7876f1 
Signed-off-by: wjhuang2016 <huangwenjun1997@gmail.com>
@wjhuang2016
address comment
231bc1b 
Signed-off-by: wjhuang2016 <huangwenjun1997@gmail.com>
@wjhuang2016
1
4ef5cd5 
Signed-off-by: wjhuang2016 <huangwenjun1997@gmail.com>
@wjhuang2016
2
212012e 
Signed-off-by: wjhuang2016 <huangwenjun1997@gmail.com>
@wjhuang2016
fix
85c23bc 
Signed-off-by: wjhuang2016 <huangwenjun1997@gmail.com>
@wjhuang2016
fix
c94aca9 
Signed-off-by: wjhuang2016 <huangwenjun1997@gmail.com>
ti-chi-bot pushed a commit to tikv/tikv that referenced this pull request Mar 3, 2021
@wjhuang2016
copr: support decoding the new index format (#9383)
32ec29d 
Signed-off-by: wjhuang2016 <huangwenjun1997@gmail.com>

<!--
Thank you for contributing to TiKV!

If you haven't already, please read TiKV's [CONTRIBUTING](https://github.com/tikv/tikv/blob/master/CONTRIBUTING.md) document.

If you're unsure about anything, just ask; somebody should be along to answer within a day or two.

PR Title Format:
1. module [, module2, module3]: what's changed
2. *: what's changed

If you want to open the **Challenge Program** pull request, please use the following template:
https://raw.githubusercontent.com/tikv/.github/master/.github/PULL_REQUEST_TEMPLATE/challenge-program.md
You can use it with query parameters: https://github.com/tikv/tikv/compare/master...${you branch}?template=challenge-program.md
-->

### What problem does this PR solve?

Refer to pingcap/tidb#20220

### What is changed and how it works?

Proposal: [xxx](url) <!-- REMOVE this line if not applicable -->

What's Changed:

### Related changes



### Check List <!--REMOVE the items that are not applicable-->

Tests <!-- At least one of them must be included. -->

- Unit test


Side effects



###  Release note
- No release note
@wjhuang2016
Copy link
Member Author

/merge

@ti-chi-bot
Copy link
Member

@wjhuang2016: It seems you want to merge this PR, I will help you trigger all the tests:

/run-all-tests

You only need to trigger /merge once, and if the CI test fails, you just re-trigger the test that failed and the bot will merge the PR for you after the CI passes.

If you have any questions about the PR merge process, please refer to pr process.

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 ti-community-infra/tichi repository.

@ti-chi-bot
Copy link
Member

This pull request has been accepted and is ready to merge.

Commit hash: db099eecdb1ac7ac335326745a94895cc428d893

@ti-chi-bot ti-chi-bot added status/can-merge Indicates a PR has been approved by a committer. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Mar 3, 2021
@wjhuang2016
merge
a91b678 
Signed-off-by: wjhuang2016 <huangwenjun1997@gmail.com>
@ti-chi-bot ti-chi-bot removed status/can-merge Indicates a PR has been approved by a committer. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Mar 3, 2021
@coocood
Copy link
Member

coocood commented Mar 3, 2021

/lgtm

@coocood
Copy link
Member

coocood commented Mar 3, 2021

/merge

@ti-chi-bot
Copy link
Member

@coocood: It seems you want to merge this PR, I will help you trigger all the tests:

/run-all-tests

You only need to trigger /merge once, and if the CI test fails, you just re-trigger the test that failed and the bot will merge the PR for you after the CI passes.

If you have any questions about the PR merge process, please refer to pr process.

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 ti-community-infra/tichi repository.

@ti-chi-bot
Copy link
Member

This pull request has been accepted and is ready to merge.

Commit hash: a91b678

@ti-chi-bot ti-chi-bot added the status/can-merge Indicates a PR has been approved by a committer. label Mar 3, 2021
@ti-chi-bot
Copy link
Member

@wjhuang2016: Your PR was out of date, I have automatically updated it for you.

At the same time I will also trigger all tests for you:

/run-all-tests

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 ti-community-infra/tichi repository.

@Rustin170506
Copy link
Member

/run-tics-test

1 similar comment
@Rustin170506
Copy link
Member

/run-tics-test

@ti-chi-bot ti-chi-bot merged commit b5a9076 into pingcap:master Mar 3, 2021
@Tjianke Tjianke mentioned this pull request Mar 4, 2021
Merged
@tangenta tangenta mentioned this pull request Mar 5, 2021
Closed
@wjhuang2016 wjhuang2016 mentioned this pull request May 25, 2021
Closed
This was referenced Aug 27, 2022
Closed
Merged
@wjhuang2016 wjhuang2016 deleted the new_index_encode_V5 branch November 17, 2022 11:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@coocood coocood coocood left review comments

@tangenta tangenta tangenta approved these changes

@SunRunAway SunRunAway Awaiting requested review from SunRunAway

Assignees
No one assigned
Labels
component/expression sig/execution SIG execution sig/sql-infra SIG: SQL Infra size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. status/can-merge Indicates a PR has been approved by a committer. status/LGT2 Indicates that a PR has LGTM 2.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

*: unexpected error with both new collation and clustered index are enabled.
7 participants
@wjhuang2016 @coocood @ti-chi-bot @tangenta @lysu @Rustin170506 @ti-srebot