-
Notifications
You must be signed in to change notification settings - Fork 5.9k
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: prohibit modification decimal precision to avoid inconsistency problem #10433
Conversation
/run-all-tests |
Codecov Report
@@ Coverage Diff @@
## master #10433 +/- ##
================================================
+ Coverage 77.0483% 77.0522% +0.0039%
================================================
Files 412 412
Lines 86508 86501 -7
================================================
- Hits 66653 66651 -2
+ Misses 14744 14740 -4
+ Partials 5111 5110 -1 |
ddl/ddl_api.go
Outdated
@@ -2192,6 +2192,10 @@ func modifiableCharsetAndCollation(toCharset, toCollate, origCharset, origCollat | |||
// It returns true if the two types has the same Charset and Collation, the same sign, both are | |||
// integer types or string types, and new Flen and Decimal must be greater than or equal to origin. | |||
func modifiable(origin *types.FieldType, to *types.FieldType) error { | |||
// Because EncodeDecimal encodes the precision information, if change the decimal precision, need rewrite the record. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The root cause is modifying decimal precision needs to rewrite binary representation of that decimal?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done.
c.Assert(err, NotNil) | ||
c.Assert(err.Error(), Equals, "[ddl:203]unsupported modify decimal column precision") | ||
tk.MustExec(`delete from t1;`) | ||
tk.MustExec(`admin check table t1;`) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why check table after all rows have been deleted?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tidb> create table t1 (a decimal(2,1), index(a))
Query OK, 0 rows affected
Time: 0.070s
tidb> insert into t1 set a='-3.9'
Query OK, 1 row affected
Time: 0.009s
tidb> alter table t1 modify column a DECIMAL(22, 3);
Query OK, 0 rows affected
Time: 0.065s
tidb> delete from t1;
Query OK, 1 row affected
Time: 0.003s
tidb> admin check table t1
(8003, u't1 err:[admin:1]index:&admin.RecordData{Handle:1, Values:[]types.Datum{types.Datum{k:0x8, collation:0x0, decimal:0x1, length:0x2, i:0, b:[]uint8(nil), x:(*types.MyDecimal)(0xc000ab48a0)}}} != record:<nil>')
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
…stency problem (pingcap#10433)" This reverts commit 0e9a6bf.
What problem does this PR solve?
What is changed and how it works?
Because EncodeDecimal encodes the precision information, if change the decimal precision, need rewrite the record.
This PR just simple prohibit modification decimal precision to avoid inconsistency problem.
After TiDB support modifies column type with rewrite data, This problem will be completely solved.
Check List
Tests
Code changes
Side effects
Related changes