From d176c8babb278a83ee5d4d12a78c8e938ab9ae60 Mon Sep 17 00:00:00 2001 From: "Jonathan A. Sternberg" Date: Thu, 7 Apr 2016 10:52:32 -0400 Subject: [PATCH] Throw an error when an invalid expression is used with aux iterators The following query was fixed previously: SELECT 'value' FROM cpu This ended up hitting the `buildExprIterator()` code path and was handled properly. But this query: SELECT 'value', value FROM cpu This took a different code path that would trigger a panic because it triggered a panic instead of an error condition. This code path has now been modified to trigger an error instead of a panic. Fixes #6248. --- CHANGELOG.md | 1 + influxql/select.go | 31 +++++++++++++++++++------------ influxql/select_test.go | 4 ++++ 3 files changed, 24 insertions(+), 12 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index dc1d3d7e4fe..fdb44d1d944 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,7 @@ ### Bugfixes - [#6206](https://github.com/influxdata/influxdb/issues/6206): Handle nil values from the tsm1 cursor correctly. +- [#6248](https://github.com/influxdata/influxdb/issues/6248): Panic using incorrectly quoted "queries" field key. ## v0.12.0 [2016-04-05] ### Release Notes diff --git a/influxql/select.go b/influxql/select.go index 79b9881aac9..9a050833dd5 100644 --- a/influxql/select.go +++ b/influxql/select.go @@ -110,20 +110,27 @@ func buildAuxIterators(fields Fields, ic IteratorCreator, opt IteratorOptions) ( // Generate iterators for each field. itrs := make([]Iterator, len(fields)) - for i, f := range fields { - expr := Reduce(f.Expr, nil) - switch expr := expr.(type) { - case *VarRef: - itrs[i] = aitr.Iterator(expr.Val) - case *BinaryExpr: - itr, err := buildExprIterator(expr, aitr, opt) - if err != nil { - return nil, fmt.Errorf("error constructing iterator for field '%s': %s", f.String(), err) + if err := func() error { + for i, f := range fields { + expr := Reduce(f.Expr, nil) + switch expr := expr.(type) { + case *VarRef: + itrs[i] = aitr.Iterator(expr.Val) + case *BinaryExpr: + itr, err := buildExprIterator(expr, aitr, opt) + if err != nil { + return fmt.Errorf("error constructing iterator for field '%s': %s", f.String(), err) + } + itrs[i] = itr + default: + return fmt.Errorf("invalid expression type: %T", expr) } - itrs[i] = itr - default: - panic("unreachable") } + return nil + }(); err != nil { + Iterators(Iterators(itrs).filterNonNil()).Close() + aitr.Close() + return nil, err } // Background the primary iterator since there is no reader for it. diff --git a/influxql/select_test.go b/influxql/select_test.go index 18cfd020c6b..f8f21cc3ea4 100644 --- a/influxql/select_test.go +++ b/influxql/select_test.go @@ -2021,6 +2021,10 @@ func TestSelect_InvalidQueries(t *testing.T) { q: `SELECT 'value' FROM cpu`, err: `invalid expression type: *influxql.StringLiteral`, }, + { + q: `SELECT 'value', value FROM cpu`, + err: `invalid expression type: *influxql.StringLiteral`, + }, } for i, tt := range tests {