SQL: Pull col-offset validation into helper

sql/scan.go

@@ -129,6 +129,7 @@ type scanNode struct {
 	isSecondaryIndex bool
 	reverse          bool
 	columns          []column
+	originalCols     []column // copy of `columns` before additions (e.g. by sort or group)
 	columnIDs        []ColumnID
 	ordering         []int
 	exactPrefix      int
@@ -389,6 +390,9 @@ func (n *scanNode) initTargets(targets parser.SelectExprs) error {
 			return n.err
 		}
 	}
+	// `groupBy` or `orderBy` may internally add additional columns which we
+	// do not want to include in validation of e.g. `GROUP BY 2`.
+	n.originalCols = n.columns
 	return nil
 }
 
@@ -516,6 +520,26 @@ func (n *scanNode) addRender(target parser.SelectExpr) error {
 	return nil
 }
 
+func (n *scanNode) colIndex(expr parser.Expr) (int, error) {
+	index := 0
+
+	switch i := expr.(type) {
+	case parser.DInt:
+		index = int(i)
+	case parser.Datum:
+		return -1, fmt.Errorf("non-integer constant column index: %s", expr)
+	default:
+		return -1, nil
+	}
+
+	if index < 1 || index > len(n.originalCols) {
+		return -1, fmt.Errorf("invalid column index: %d not in range [1, %d]",
+			index, len(n.originalCols))
+	}
+
+	return index - 1, nil
+}
+
 func (n *scanNode) processKV(kv client.KeyValue) bool {
 	if n.indexKey == nil {
 		// Reset the qvals map expressions to nil. The expressions will get filled

sql/sort.go

@@ -87,20 +87,10 @@ func (p *planner) orderBy(n *parser.Select, s *scanNode) (*sortNode, error) {
 		if index == 0 {
 			// The order by expression matched neither an output column nor an
 			// existing render target.
-			if datum, ok := expr.(parser.Datum); ok {
-				// If we evaluated to an int, use that as an index to order by. This
-				// handles cases like:
-				//
-				//   SELECT * FROM t ORDER BY 1
-				i, ok := datum.(parser.DInt)
-				if !ok {
-					return nil, fmt.Errorf("invalid ORDER BY: %s", expr)
-				}
-				index = int(i)
-				if index < 1 || index > len(columns) {
-					return nil, fmt.Errorf("invalid ORDER BY index: %d not in range [1, %d]",
-						index, len(columns))
-				}
+			if col, err := s.colIndex(expr); err != nil {
+				return nil, err
+			} else if col >= 0 {
+				index = col + 1
 			} else {
 				// Add a new render expression to use for ordering. This handles cases
 				// were the expression is either not a qualified name or is a qualified

sql/testdata/order_by

@@ -136,10 +136,10 @@ SELECT a FROM t ORDER BY (((a)))
 query error incompatible value types bool, int
 SELECT CASE a WHEN 1 THEN b ELSE c END as val FROM t ORDER BY val
 
-query error invalid ORDER BY index: 0 not in range \[1, 3\]
+query error invalid column index: 0 not in range \[1, 3\]
 SELECT * FROM t ORDER BY 0
 
-query error invalid ORDER BY: true
+query error non-integer constant column index: true
 SELECT * FROM t ORDER BY true
 
 query error qualified name "t.foo" not found