Allow numerical GROUP/ORDER clauses

src/query/expr/parser_test.go

@@ -545,6 +545,15 @@ func TestParsingSQL(t *testing.T) {
 		{"SELECT foo FROM bar GROUP BY foo ORDER BY foo ASC NULLS LAST, bar DESC NULLS FIRST", nil},
 		{"SELECT foo FROM bar GROUP BY foo ORDER BY foo ASC NULLS LAST, bar DESC NULLS FIRST LIMIT 3", nil},
 
+		// GROUP BY number
+		{"SELECT foo FROM bar GROUP BY 1", nil},
+		{"SELECT foo, baz FROM bar GROUP BY 1, 2", nil},
+		// ARCH: all those NULLS LAST/FIRST are due to roundtrip testing
+		{"SELECT foo, baz FROM bar ORDER BY 1 ASC NULLS LAST", nil},
+		{"SELECT foo, baz FROM bar ORDER BY 1 ASC NULLS LAST, 2 ASC NULLS LAST", nil},
+		{"SELECT foo, baz FROM bar ORDER BY 1 DESC NULLS FIRST, 2 ASC NULLS LAST", nil},
+		{"SELECT foo, baz FROM bar ORDER BY 1 ASC NULLS LAST, 2 DESC NULLS LAST", nil},
+
 		{"SELECT foo FROM bar@234", errInvalidQuery},
 		{"SELECT foo FROM bar GROUP for 1", errInvalidQuery},
 		{"SELECT foo FROM bar GROUP BY foo LIMIT foo", errInvalidQuery},

src/query/expr/types.go

@@ -81,6 +81,12 @@ type Integer struct {
 	value int64
 }
 
+// TODO/ARCH: doing this to avoid too many changes across the codebase, consider
+// exporting the field
+func (ex *Integer) Value() int64 {
+	return ex.value
+}
+
 func (ex *Integer) ReturnType(ts column.TableSchema) (column.Schema, error) {
 	return column.Schema{
 		Name:     ex.String(),

src/query/query.go

@@ -18,6 +18,7 @@ var errInvalidLimitValue = errors.New("invalid limit value")
 var errInvalidProjectionInAggregation = errors.New("selections in aggregating expressions need to be either the group by clauses or aggregating expressions (e.g. sum(foo))")
 var errInvalidFilter = errors.New("invalid WHERE clause")
 var errInvalidOrderClause = errors.New("invalid ORDER BY clause")
+var errInvalidGroupbyClause = errors.New("invalid GROUP BY clause")
 var errQueryNoDatasetIdentifiers = errors.New("query without a dataset has identifiers in the SELECT clause")
 
 // Result holds the result of a query, at this point it's fairly literal - in the future we may want
@@ -386,6 +387,13 @@ func reorder(res *Result, q expr.Query) error {
 			nullsFirst = oby.NullsFirst
 			needle = oby.Children()[0]
 		}
+		// TODO/ARCH: I wanted to change q.Order in place... but we can't create a new Ordering,
+		// because `.inner` is private and I didn't want to expose it. But it might be the right way to go
+		// TODO: test this properly (we have parsing tests, not implementation testing)
+		if idx, ok := needle.(*expr.Integer); ok {
+			needle = q.Select[idx.Value()-1] // no need to validate any more, already did that
+		}
+
 		pos := lookupExpr(needle, q.Select)
 		if pos == -1 {
 			return fmt.Errorf("cannot sort by a column not in projections: %s", needle)
@@ -506,6 +514,15 @@ func Run(db *database.Database, q expr.Query) (*Result, error) {
 				proj = wrapped.Children()[0]
 			}
 
+			// ORDER BY 1, 2
+			if idx, ok := proj.(*expr.Integer); ok {
+				n := idx.Value()
+				if n < 1 || n > int64(len(q.Select)) {
+					return nil, errInvalidOrderClause
+				}
+				continue
+			}
+
 			posS := lookupExpr(proj, q.Select)
 			posG := -1
 			if q.Aggregate != nil {
@@ -528,6 +545,17 @@ func Run(db *database.Database, q expr.Query) (*Result, error) {
 		limit = *q.Limit
 	}
 	if q.Aggregate != nil || allAggregations {
+		// edit GROUP BY 1, 2 in place (replace them by their respective columns)
+		for j, agg := range q.Aggregate {
+			if idx, ok := agg.(*expr.Integer); ok {
+				n := idx.Value()
+				if n < 1 || n > int64(len(q.Select)) {
+					return nil, errInvalidGroupbyClause
+				}
+				q.Aggregate[j] = q.Select[n-1]
+			}
+		}
+
 		if err := aggregate(db, ds, res, q); err != nil {
 			return nil, err
 		}

src/query/query_test.go

@@ -384,6 +384,13 @@ func TestQuerySetup(t *testing.T) {
 		{"SELECT bar FROM dataset GROUP BY foo", errInvalidProjectionInAggregation},
 		{"SELECT foo FROM dataset GROUP BY nullif(foo, 2)", errInvalidProjectionInAggregation},
 		{"SELECT foo FROM dataset ORDER by FOO", nil},
+		{"SELECT foo FROM dataset ORDER by 1", nil},
+		{"SELECT foo, bar FROM dataset ORDER by 1, 2", nil},
+		{"SELECT * FROM dataset ORDER by 1, 2", nil},
+		{"SELECT * FROM dataset ORDER by 1, bar", nil},
+		{"SELECT foo, bar FROM dataset ORDER by 0, 1", errInvalidOrderClause}, // underflow
+		{"SELECT foo, bar FROM dataset ORDER by 2, 3", errInvalidOrderClause}, // overflow
+		{"SELECT * FROM dataset ORDER by 1, 2, 4", errInvalidOrderClause},     // overflow
 		{"SELECT foo FROM dataset ORDER by bar", errInvalidOrderClause},
 		{"SELECT foo FROM dataset ORDER by foo, bar", errInvalidOrderClause},
 		{"SELECT foo FROM dataset LIMIT 0", nil}, // cannot test -2, because that fails with a parser error
@@ -401,6 +408,13 @@ func TestQuerySetup(t *testing.T) {
 		// we get a parser issue, because we can get multiple where clauses only in JSON unmarshaling of queries
 		// {"SELECT foo FROM dataset WHERE foo > 0, foo < 3", errInvalidFilter},
 
+		{"SELECT * FROM dataset GROUP BY 1, 2, 3", nil},
+		{"SELECT foo, bar FROM dataset GROUP BY 1, 2", nil},
+		{"SELECT foo, bar FROM dataset GROUP BY 1, bar", nil},
+		{"SELECT foo, bar FROM dataset GROUP BY 0, 1", errInvalidGroupbyClause}, // underflow
+		{"SELECT foo, bar FROM dataset GROUP BY 1, 3", errInvalidGroupbyClause}, // overflow
+		{"SELECT * FROM dataset GROUP BY 1, 4", errInvalidGroupbyClause},        // overflow
+
 		// relabeling can be tricky, especially when looking up columns across parts of the query - these are all legal
 		// BUT this doesn't work the same for WHERE clauses - we cannot filter on relabeled fields
 		{"SELECT foo AS bar FROM dataset GROUP BY foo", nil},