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

ddl, util: Handle default values for generating column expressions when adding an index #9371

Merged
merged 8 commits into from
Feb 27, 2019

Conversation

zimulala
Copy link
Contributor

What problem does this PR solve?

Fix #9311
Before this PR:

tidb> create table t(id int primary key);
Query OK, 0 rows affected (0.01 sec)
tidb> insert into t values(1);
Query OK, 1 row affected (0.00 sec)
tidb> ALTER TABLE t ADD COLUMN d date DEFAULT '9999-12-31';
Query OK, 0 rows affected (0.02 sec)
tidb> ALTER TABLE t ADD COLUMN d1 date as (DATE_SUB(d, INTERVAL 31 DAY));
Query OK, 0 rows affected (0.02 sec)
tidb> ALTER TABLE t ADD INDEX idx_d(d1);
ERROR 1105 (HY000): cannot decode index value, because [types:1292]invalid time format: '{0 0 0 0 0 0 0}'
tidb> ALTER TABLE t ADD COLUMN id1 int as (id+1);
Query OK, 0 rows affected (0.02 sec)
tidb> ALTER TABLE t ADD INDEX idx1(id1); // The idx1's value is 0 in store. 
Query OK, 0 rows affected (0.02 sec)
tidb> select id1 from t use index(idx1); // The idx'1 value is 0, but we will do `cast(plus(test.t.id, 1))`.
+------+
| id1  |
+------+
|    2 |
+------+
1 row in set (0.00 sec)

Now:

tidb> create table t(id int primary key);
Query OK, 0 rows affected (0.01 sec)
tidb> insert into t values(1);
Query OK, 1 row affected (0.00 sec)
tidb> ALTER TABLE t ADD COLUMN d date DEFAULT '9999-12-31';
Query OK, 0 rows affected (0.02 sec)
tidb> ALTER TABLE t ADD COLUMN d1 date as (DATE_SUB(d, INTERVAL 31 DAY));
Query OK, 0 rows affected (0.02 sec)
tidb> ALTER TABLE t ADD INDEX idx_d(d1);
Query OK, 0 rows affected (0.07 sec)
tidb> ALTER TABLE t ADD COLUMN id1 int as (id+1);
Query OK, 0 rows affected (0.02 sec)
tidb> ALTER TABLE t ADD INDEX idx1(id1);
Query OK, 0 rows affected (0.02 sec)
tidb> select id1 from t use index(idx1);
+------+
| id1  |
+------+
|    2 |
+------+
1 row in set (0.00 sec)

What is changed and how it works?

Add default value and handle value to the row before evaluating the generated expression.

Check List

Tests

  • Unit test

Code changes

  • Has exported function/method change

Related changes

  • Need to cherry-pick to the release branch

@zimulala
Copy link
Contributor Author

/run-all-tests

@codecov-io
Copy link

codecov-io commented Feb 20, 2019

Codecov Report

Merging #9371 into master will increase coverage by <.01%.
The diff coverage is 88.88%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #9371      +/-   ##
==========================================
+ Coverage   67.41%   67.42%   +<.01%     
==========================================
  Files         373      373              
  Lines       78557    78557              
==========================================
+ Hits        52962    52966       +4     
+ Misses      20840    20837       -3     
+ Partials     4755     4754       -1
Impacted Files Coverage Δ
ddl/index.go 80.45% <100%> (ø) ⬆️
util/admin/admin.go 65.85% <80%> (ø) ⬆️
expression/schema.go 93.75% <0%> (-0.79%) ⬇️
executor/distsql.go 71.78% <0%> (-0.46%) ⬇️
executor/executor.go 68.16% <0%> (+0.13%) ⬆️
executor/join.go 79.58% <0%> (+0.51%) ⬆️
store/tikv/scan.go 77.31% <0%> (+3.36%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 45f9d6a...0192452. Read the comment docs.

@zimulala
Copy link
Contributor Author

PTAL @winkyao @crazycs520

1 similar comment
@zimulala
Copy link
Contributor Author

PTAL @winkyao @crazycs520

Copy link
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
Contributor

/run-all-tests

Copy link
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

@tiancaiamao
Copy link
Contributor

LGTM

@tiancaiamao tiancaiamao added the status/LGT3 The PR has already had 3 LGTM. label Feb 26, 2019
@zz-jason zz-jason merged commit 8369ed2 into pingcap:master Feb 27, 2019
@zimulala zimulala deleted the fix-gcExpr-bug branch March 21, 2019 06:46
zimulala added a commit to zimulala/tidb that referenced this pull request Aug 8, 2019
@you06 you06 added the sig/sql-infra SIG: SQL Infra label Mar 4, 2020
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.

Add an index to the virtual generated column whose expression contains the column added by "add column"
7 participants