Skip to content

Commit

Permalink
sqlparser: use integers instead of literals for Length/Precision (#15256
Browse files Browse the repository at this point in the history
)

Signed-off-by: Vicent Marti <vmg@strn.cat>
  • Loading branch information
vmg authored Feb 16, 2024
1 parent f82fb7c commit b539ce9
Show file tree
Hide file tree
Showing 37 changed files with 1,921 additions and 1,958 deletions.
31 changes: 31 additions & 0 deletions go/ptr/ptr.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
/*
Copyright 2024 The Vitess Authors.
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package ptr

// Of returns a pointer to the given value
func Of[T any](x T) *T {
return &x
}

// Unwrap dereferences the given pointer if it's not nil.
// Otherwise, it returns default_
func Unwrap[T any](x *T, default_ T) T {
if x != nil {
return *x
}
return default_
}
21 changes: 21 additions & 0 deletions go/test/endtoend/vtgate/queries/aggregation/aggregation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -557,6 +557,27 @@ func TestComplexAggregation(t *testing.T) {
})
}

func TestJoinAggregation(t *testing.T) {
// This is new functionality in Vitess 20
utils.SkipIfBinaryIsBelowVersion(t, 20, "vtgate")

mcmp, closer := start(t)
defer closer()

mcmp.Exec("insert into t1(t1_id, `name`, `value`, shardkey) values(1,'a1','foo',100), (2,'b1','foo',200), (3,'c1','foo',300), (4,'a1','foo',100), (5,'d1','toto',200), (6,'c1','tata',893), (7,'a1','titi',2380), (8,'b1','tete',12833), (9,'e1','yoyo',783493)")

mcmp.Exec(`insert into bet_logs(id, merchant_game_id, bet_amount, game_id) values
(1, 1, 22.5, 40), (2, 1, 15.3, 40),
(3, 2, 22.5, 40), (4, 2, 15.3, 40),
(5, 3, 22.5, 40), (6, 3, 15.3, 40),
(7, 3, 22.5, 40), (8, 4, 15.3, 40)
`)

mcmp.Exec("set @@sql_mode = ' '")
mcmp.Exec(`SELECT t1.name, SUM(b.bet_amount) AS bet_amount FROM bet_logs as b LEFT JOIN t1 ON b.merchant_game_id = t1.t1_id GROUP BY b.merchant_game_id`)
mcmp.Exec(`SELECT t1.name, CAST(SUM(b.bet_amount) AS DECIMAL(20,6)) AS bet_amount FROM bet_logs as b LEFT JOIN t1 ON b.merchant_game_id = t1.t1_id GROUP BY b.merchant_game_id`)
}

// TestGroupConcatAggregation tests the group_concat function with vitess doing the aggregation.
func TestGroupConcatAggregation(t *testing.T) {
mcmp, closer := start(t)
Expand Down
10 changes: 9 additions & 1 deletion go/test/endtoend/vtgate/queries/aggregation/schema.sql
Original file line number Diff line number Diff line change
Expand Up @@ -96,4 +96,12 @@ CREATE TABLE dept (
loc VARCHAR(13),
PRIMARY KEY (deptno)
) Engine = InnoDB
COLLATE = utf8mb4_general_ci;
COLLATE = utf8mb4_general_ci;

CREATE TABLE bet_logs (
id bigint unsigned NOT NULL,
merchant_game_id bigint unsigned NOT NULL,
bet_amount DECIMAL(20, 8),
game_id bigint,
PRIMARY KEY (id)
) ENGINE InnoDB;
8 changes: 8 additions & 0 deletions go/test/endtoend/vtgate/queries/aggregation/vschema.json
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,14 @@
"name": "hash"
}
]
},
"bet_logs": {
"column_vindexes": [
{
"column": "id",
"name": "hash"
}
]
}
}
}
26 changes: 11 additions & 15 deletions go/vt/schemadiff/table.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import (

golcs "github.com/yudai/golcs"

"vitess.io/vitess/go/ptr"
"vitess.io/vitess/go/vt/sqlparser"
)

Expand Down Expand Up @@ -530,10 +531,7 @@ func (c *CreateTableEntity) normalizeColumnOptions() {
// "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 = ptr.Of(1)

if col.Type.Options.Default != nil {
val, ok := col.Type.Options.Default.(sqlparser.BoolVal)
Expand Down Expand Up @@ -562,16 +560,14 @@ 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 != nil && col.Type.Scale == 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 *col.Type.Length <= 24 {
col.Type.Type = "float"
} else {
col.Type.Type = "double"
}
col.Type.Length = nil
}
Expand Down Expand Up @@ -627,7 +623,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 != nil && *colType.Length == 1
}

func (c *CreateTableEntity) normalizePartitionOptions() {
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
28 changes: 14 additions & 14 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.

32 changes: 16 additions & 16 deletions go/vt/sqlparser/ast_equals.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_format.go
Original file line number Diff line number Diff line change
Expand Up @@ -696,10 +696,10 @@ 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)
buf.astPrintf(ct, "(%d,%d)", *ct.Length, *ct.Scale)

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

if ct.EnumValues != nil {
Expand Down Expand Up @@ -824,7 +824,7 @@ func (idx *IndexDefinition) Format(buf *TrackedBuffer) {
} else {
buf.astPrintf(idx, "%v", col.Column)
if col.Length != nil {
buf.astPrintf(idx, "(%v)", col.Length)
buf.astPrintf(idx, "(%d)", *col.Length)
}
}
if col.Direction == DescOrder {
Expand Down Expand Up @@ -1852,9 +1852,9 @@ func (node *ConvertUsingExpr) Format(buf *TrackedBuffer) {
func (node *ConvertType) Format(buf *TrackedBuffer) {
buf.astPrintf(node, "%#s", node.Type)
if node.Length != nil {
buf.astPrintf(node, "(%v", node.Length)
buf.astPrintf(node, "(%d", *node.Length)
if node.Scale != nil {
buf.astPrintf(node, ", %v", node.Scale)
buf.astPrintf(node, ", %d", *node.Scale)
}
buf.astPrintf(node, ")")
}
Expand Down
Loading

0 comments on commit b539ce9

Please sign in to comment.