Skip to content

ddl: fix the parallel problem of "set default value" and other DDL#11341

Merged
bb7133 merged 3 commits into
pingcap:masterfrom
zimulala:set-default-value
Jul 20, 2019
Merged

ddl: fix the parallel problem of "set default value" and other DDL#11341
bb7133 merged 3 commits into
pingcap:masterfrom
zimulala:set-default-value

Conversation

@zimulala
Copy link
Copy Markdown
Contributor

@zimulala zimulala commented Jul 19, 2019

What problem does this PR solve?

The problem occurs as follows:

create table tx (c1 varchar(64), c2 enum('N','Y') not null default 'N', primary key idx2 (c2, c1));
insert into tx values('a', 'N');

Then execute the following DDL statements in parallel:

alter table tx add column cx int after c1
alter table tx alter c2 set default 'N'

Finally execute the following statement:

delete from tx where c1='a'

We will get the error as follows:

value *errors.withStack = [table:2]Index column c2 offset out of bound, offset: 2, row: [{6 0 0 0 0 [97] <nil>} {10 0 0 0 1 [78] <nil>}] ("[table:2]Index column c2 offset out of bound, offset: 2, row: [{6 0 0 0 0 [97] <nil>} {10 0 0 0 1 [78] <nil>}]")

What is changed and how it works?

We don't change the offset when executing the DDL of alter table .. set default.

Check List

Tests

  • Unit test

Related changes

  • Need to cherry-pick to the release branch

@codecov
Copy link
Copy Markdown

codecov Bot commented Jul 19, 2019

Codecov Report

Merging #11341 into master will decrease coverage by 0.2067%.
The diff coverage is 100%.

@@               Coverage Diff               @@
##             master    #11341        +/-   ##
===============================================
- Coverage   81.4477%   81.241%   -0.2068%     
===============================================
  Files           423       423                
  Lines         90884     90165       -719     
===============================================
- Hits          74023     73251       -772     
- Misses        11577     11602        +25     
- Partials       5284      5312        +28

Copy link
Copy Markdown
Contributor

@crazycs520 crazycs520 left a comment

Choose a reason for hiding this comment

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

LGTM

@crazycs520
Copy link
Copy Markdown
Contributor

/run-all-tests

@zimulala zimulala added status/LGT1 Indicates that a PR has LGTM 1. status/all tests passed labels Jul 19, 2019
Copy link
Copy Markdown
Contributor

@winkyao winkyao left a comment

Choose a reason for hiding this comment

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

LGTM

@winkyao winkyao added status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Jul 19, 2019
@bb7133 bb7133 added status/LGT3 The PR has already had 3 LGTM. and removed status/LGT2 Indicates that a PR has LGTM 2. labels Jul 20, 2019
Copy link
Copy Markdown
Member

@bb7133 bb7133 left a comment

Choose a reason for hiding this comment

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

LGTM

@bb7133 bb7133 merged commit 85de5df into pingcap:master Jul 20, 2019
@sre-bot
Copy link
Copy Markdown
Contributor

sre-bot commented Jul 20, 2019

cherry pick to release-2.1 failed

@sre-bot
Copy link
Copy Markdown
Contributor

sre-bot commented Jul 20, 2019

cherry pick to release-3.0 in PR #11346

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

sig/sql-infra SIG: SQL Infra status/LGT3 The PR has already had 3 LGTM. type/bugfix This PR fixes a bug.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants