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

expression, ddl: check the argument count for the generated column (#22154) #22174

Merged
merged 14 commits into from
Jan 27, 2021
Merged

expression, ddl: check the argument count for the generated column (#22154) #22174

merged 14 commits into from
Jan 27, 2021

Conversation

ti-srebot
Copy link
Contributor

cherry-pick #22154 to release-4.0

What problem does this PR solve?

Issue Number: close #22121

Problem Summary:
We didn't check the argument count for the generated column. So we can create the invalid generated column.
If insert or add index, the server would return errors.

What is changed and how it works?

Check the argument count for the generated column.

Related changes

  • Need to cherry-pick to the release branch

Check List

Tests

  • Unit test

Side effects

Release note

  • Forbidden the invalid generated column with incorrect argument count.

@wjhuang2016 @ti-srebot
cherry pick pingcap#22154 to release-4.0
00cf04a 
Signed-off-by: ti-srebot <ti-srebot@pingcap.com>
@ti-srebot
Copy link
Contributor Author

/run-all-tests

@ti-srebot ti-srebot mentioned this pull request Jan 5, 2021
Merged
@ti-srebot ti-srebot added this to the v4.0.9 milestone Jan 5, 2021
@ti-srebot
Copy link
Contributor Author

@wjhuang2016 you're already a collaborator in bot's repo.

AilinKid
AilinKid reviewed Jan 6, 2021
View reviewed changes
Copy link
Contributor

@AilinKid AilinKid left a comment

Choose a reason for hiding this comment

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

LGTM

@ti-srebot ti-srebot added the status/LGT1 Indicates that a PR has LGTM 1. label Jan 6, 2021
@github-actions github-actions bot added the sig/sql-infra SIG: SQL Infra label Jan 7, 2021
@jebter jebter modified the milestones: v4.0.9, v4.0.10 Jan 7, 2021
@jebter jebter modified the milestones: v4.0.10, v4.0.11 Jan 18, 2021
@qw4990
Copy link
Contributor

qw4990 commented Jan 25, 2021

PTAL @wjhuang2016

qw4990
qw4990 previously approved these changes Jan 26, 2021
View reviewed changes
Copy link
Contributor

@qw4990 qw4990 left a comment

Choose a reason for hiding this comment

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

LGTM

@ti-srebot ti-srebot added status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Jan 26, 2021
@qw4990
Copy link
Contributor

qw4990 commented Jan 26, 2021

/merge

@ti-srebot ti-srebot added the status/can-merge Indicates a PR has been approved by a committer. label Jan 26, 2021
@ti-srebot
Copy link
Contributor Author

Your auto merge job has been accepted, waiting for:

  • 21945
  • 22136
  • 22119
  • 21853

@qw4990
Copy link
Contributor

qw4990 commented Jan 26, 2021

/merge

1 similar comment
@qw4990
Copy link
Contributor

qw4990 commented Jan 26, 2021

/merge

@ti-srebot
Copy link
Contributor Author

Your auto merge job has been accepted, waiting for:

  • 21945
  • 22136
  • 22119
  • 21853
  • 21881
  • 22527
  • 22527
  • 22148

@ti-srebot
Copy link
Contributor Author

/run-all-tests

@ti-srebot
Copy link
Contributor Author

@ti-srebot merge failed.

@ti-srebot
Copy link
Contributor Author

/run-all-tests

@ti-srebot
Copy link
Contributor Author

@ti-srebot merge failed.

@qw4990
Copy link
Contributor

qw4990 commented Jan 27, 2021

/run-all-tests

@qw4990
Copy link
Contributor

qw4990 commented Jan 27, 2021 

[2021-01-27T03:20:39.460Z] FAIL: db_test.go:2903: testDBSuite3.TestGeneratedColumnDDL
[2021-01-27T03:20:39.460Z] 
[2021-01-27T03:20:39.460Z] db_test.go:2956:
[2021-01-27T03:20:39.460Z]     // Test incorrect parameter count.
[2021-01-27T03:20:39.460Z]     s.tk.MustGetErrCode("create table test_gv_incorrect_pc(a double, b int as (lower(a, 2)))", errno.ErrWrongParamcountToNativeFct)
[2021-01-27T03:20:39.460Z] /home/jenkins/agent/workspace/tidb_ghpr_check/go/src/github.com/pingcap/tidb/util/testkit/testkit.go:295:
[2021-01-27T03:20:39.460Z]     tk.c.Assert(err, check.NotNil)
[2021-01-27T03:20:39.460Z] ... value = nil

PTAL @wjhuang2016

@qw4990
Copy link
Contributor

qw4990 commented Jan 27, 2021

/run-all-tests

@qw4990 qw4990 merged commit 7bb48f8 into pingcap:release-4.0 Jan 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@AilinKid AilinKid AilinKid left review comments

@qw4990 qw4990 qw4990 approved these changes

@lzmhhh123 lzmhhh123 Awaiting requested review from lzmhhh123

@tangenta tangenta Awaiting requested review from tangenta

@xiongjiwei xiongjiwei Awaiting requested review from xiongjiwei

Assignees

@wjhuang2016 wjhuang2016

Labels
component/expression sig/sql-infra SIG: SQL Infra status/can-merge Indicates a PR has been approved by a committer. status/LGT2 Indicates that a PR has LGTM 2. type/bugfix This PR fixes a bug. type/4.0-cherry-pick
Projects
None yet
Milestone
v4.0.11
Development

Successfully merging this pull request may close these issues.
6 participants
@ti-srebot @qw4990 @AilinKid @jebter @wjhuang2016 @zz-jason