Skip to content
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 select star, col1 orderby col1 bug. #7743

Merged
merged 10 commits into from
Apr 8, 2021
6 changes: 3 additions & 3 deletions go/vt/vtgate/engine/cached_size.go

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

19 changes: 13 additions & 6 deletions go/vt/vtgate/engine/comparer.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,16 +23,22 @@ import (

// comparer is the struct that has the logic for comparing two rows in the result set
type comparer struct {
orderBy, weightString int
desc bool
orderBy, weightString, starColFixedIndex int
systay marked this conversation as resolved.
Show resolved Hide resolved
desc bool
}

// compare compares two rows given the comparer and returns which one should be earlier in the result set
// -1 if the first row should be earlier
// 1 is the second row should be earlier
// 0 if both the rows have equal ordering
func (c *comparer) compare(r1, r2 []sqltypes.Value) (int, error) {
cmp, err := evalengine.NullsafeCompare(r1[c.orderBy], r2[c.orderBy])
var colIndex int
if c.starColFixedIndex > c.orderBy && c.starColFixedIndex < len(r1) {
systay marked this conversation as resolved.
Show resolved Hide resolved
colIndex = c.starColFixedIndex
} else {
colIndex = c.orderBy
}
cmp, err := evalengine.NullsafeCompare(r1[colIndex], r2[colIndex])
if err != nil {
_, isComparisonErr := err.(evalengine.UnsupportedComparisonError)
if !(isComparisonErr && c.weightString != -1) {
Expand All @@ -58,9 +64,10 @@ func extractSlices(input []OrderbyParams) []*comparer {
var result []*comparer
for _, order := range input {
result = append(result, &comparer{
orderBy: order.Col,
weightString: order.WeightStringCol,
desc: order.Desc,
orderBy: order.Col,
weightString: order.WeightStringCol,
desc: order.Desc,
starColFixedIndex: order.StarColFixedIndex,
})
}
return result
Expand Down
8 changes: 6 additions & 2 deletions go/vt/vtgate/engine/route.go
Original file line number Diff line number Diff line change
Expand Up @@ -128,12 +128,16 @@ type OrderbyParams struct {
Col int
// WeightStringCol is the weight_string column that will be used for sorting.
// It is set to -1 if such a column is not added to the query
WeightStringCol int
Desc bool
WeightStringCol int
Desc bool
StarColFixedIndex int
}

func (obp OrderbyParams) String() string {
val := strconv.Itoa(obp.Col)
if obp.StarColFixedIndex > obp.Col {
val = strconv.Itoa(obp.StarColFixedIndex)
}
if obp.Desc {
val += " DESC"
} else {
Expand Down
8 changes: 5 additions & 3 deletions go/vt/vtgate/planbuilder/memory_sort.go
Original file line number Diff line number Diff line change
Expand Up @@ -82,10 +82,12 @@ func newMemorySort(plan logicalPlan, orderBy sqlparser.OrderBy) (*memorySort, er
if colNumber == -1 {
return nil, fmt.Errorf("unsupported: memory sort: order by must reference a column in the select list: %s", sqlparser.String(order))
}

ob := engine.OrderbyParams{
Col: colNumber,
WeightStringCol: -1,
Desc: order.Direction == sqlparser.DescOrder,
Col: colNumber,
WeightStringCol: -1,
Desc: order.Direction == sqlparser.DescOrder,
StarColFixedIndex: colNumber,
}
ms.eMemorySort.OrderBy = append(ms.eMemorySort.OrderBy, ob)
}
Expand Down
31 changes: 28 additions & 3 deletions go/vt/vtgate/planbuilder/ordering.go
Original file line number Diff line number Diff line change
Expand Up @@ -289,10 +289,35 @@ func planRouteOrdering(orderBy sqlparser.OrderBy, node *route) (logicalPlan, err
if colNumber == -1 {
return nil, fmt.Errorf("unsupported: in scatter query: order by must reference a column in the select list: %s", sqlparser.String(order))
}
starColFixedIndex := colNumber
if selectStatement, ok := node.Select.(*sqlparser.Select); ok {
for i, selectExpr := range selectStatement.SelectExprs {
if starExpr, ok := selectExpr.(*sqlparser.StarExpr); ok {
if i < colNumber {
tableName := starExpr.TableName
tableMap := node.resultColumns[i].column.st.tables
var tableMeta *table
if tableName.IsEmpty() && len(tableMap) == 1 {
for j := range tableMap {
tableMeta = tableMap[j]
}
} else {
tableMeta = tableMap[tableName]
}
if tableMeta == nil {
break
}
starColFixedIndex += len(tableMeta.columnNames) - 1
systay marked this conversation as resolved.
Show resolved Hide resolved
}
}
}
}

ob := engine.OrderbyParams{
Col: colNumber,
WeightStringCol: -1,
Desc: order.Direction == sqlparser.DescOrder,
Col: colNumber,
WeightStringCol: -1,
Desc: order.Direction == sqlparser.DescOrder,
StarColFixedIndex: starColFixedIndex,
}
node.eroute.OrderBy = append(node.eroute.OrderBy, ob)

Expand Down
76 changes: 76 additions & 0 deletions go/vt/vtgate/planbuilder/testdata/postprocess_cases.txt
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,82 @@ Gen4 plan same as above
}
}

# ORDER BY on select t.*
"select t.*, t.col from user t order by t.col"
{
"QueryType": "SELECT",
"Original": "select t.*, t.col from user t order by t.col",
"Instructions": {
"OperatorType": "Route",
"Variant": "SelectScatter",
"Keyspace": {
"Name": "user",
"Sharded": true
},
"FieldQuery": "select t.*, t.col, weight_string(t.col) from `user` as t where 1 != 1",
"OrderBy": "9 ASC",
"Query": "select t.*, t.col, weight_string(t.col) from `user` as t order by t.col asc",
"Table": "`user`"
}
}

# ORDER BY on select *
"select *, col from user order by col"
{
"QueryType": "SELECT",
"Original": "select *, col from user order by col",
"Instructions": {
"OperatorType": "Route",
"Variant": "SelectScatter",
"Keyspace": {
"Name": "user",
"Sharded": true
},
"FieldQuery": "select *, col, weight_string(col) from `user` where 1 != 1",
"OrderBy": "9 ASC",
"Query": "select *, col, weight_string(col) from `user` order by col asc",
"Table": "`user`"
}
}

# ORDER BY on select multi t.*
"select t.*, t.name, t.*, t.col from user t order by t.col"
{
"QueryType": "SELECT",
"Original": "select t.*, t.name, t.*, t.col from user t order by t.col",
"Instructions": {
"OperatorType": "Route",
"Variant": "SelectScatter",
"Keyspace": {
"Name": "user",
"Sharded": true
},
"FieldQuery": "select t.*, t.`name`, t.*, t.col, weight_string(t.col) from `user` as t where 1 != 1",
"OrderBy": "19 ASC",
"Query": "select t.*, t.`name`, t.*, t.col, weight_string(t.col) from `user` as t order by t.col asc",
"Table": "`user`"
}
}

# ORDER BY on select multi *
"select *, name, *, col from user order by col"
{
"QueryType": "SELECT",
"Original": "select *, name, *, col from user order by col",
"Instructions": {
"OperatorType": "Route",
"Variant": "SelectScatter",
"Keyspace": {
"Name": "user",
"Sharded": true
},
"FieldQuery": "select *, `name`, *, col, weight_string(col) from `user` where 1 != 1",
"OrderBy": "19 ASC",
"Query": "select *, `name`, *, col, weight_string(col) from `user` order by col asc",
"Table": "`user`"
}
}

# ORDER BY works for select * from authoritative table
"select * from authoritative order by user_id"
{
Expand Down