Parser fix, only allow ORDER BY ASC and ORDER BY time ASC

CHANGELOG.md

@@ -34,6 +34,7 @@
 - [#3244](https://github.com/influxdb/influxdb/pull/3244): Wire up admin privilege grant and revoke.
 - [#3259](https://github.com/influxdb/influxdb/issues/3259): Respect privileges for queries.
 - [#3256](https://github.com/influxdb/influxdb/pull/3256): Remove unnecessary timeout in WaitForLeader(). Thanks @cannium.
+- [#3380](https://github.com/influxdb/influxdb/issue/3380): Parser fix, only allow ORDER BY ASC and ORDER BY time ASC.
 
 ## v0.9.1 [2015-07-02]
 

influxql/ast.go

@@ -274,9 +274,15 @@ type SortField struct {
 // String returns a string representation of a sort field
 func (field *SortField) String() string {
 	var buf bytes.Buffer
-	_, _ = buf.WriteString(field.Name)
-	_, _ = buf.WriteString(" ")
-	_, _ = buf.WriteString(strconv.FormatBool(field.Ascending))
+	if field.Name == "" {
+		_, _ = buf.WriteString(field.Name)
+		_, _ = buf.WriteString(" ")
+	}
+	if field.Ascending {
+		_, _ = buf.WriteString("ASC")
+	} else {
+		_, _ = buf.WriteString("DESC")
+	}
 	return buf.String()
 }
 

influxql/parser.go

@@ -1742,10 +1742,22 @@ func (p *Parser) parseOrderBy() (SortFields, error) {
 	return fields, nil
 }
 
-// parseSortFields parses all fields of and ORDER BY clause.
+// parseSortFields parses the sort fields for an ORDER BY clause.
 func (p *Parser) parseSortFields() (SortFields, error) {
 	var fields SortFields
 
+	// If first token is ASC or DESC, all fields are sorted.
+	if tok, pos, lit := p.scanIgnoreWhitespace(); tok == ASC || tok == DESC {
+		if tok == DESC {
+			// Token must be ASC, until other sort orders are supported.
+			return nil, errors.New("only ORDER BY time ASC supported at this time")
+		}
+		return append(fields, &SortField{Ascending: (tok == ASC)}), nil
+	} else if tok != IDENT {
+		return nil, newParseError(tokstr(tok, lit), []string{"identifier", "ASC", "DESC"}, pos)
+	}
+	p.unscan()
+
 	// At least one field is required.
 	field, err := p.parseSortField()
 	if err != nil {
@@ -1770,20 +1782,33 @@ func (p *Parser) parseSortFields() (SortFields, error) {
 		fields = append(fields, field)
 	}
 
+	// First SortField must be time ASC, until other sort orders are supported.
+	if len(fields) > 1 || fields[0].Name != "time" || !fields[0].Ascending {
+		return nil, errors.New("only ORDER BY time ASC supported at this time")
+	}
+
 	return fields, nil
 }
 
 // parseSortField parses one field of an ORDER BY clause.
 func (p *Parser) parseSortField() (*SortField, error) {
 	field := &SortField{}
 
-	// Next token must be ASC, until other sort orders are supported.
+	// Parse sort field name.
+	ident, err := p.parseIdent()
+	if err != nil {
+		return nil, err
+	}
+	field.Name = ident
+
+	// Check for optional ASC or DESC clause. Default is ASC.
 	tok, _, _ := p.scanIgnoreWhitespace()
-	if tok != ASC {
-		return nil, errors.New("only ORDER BY ASC supported at this time")
+	if tok != ASC && tok != DESC {
+		p.unscan()
+		tok = ASC
 	}
+	field.Ascending = (tok == ASC)
 
-	field.Ascending = true
 	return field, nil
 }
 

influxql/parser_test.go

@@ -1215,9 +1215,14 @@ func TestParser_ParseStatement(t *testing.T) {
 		{s: `SELECT field1 FROM myseries OFFSET`, err: `found EOF, expected number at line 1, char 36`},
 		{s: `SELECT field1 FROM myseries OFFSET 10.5`, err: `fractional parts not allowed in OFFSET at line 1, char 36`},
 		{s: `SELECT field1 FROM myseries ORDER`, err: `found EOF, expected BY at line 1, char 35`},
-		{s: `SELECT field1 FROM myseries ORDER BY /`, err: `only ORDER BY ASC supported at this time`},
-		{s: `SELECT field1 FROM myseries ORDER BY 1`, err: `only ORDER BY ASC supported at this time`},
-		{s: `SELECT field1 FROM myseries ORDER BY DESC`, err: `only ORDER BY ASC supported at this time`},
+		{s: `SELECT field1 FROM myseries ORDER BY`, err: `found EOF, expected identifier, ASC, DESC at line 1, char 38`},
+		{s: `SELECT field1 FROM myseries ORDER BY /`, err: `found /, expected identifier, ASC, DESC at line 1, char 38`},
+		{s: `SELECT field1 FROM myseries ORDER BY 1`, err: `found 1, expected identifier, ASC, DESC at line 1, char 38`},
+		{s: `SELECT field1 FROM myseries ORDER BY time ASC,`, err: `found EOF, expected identifier at line 1, char 47`},
+		{s: `SELECT field1 FROM myseries ORDER BY DESC`, err: `only ORDER BY time ASC supported at this time`},
+		{s: `SELECT field1 FROM myseries ORDER BY field1`, err: `only ORDER BY time ASC supported at this time`},
+		{s: `SELECT field1 FROM myseries ORDER BY time DESC`, err: `only ORDER BY time ASC supported at this time`},
+		{s: `SELECT field1 FROM myseries ORDER BY time, field1`, err: `only ORDER BY time ASC supported at this time`},
 		{s: `SELECT field1 AS`, err: `found EOF, expected identifier at line 1, char 18`},
 		{s: `SELECT field1 FROM foo group by time(1s)`, err: `GROUP BY requires at least one aggregate function`},
 		{s: `SELECT count(value) FROM foo group by time(1s)`, err: `aggregate functions with GROUP BY time require a WHERE time clause`},