-
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: ignore integer zerofill size attribute when changing the column types #20862
Conversation
Signed-off-by: AilinKid <314806019@qq.com>
ddl/column.go
Outdated
needTruncationOrToggleSignForInteger := func() bool { | ||
return newColFlen > 0 && newColFlen < oldColFlen || toUnsigned != originUnsigned | ||
} |
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 not use a variable instead?
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.
Cause for most of the cases, we don't need to trigger the computation of it, like a kind of lazy call if necessary?
Signed-off-by: AilinKid <314806019@qq.com>
Signed-off-by: AilinKid <314806019@qq.com>
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
/run-all-tests |
LGTM |
/merge |
/run-all-tests |
@AilinKid merge failed. |
/run-unit-test |
1 similar comment
/run-unit-test |
Signed-off-by: ti-srebot <ti-srebot@pingcap.com>
cherry pick to release-4.0 in PR #20986 |
What problem does this PR solve?
Issue Number: close #20529
What is changed and how it works?
how does it work?
Like int(3), bigint(2), the number enclosed will be stored as
flen
in the field type, while it shouldn't be concerned when we make column type change from int(3) to(2) . Because bigint is quite larger than int, there is no need to take a reorg process.what is changed
We should use the default
flen
of the specified field type rather than the originflen
.Check List
Tests
Release note