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

*: fix update bug in cluster index #18045

Merged
merged 5 commits into from
Jun 16, 2020

Conversation

wjhuang2016
Copy link
Member

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

What problem does this PR solve?

Issue Number: close #18043

Problem Summary:

What is changed and how it works?

  1. In buildDataSource, consider common handle as a handleCol.
  2. Implement tryDecodeFromHandle for the common handle. So that we can start a server after ignore the EnableClusterIndex checking in create table.
  3. Remove the logic of tryDecodeFromHandle in rowDecoder, it's used only for computer virtual generated column.

Related changes

Check List

Tests

  • Unit test

Side effects

Release note

  • No Release note

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

tk.MustExec("drop table if exists t")
tk.MustExec("create table t(a char(10) primary key, b char(10));")
tk.MustExec("insert into t values(\"a\", \"\");")
Copy link
Member

Choose a reason for hiding this comment

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

You can use single quote to make it more readable.

tk.MustExec("drop table if exists t")
tk.MustExec("create table t(a char(10) primary key, b char(10));")
tk.MustExec("insert into t values(\"a\", \"\");")
tk.MustExec("update t set a=\"c\" where t.a=\"a\" and b=\"\";")
Copy link
Member

Choose a reason for hiding this comment

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

Need select and check the result?

@@ -2827,7 +2827,7 @@ func (b *PlanBuilder) buildDataSource(ctx context.Context, tn *ast.TableName, as
IsHidden: col.Hidden,
}

if tableInfo.PKIsHandle && mysql.HasPriKeyFlag(col.Flag) {
if col.IsPKHandleColumn(tableInfo) || col.IsCommonHandleColumn(tableInfo) {
Copy link
Member

Choose a reason for hiding this comment

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

There could be multiple handleCols if the table has a multi-column primary key.

@codecov
Copy link

codecov bot commented Jun 16, 2020

Codecov Report

Merging #18045 into master will decrease coverage by 0.6953%.
The diff coverage is 44.4444%.

@@               Coverage Diff                @@
##             master     #18045        +/-   ##
================================================
- Coverage   80.1399%   79.4445%   -0.6954%     
================================================
  Files           524        524                
  Lines        147356     142191      -5165     
================================================
- Hits         118091     112963      -5128     
+ Misses        20089      20073        -16     
+ Partials       9176       9155        -21     

Signed-off-by: wjhuang2016 <huangwenjun1997@gmail.com>
Signed-off-by: wjhuang2016 <huangwenjun1997@gmail.com>
Signed-off-by: wjhuang2016 <huangwenjun1997@gmail.com>
@coocood
Copy link
Member

coocood commented Jun 16, 2020

LGTM

@coocood coocood requested a review from lzmhhh123 June 16, 2020 08:23
@coocood
Copy link
Member

coocood commented Jun 16, 2020

/run-all-tests

Copy link
Contributor

@lzmhhh123 lzmhhh123 left a comment

Choose a reason for hiding this comment

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

LGTM

@lzmhhh123 lzmhhh123 added the status/LGT2 Indicates that a PR has LGTM 2. label Jun 16, 2020
@lzmhhh123
Copy link
Contributor

/run-integration-ddl-test

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
sig/execution SIG execution status/LGT2 Indicates that a PR has LGTM 2. type/new-feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

update in cluster index returns Field doesn't have a default value
3 participants