Repository: vitessio/vitess
Pull request #7743 (2021-03-25 10:10:02)

fix select star, col1 orderby col1 bug.
shard env.
fix select star, col1 orderby col1 bug.
The problem is col1 parse index is 1.  But before this column, there have a star(*), this will be some column.
so finally orderby from multi shards, use order index 1 will be make a bug.
This is a company inner fix, base on Vitess 6.0.

So I not really sure, Vitess9.0 whether is fix this bug or doesn't support this typical sql.

```
select t.*, t.customer_id from customer t where email = 'aaa11' order by customer_id desc limit 0, 3;
```

systay: Hi @yangxuanjia!

Thank you so much for sharing this fix with us.
We need two things to make progress on this PR. First, we need all commits to have a signoff signature to be acceptable. 

1. You will need to amend your commits with `git commit --amend --signoff` so they can be legally accepted. Let me know if this is an issue and I can guide you through it.

2. We need tests to be added, showing what you are trying to accomplish. I think adding something to the `postprocess.txt` could work.

systay: Oh, and I see that you will also need to do a `make sizegen` to update the `CachedSize` methods

Patch:
diff --git a/go/vt/vtgate/engine/cached_size.go b/go/vt/vtgate/engine/cached_size.go
index 07139186e2..ea4cdfb9f2 100644
--- a/go/vt/vtgate/engine/cached_size.go
+++ b/go/vt/vtgate/engine/cached_size.go
@@ -307,7 +307,7 @@ func (cached *MemorySort) CachedSize(alloc bool) int64 {
 	size += cached.UpperLimit.CachedSize(false)
 	// field OrderBy []vitess.io/vitess/go/vt/vtgate/engine.OrderbyParams
 	{
-		size += int64(cap(cached.OrderBy)) * int64(17)
+		size += int64(cap(cached.OrderBy)) * int64(32)
 	}
 	// field Input vitess.io/vitess/go/vt/vtgate/engine.Primitive
 	if cc, ok := cached.Input.(cachedObject); ok {
@@ -334,7 +334,7 @@ func (cached *MergeSort) CachedSize(alloc bool) int64 {
 	}
 	// field OrderBy []vitess.io/vitess/go/vt/vtgate/engine.OrderbyParams
 	{
-		size += int64(cap(cached.OrderBy)) * int64(17)
+		size += int64(cap(cached.OrderBy)) * int64(32)
 	}
 	return size
 }
@@ -550,7 +550,7 @@ func (cached *Route) CachedSize(alloc bool) int64 {
 	}
 	// field OrderBy []vitess.io/vitess/go/vt/vtgate/engine.OrderbyParams
 	{
-		size += int64(cap(cached.OrderBy)) * int64(17)
+		size += int64(cap(cached.OrderBy)) * int64(32)
 	}
 	// field SysTableTableSchema vitess.io/vitess/go/vt/vtgate/evalengine.Expr
 	if cc, ok := cached.SysTableTableSchema.(cachedObject); ok {
diff --git a/go/vt/vtgate/engine/comparer.go b/go/vt/vtgate/engine/comparer.go
index a685c4816a..90b4e62f22 100644
--- a/go/vt/vtgate/engine/comparer.go
+++ b/go/vt/vtgate/engine/comparer.go
@@ -23,8 +23,8 @@ 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
+	desc                                     bool
 }
 
 // compare compares two rows given the comparer and returns which one should be earlier in the result set
@@ -32,7 +32,13 @@ type comparer struct {
 // 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) {
+		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) {
@@ -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
diff --git a/go/vt/vtgate/engine/route.go b/go/vt/vtgate/engine/route.go
index b77d8c115b..904afce420 100644
--- a/go/vt/vtgate/engine/route.go
+++ b/go/vt/vtgate/engine/route.go
@@ -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 {
diff --git a/go/vt/vtgate/planbuilder/memory_sort.go b/go/vt/vtgate/planbuilder/memory_sort.go
index 1f96ed47d5..fe741c631c 100644
--- a/go/vt/vtgate/planbuilder/memory_sort.go
+++ b/go/vt/vtgate/planbuilder/memory_sort.go
@@ -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)
 	}
diff --git a/go/vt/vtgate/planbuilder/ordering.go b/go/vt/vtgate/planbuilder/ordering.go
index c8c5273909..8350a5ed80 100644
--- a/go/vt/vtgate/planbuilder/ordering.go
+++ b/go/vt/vtgate/planbuilder/ordering.go
@@ -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
+					}
+				}
+			}
+		}
+
 		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)
 


Test patch:
diff --git a/go/vt/vtgate/planbuilder/testdata/postprocess_cases.txt b/go/vt/vtgate/planbuilder/testdata/postprocess_cases.txt
index a8e9497f8e..b1bc45b720 100644
--- a/go/vt/vtgate/planbuilder/testdata/postprocess_cases.txt
+++ b/go/vt/vtgate/planbuilder/testdata/postprocess_cases.txt
@@ -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"
 {