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.

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

executor: should handle virtual columns when fetching duplicate rows in batchChecker #10370

Merged
merged 6 commits into from May 8, 2019

Conversation

Projects
None yet
4 participants
@jackysp
Copy link
Member

commented May 6, 2019

Signed-off-by: Shuaipeng Yu jackysp@gmail.com

What problem does this PR solve?

When fetching the duplicate rows in batchChecker, the virtual columns won't contribute to the dupKV map. So the replace/insert on duplicate will always find duplicate keys.

What is changed and how it works?

Should handle virtual columns when fetching duplicate rows in batchChecker.

Check List

Tests

  • Unit test

Code changes

  • Has exported function/method change

Side effects

  • Increased code complexity

Related changes

  • Need to cherry-pick to the release branch
@jackysp

This comment has been minimized.

Copy link
Member Author

commented May 6, 2019

I'll add some cases tomorrow.

@codecov

This comment has been minimized.

Copy link

commented May 6, 2019

Codecov Report

Merging #10370 into master will increase coverage by 0.0198%.
The diff coverage is 66.6666%.

@@               Coverage Diff                @@
##             master     #10370        +/-   ##
================================================
+ Coverage   77.6497%   77.6695%   +0.0198%     
================================================
  Files           412        411         -1     
  Lines         85547      85444       -103     
================================================
- Hits          66427      66364        -63     
+ Misses        14150      14116        -34     
+ Partials       4970       4964         -6
@codecov

This comment has been minimized.

Copy link

commented May 7, 2019

Codecov Report

Merging #10370 into master will increase coverage by 0.0201%.
The diff coverage is 69.2307%.

@@               Coverage Diff                @@
##             master     #10370        +/-   ##
================================================
+ Coverage   77.3876%   77.4078%   +0.0201%     
================================================
  Files           412        412                
  Lines         85586      85596        +10     
================================================
+ Hits          66233      66258        +25     
+ Misses        14334      14324        -10     
+ Partials       5019       5014         -5

@jackysp jackysp marked this pull request as ready for review May 7, 2019

jackysp added some commits May 6, 2019

get row should handle virtual columns
Signed-off-by: Shuaipeng Yu <jackysp@gmail.com>
add a replace test
Signed-off-by: Shuaipeng Yu <jackysp@gmail.com>
add test case for insert on dup
Signed-off-by: Shuaipeng Yu <jackysp@gmail.com>
update case name
Signed-off-by: Shuaipeng Yu <jackysp@gmail.com>

@jackysp jackysp force-pushed the jackysp:fix_gen_batch branch from f9bc7f5 to 79e42bf May 7, 2019

@jackysp

This comment has been minimized.

Copy link
Member Author

commented May 7, 2019

/run-all-tests

@jackysp jackysp requested review from tiancaiamao and amyangfei May 7, 2019

eval for virtual only
Signed-off-by: Shuaipeng Yu <jackysp@gmail.com>
@jackysp

This comment has been minimized.

Copy link
Member Author

commented May 7, 2019

/run-all-tests

@winoros
Copy link
Member

left a comment

lgtm

@winoros winoros added the status/LGT1 label May 7, 2019

@amyangfei
Copy link
Member

left a comment

LGTM

@amyangfei amyangfei added status/LGT2 and removed status/LGT1 labels May 7, 2019

@ngaut

ngaut approved these changes May 8, 2019

@jackysp jackysp merged commit 1c4ebee into pingcap:master May 8, 2019

8 checks passed

ci/circleci Your tests passed on CircleCI!
Details
codecov/patch 69.2307% of diff hit (target 0%)
Details
codecov/project 77.4078% (+0.0201%) compared to 1ef1d8a
Details
idc-jenkins-ci-tidb/build Jenkins job succeeded.
Details
idc-jenkins-ci-tidb/build_check_race Jenkins job succeeded.
Details
idc-jenkins-ci-tidb/check_dev Jenkins job succeeded.
Details
idc-jenkins-ci-tidb/check_dev_2 Jenkins job succeeded.
Details
license/cla Contributor License Agreement is signed.
Details

jackysp added a commit to jackysp/tidb that referenced this pull request May 8, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.