diff --git a/CHANGELOG.md b/CHANGELOG.md index a8cb58ac80e..90ff08814e8 100644 --- a/CHANGELOG.md +++ b/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] diff --git a/influxql/ast.go b/influxql/ast.go index 36803d5b238..c1ec1e34636 100644 --- a/influxql/ast.go +++ b/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() } diff --git a/influxql/parser.go b/influxql/parser.go index b6a93db41a2..6a8e34913dc 100644 --- a/influxql/parser.go +++ b/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,6 +1782,11 @@ 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 } @@ -1777,13 +1794,21 @@ func (p *Parser) parseSortFields() (SortFields, error) { 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 } diff --git a/influxql/parser_test.go b/influxql/parser_test.go index 6d79f1de625..2e1e9ffd79c 100644 --- a/influxql/parser_test.go +++ b/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`},