Skip to content

Commit

Permalink
sqlparser: No literals used for length & scale
Browse files Browse the repository at this point in the history
We don't want to use literals for length & scale, since things like the
planner treat literals as expressions (since they are!) and act on them.

Length & scale properties are type properties though and not expressions
and they should never be changed / extracted / or anything like that.

Signed-off-by: Dirkjan Bussink <d.bussink@gmail.com>
  • Loading branch information
dbussink committed Feb 15, 2024
1 parent 365f936 commit 9f6f007
Show file tree
Hide file tree
Showing 17 changed files with 1,500 additions and 1,567 deletions.
33 changes: 14 additions & 19 deletions go/vt/schemadiff/table.go
Original file line number Diff line number Diff line change
Expand Up @@ -522,18 +522,15 @@ func (c *CreateTableEntity) normalizeColumnOptions() {
// We can remove the length except when we have a boolean, which is
// stored as a tinyint(1) and treated special.
if !isBool(col.Type) {
col.Type.Length = nil
col.Type.Length = -1
}
}

// Normalize boolean to tinyint(1). Otherwise we get a diff if the desired schema has a boolean column because
// "show create table" reports it as a tinyint(1).
if col.Type.Type == "boolean" {
col.Type.Type = "tinyint"
col.Type.Length = &sqlparser.Literal{
Type: sqlparser.IntVal,
Val: "1",
}
col.Type.Length = 1

if col.Type.Options.Default != nil {
val, ok := col.Type.Options.Default.(sqlparser.BoolVal)
Expand Down Expand Up @@ -562,18 +559,16 @@ func (c *CreateTableEntity) normalizeColumnOptions() {
col.Type.Type = "double"
}

if col.Type.Length != nil && col.Type.Scale == nil && col.Type.Length.Type == sqlparser.IntVal {
if l, err := strconv.ParseInt(col.Type.Length.Val, 10, 64); err == nil {
// See https://dev.mysql.com/doc/refman/8.0/en/floating-point-types.html, but the docs are
// subtly wrong. We use a float for a precision of 24, not a double as the documentation
// mentioned. Validated against the actual behavior of MySQL.
if l <= 24 {
col.Type.Type = "float"
} else {
col.Type.Type = "double"
}
if col.Type.Length >= 0 && col.Type.Scale == -1 {
// See https://dev.mysql.com/doc/refman/8.0/en/floating-point-types.html, but the docs are
// subtly wrong. We use a float for a precision of 24, not a double as the documentation
// mentioned. Validated against the actual behavior of MySQL.
if col.Type.Length <= 24 {
col.Type.Type = "float"
} else {
col.Type.Type = "double"
}
col.Type.Length = nil
col.Type.Length = -1
}
}

Expand Down Expand Up @@ -627,7 +622,7 @@ func (c *CreateTableEntity) normalizeIndexOptions() {
}

func isBool(colType *sqlparser.ColumnType) bool {
return colType.Type == sqlparser.KeywordString(sqlparser.TINYINT) && colType.Length != nil && sqlparser.CanonicalString(colType.Length) == "1"
return colType.Type == sqlparser.KeywordString(sqlparser.TINYINT) && colType.Length == 1
}

func (c *CreateTableEntity) normalizePartitionOptions() {
Expand All @@ -653,7 +648,7 @@ func newPrimaryKeyIndexDefinitionSingleColumn(name sqlparser.IdentifierCI) *sqlp
Name: sqlparser.NewIdentifierCI("PRIMARY"),
Type: sqlparser.IndexTypePrimary,
},
Columns: []*sqlparser.IndexColumn{{Column: name}},
Columns: []*sqlparser.IndexColumn{{Column: name, Length: -1}},
}
return index
}
Expand Down Expand Up @@ -778,7 +773,7 @@ func (c *CreateTableEntity) normalizeForeignKeyIndexes() {
},
}
for _, col := range fk.Source {
indexColumn := &sqlparser.IndexColumn{Column: col}
indexColumn := &sqlparser.IndexColumn{Column: col, Length: -1}
indexDefinition.Columns = append(indexDefinition.Columns, indexColumn)
}
c.TableSpec.Indexes = append(c.TableSpec.Indexes, indexDefinition)
Expand Down
8 changes: 4 additions & 4 deletions go/vt/sqlparser/ast.go
Original file line number Diff line number Diff line change
Expand Up @@ -1815,10 +1815,10 @@ type ColumnType struct {
Options *ColumnTypeOptions

// Numeric field options
Length *Literal
Length int
Unsigned bool
Zerofill bool
Scale *Literal
Scale int

// Text field options
Charset ColumnCharset
Expand Down Expand Up @@ -3427,8 +3427,8 @@ func (ListArg) iColTuple() {}
// ConvertType represents the type in call to CONVERT(expr, type)
type ConvertType struct {
Type string
Length *Literal
Scale *Literal
Length int
Scale int
Charset ColumnCharset
}

Expand Down
5 changes: 0 additions & 5 deletions go/vt/sqlparser/ast_clone.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

24 changes: 0 additions & 24 deletions go/vt/sqlparser/ast_copy_on_rewrite.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

12 changes: 6 additions & 6 deletions go/vt/sqlparser/ast_equals.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

20 changes: 10 additions & 10 deletions go/vt/sqlparser/ast_format.go
Original file line number Diff line number Diff line change
Expand Up @@ -695,11 +695,11 @@ func (col *ColumnDefinition) Format(buf *TrackedBuffer) {
func (ct *ColumnType) Format(buf *TrackedBuffer) {
buf.astPrintf(ct, "%#s", ct.Type)

if ct.Length != nil && ct.Scale != nil {
buf.astPrintf(ct, "(%v,%v)", ct.Length, ct.Scale)
if ct.Length >= 0 && ct.Scale >= 0 {
buf.astPrintf(ct, "(%d,%d)", ct.Length, ct.Scale)

} else if ct.Length != nil {
buf.astPrintf(ct, "(%v)", ct.Length)
} else if ct.Length >= 0 {
buf.astPrintf(ct, "(%d)", ct.Length)
}

if ct.EnumValues != nil {
Expand Down Expand Up @@ -823,8 +823,8 @@ func (idx *IndexDefinition) Format(buf *TrackedBuffer) {
buf.astPrintf(idx, "(%v)", col.Expression)
} else {
buf.astPrintf(idx, "%v", col.Column)
if col.Length != nil {
buf.astPrintf(idx, "(%v)", col.Length)
if col.Length >= 0 {
buf.astPrintf(idx, "(%d)", col.Length)
}
}
if col.Direction == DescOrder {
Expand Down Expand Up @@ -1851,10 +1851,10 @@ func (node *ConvertUsingExpr) Format(buf *TrackedBuffer) {
// Format formats the node.
func (node *ConvertType) Format(buf *TrackedBuffer) {
buf.astPrintf(node, "%#s", node.Type)
if node.Length != nil {
buf.astPrintf(node, "(%v", node.Length)
if node.Scale != nil {
buf.astPrintf(node, ", %v", node.Scale)
if node.Length >= 0 {
buf.astPrintf(node, "(%d", node.Length)
if node.Scale >= 0 {
buf.astPrintf(node, ", %d", node.Scale)
}
buf.astPrintf(node, ")")
}
Expand Down
22 changes: 11 additions & 11 deletions go/vt/sqlparser/ast_format_fast.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 5 additions & 5 deletions go/vt/sqlparser/ast_funcs.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,16 +69,16 @@ type IndexColumn struct {
// Only one of Column or Expression can be specified
// Length is an optional field which is only applicable when Column is used
Column IdentifierCI
Length *Literal
Length int
Expression Expr
Direction OrderDirection
}

// LengthScaleOption is used for types that have an optional length
// and scale
type LengthScaleOption struct {
Length *Literal
Scale *Literal
Length int
Scale int
}

// IndexOption is used for trailing options for indexes: COMMENT, KEY_BLOCK_SIZE, USING, WITH PARSER
Expand Down Expand Up @@ -190,9 +190,9 @@ func (ts *TableSpec) AddConstraint(cd *ConstraintDefinition) {
func (ct *ColumnType) DescribeType() string {
buf := NewTrackedBuffer(nil)
buf.Myprintf("%s", ct.Type)
if ct.Length != nil && ct.Scale != nil {
if ct.Length >= 0 && ct.Scale >= 0 {
buf.Myprintf("(%v,%v)", ct.Length, ct.Scale)
} else if ct.Length != nil {
} else if ct.Length >= 0 {
buf.Myprintf("(%v)", ct.Length)
}

Expand Down
36 changes: 10 additions & 26 deletions go/vt/sqlparser/ast_rewrite.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading

0 comments on commit 9f6f007

Please sign in to comment.