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: fix a batch get executor bug inside a transaction #13473

Merged
merged 5 commits into from
Nov 15, 2019

Conversation

tiancaiamao
Copy link
Contributor

What problem does this PR solve?

mysql> create table t (id int primary key auto_increment, name varchar(30));  
Query OK, 0 rows affected (0.01 sec)
mysql> begin; 
Query OK, 0 rows affected (0.00 sec)
mysql> insert into t values (4, 'name'); 
Query OK, 1 row affected (0.01 sec)

mysql> select * from t where id in (4); 
Empty set (0.00 sec)           // This is a bug, should be (4, 'name')!        

mysql> explain select * from t where id in (4);
+-------------------+-------+------+---------------+
| id                | count | task | operator info |
+-------------------+-------+------+---------------+
| Batch_Point_Get_1 | 1.00  | root | table:t       |
+-------------------+-------+------+---------------+
1 row in set (0.00 sec)

What is changed and how it works?

When the BatchGetExec runs inside a transaction, it should consider the data changes
in the membuffer.

Check List

Tests

  • Unit test

Release note

  • Write release note for bug-fix or new feature.

When the BatchGetExec runs inside a transaction, it should consider the data changes
in the membuffer.
@tiancaiamao tiancaiamao added type/bugfix This PR fixes a bug. sig/execution SIG execution labels Nov 14, 2019
@codecov
Copy link

codecov bot commented Nov 14, 2019

Codecov Report

Merging #13473 into master will decrease coverage by 0.1457%.
The diff coverage is 83.3333%.

@@               Coverage Diff                @@
##             master     #13473        +/-   ##
================================================
- Coverage   80.3601%   80.2144%   -0.1458%     
================================================
  Files           472        472                
  Lines        114772     114245       -527     
================================================
- Hits          92231      91641       -590     
- Misses        15387      15434        +47     
- Partials       7154       7170        +16

@zimulala
Copy link
Contributor

/run-integration-copr-test

Copy link
Member

@jackysp jackysp left a comment

Choose a reason for hiding this comment

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

LGTM

@coocood
Copy link
Member

coocood commented Nov 15, 2019

LGTM

@coocood
Copy link
Member

coocood commented Nov 15, 2019

/run-all-tests

@coocood coocood merged commit 40c9f9c into pingcap:master Nov 15, 2019
@sre-bot
Copy link
Contributor

sre-bot commented Nov 15, 2019

cherry pick to release-3.0 failed

@@ -1445,7 +1445,7 @@ func iterateSnapshotRows(store kv.Storage, priority int, t table.Table, version
ver := kv.Version{Ver: version}

snap, err := store.GetSnapshot(ver)
snap.SetPriority(priority)
snap.SetOption(kv.Priority, priority)
Copy link
Member

Choose a reason for hiding this comment

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

Is this change relative to the bug?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This change can save many lines of code.

@tiancaiamao tiancaiamao deleted the batch-get-txn branch November 15, 2019 08:57
XiaTianliang pushed a commit to XiaTianliang/tidb that referenced this pull request Dec 21, 2019
…13473)

When the BatchGetExec runs inside a transaction, it should consider the data changes
in the membuffer.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
sig/execution SIG execution type/bugfix This PR fixes a bug.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants