-
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
Fix bit default value bug #7249
Changes from 13 commits
af00e81
d98444c
d8e001d
21365d1
9190079
c1503eb
f2b8579
619da40
7ce59cd
5ac70c9
1462177
a4fd648
a5d4989
a330405
1e58ed9
5752ccb
c1747f6
0b5d065
5512cbf
8c3bb8d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -22,6 +22,7 @@ import ( | |
"github.com/juju/errors" | ||
"github.com/pingcap/tidb/mysql" | ||
"github.com/pingcap/tidb/types" | ||
"github.com/pingcap/tidb/util/hack" | ||
"github.com/pingcap/tipb/go-tipb" | ||
) | ||
|
||
|
@@ -69,6 +70,7 @@ type ColumnInfo struct { | |
Offset int `json:"offset"` | ||
OriginDefaultValue interface{} `json:"origin_default"` | ||
DefaultValue interface{} `json:"default"` | ||
DefaultValueBit []byte `json:"default_bit"` | ||
GeneratedExprString string `json:"generated_expr_string"` | ||
GeneratedStored bool `json:"generated_stored"` | ||
Dependences map[string]struct{} `json:"dependences"` | ||
|
@@ -88,6 +90,31 @@ func (c *ColumnInfo) IsGenerated() bool { | |
return len(c.GeneratedExprString) != 0 | ||
} | ||
|
||
// SetDefaultValue sets the default value. | ||
func (c *ColumnInfo) SetDefaultValue(value interface{}) error { | ||
c.DefaultValue = value | ||
if c.Tp == mysql.TypeBit { | ||
// For mysql.TypeBit type, the default value storage format must be a string. | ||
// Other value such as int must convert to string format first. | ||
if v, ok := value.(string); ok { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add more description here. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. done. |
||
c.DefaultValueBit = []byte(v) | ||
return nil | ||
} | ||
return types.ErrInvalidDefault.GenByArgs(c.Name) | ||
} | ||
return nil | ||
} | ||
|
||
// GetDefaultValue gets the default value of the column. | ||
// Default value use to stored in DefaultValue field, but now, | ||
// bit type default value will store in DefaultValueBit for fix bit default value decode/encode bug. | ||
func (c *ColumnInfo) GetDefaultValue() interface{} { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we need to change all "c.DefaultValue" to "GetDefaultValue()"? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ye~ I have already changed it. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. "if column.DefaultValue != nil" in column.go, the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we need to replace "c.DefaultValue = types.ZeroDatetimeStr" with "SetDefaultValue()" in ddl_api.go? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. oh... There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. done. |
||
if c.Tp == mysql.TypeBit && c.DefaultValueBit != nil { | ||
return hack.String(c.DefaultValueBit) | ||
} | ||
return c.DefaultValue | ||
} | ||
|
||
// FindColumnInfo finds ColumnInfo in cols by name. | ||
func FindColumnInfo(cols []*ColumnInfo, name string) *ColumnInfo { | ||
name = strings.ToLower(name) | ||
|
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.
Is it possible to use a uniform default value in cloumn info?
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.
https://docs.google.com/document/d/17REO2MQ5v_AFOBE9ovehZDGlrtvhjCy3S1p7F1HWaSc/edit
This document explains the reason to do this way.
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.
It is better to write down how to do it in the description of PR.
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.
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.
@ciscoxll done. PTAL
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.
Will it be incompatible when rolling update TiDB?
As follows case:
On new TiDB, we builds a "add column" job , then we handle the DDL job on Old TiDB.
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.
@crazycs520
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.
@zimulala No. It is compatible.
if old tidb to handle job that created by new tidb, old tidb will ignore the
DefaultValueBit
.I have done an experiment in this scene.