From a35d9602cda002e84ad32927f2ba0736659a3e98 Mon Sep 17 00:00:00 2001 From: "Jonathan A. Sternberg" Date: Tue, 15 Mar 2016 15:33:52 -0400 Subject: [PATCH] Fix where filters when a OR is used and when a tag does not exist If an OR was used, merging filters between different expressions would not work correctly. If one of the sides had a set of series ids with a condition and the other side had no series ids associated with the expression, all of the series from the side with a condition would have the condition ignored. Instead of defaulting a non-existant series filter to true, it should just be false and the evaluation of the one side that does exist should take care of determining if the series id should be included or not. The AND condition used false correctly so did not have to be changed. If a tag did not exist and `!=` or `!~` were used, it would return false even though the neither a field or a tag equaled those values. This has now been modified to correctly return the correct series ids and the correct condition. Also fixed a panic that would occur when a tag caused a field access to become unnecessary. The filter using the field access still got created and used even though it was unnecessary, resulting in an attempted access to a non-initialized map. Fixes #5152 and a bunch of other miscellaneous issues. --- CHANGELOG.md | 2 ++ cmd/influxd/run/server_test.go | 18 ++++++++++++++++++ tsdb/engine/tsm1/engine.go | 25 ++++++++++++++----------- tsdb/meta.go | 27 +++++++++++++++------------ 4 files changed, 49 insertions(+), 23 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index fecfaef2ad..ea0935b904 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,8 @@ ### Bugfixes +- [#5152](https://github.com/influxdata/influxdb/issues/5152): Fix where filters when a tag and a filter are combined with OR. + ## v0.11.0 [unreleased] ### Release Notes diff --git a/cmd/influxd/run/server_test.go b/cmd/influxd/run/server_test.go index c041d5ce26..a5c2fcf4cb 100644 --- a/cmd/influxd/run/server_test.go +++ b/cmd/influxd/run/server_test.go @@ -4219,6 +4219,24 @@ func TestServer_Query_Where_With_Tags(t *testing.T) { command: `select foo from where_events where (tennant = 'paul' OR tennant = 'david') AND time > 1s AND (foo = 'bar' OR foo = 'baz' OR foo = 'bap')`, exp: `{"results":[{"series":[{"name":"where_events","columns":["time","foo"],"values":[["2009-11-10T23:00:02Z","bar"],["2009-11-10T23:00:03Z","baz"],["2009-11-10T23:00:06Z","bap"]]}]}]}`, }, + &Query{ + name: "tag or field", + params: url.Values{"db": []string{"db0"}}, + command: `select foo from where_events where tennant = 'paul' OR foo = 'bar'`, + exp: `{"results":[{"series":[{"name":"where_events","columns":["time","foo"],"values":[["2009-11-10T23:00:02Z","bar"],["2009-11-10T23:00:03Z","baz"],["2009-11-10T23:00:04Z","bat"],["2009-11-10T23:00:05Z","bar"]]}]}]}`, + }, + &Query{ + name: "non-existant tag and field", + params: url.Values{"db": []string{"db0"}}, + command: `select foo from where_events where tenant != 'paul' AND foo = 'bar'`, + exp: `{"results":[{"series":[{"name":"where_events","columns":["time","foo"],"values":[["2009-11-10T23:00:02Z","bar"],["2009-11-10T23:00:05Z","bar"]]}]}]}`, + }, + &Query{ + name: "non-existant tag or field", + params: url.Values{"db": []string{"db0"}}, + command: `select foo from where_events where tenant != 'paul' OR foo = 'bar'`, + exp: `{"results":[{"series":[{"name":"where_events","columns":["time","foo"],"values":[["2009-11-10T23:00:02Z","bar"],["2009-11-10T23:00:03Z","baz"],["2009-11-10T23:00:04Z","bat"],["2009-11-10T23:00:05Z","bar"],["2009-11-10T23:00:06Z","bap"]]}]}]}`, + }, &Query{ name: "where on tag that should be double quoted but isn't", params: url.Values{"db": []string{"db0"}}, diff --git a/tsdb/engine/tsm1/engine.go b/tsdb/engine/tsm1/engine.go index 349cdd8d5d..82228b95ee 100644 --- a/tsdb/engine/tsm1/engine.go +++ b/tsdb/engine/tsm1/engine.go @@ -778,8 +778,8 @@ func (e *Engine) createVarRefIterator(opt influxql.IteratorOptions) ([]influxql. if err := func() error { mms := tsdb.Measurements(e.index.MeasurementsByName(influxql.Sources(opt.Sources).Names())) - // Retrieve non-time names from condition (includes tags). - conditionNames := influxql.ExprNames(opt.Condition) + // Retrieve the maximum number of fields (without time). + conditionFields := make([]string, len(influxql.ExprNames(opt.Condition))) for _, mm := range mms { // Determine tagsets for this measurement based on dimensions and filters. @@ -791,17 +791,20 @@ func (e *Engine) createVarRefIterator(opt influxql.IteratorOptions) ([]influxql. // Calculate tag sets and apply SLIMIT/SOFFSET. tagSets = influxql.LimitTagSets(tagSets, opt.SLimit, opt.SOffset) - // Filter the names from condition to only fields from the measurement. - conditionFields := make([]string, 0, len(conditionNames)) - for _, f := range conditionNames { - if mm.HasField(f) { - conditionFields = append(conditionFields, f) - } - } - for _, t := range tagSets { for i, seriesKey := range t.SeriesKeys { - itr, err := e.createVarRefSeriesIterator(ref, mm, seriesKey, t, t.Filters[i], conditionFields, opt) + fields := 0 + if t.Filters[i] != nil { + // Retrieve non-time fields from this series filter and filter out tags. + for _, f := range influxql.ExprNames(t.Filters[i]) { + if mm.HasField(f) { + conditionFields[fields] = f + fields++ + } + } + } + + itr, err := e.createVarRefSeriesIterator(ref, mm, seriesKey, t, t.Filters[i], conditionFields[:fields], opt) if err != nil { return err } else if itr == nil { diff --git a/tsdb/meta.go b/tsdb/meta.go index 0077c945b8..0796007938 100644 --- a/tsdb/meta.go +++ b/tsdb/meta.go @@ -666,11 +666,11 @@ func mergeSeriesFilters(op influxql.Token, ids SeriesIDs, lfilters, rfilters Fil // +==========+==========+==========+=======================+=======================+ // | operator | LHS | RHS | intermediate expr | reduced filter | // +==========+==========+==========+=======================+=======================+ - // | | | | true OR | true | + // | | | | false OR | | // | |----------+----------+-----------------------+-----------------------+ - // | OR | | | OR true | true | + // | OR | | | OR false | | // | |----------+----------+-----------------------+-----------------------+ - // | | | | true OR true | true | + // | | | | false OR false | false | // | |----------+----------+-----------------------+-----------------------+ // | | | | OR | OR | // +----------+----------+----------+-----------------------+-----------------------+ @@ -684,21 +684,16 @@ func mergeSeriesFilters(op influxql.Token, ids SeriesIDs, lfilters, rfilters Fil // +----------+----------+----------+-----------------------+-----------------------+ // *literal false filters and series IDs should be excluded from the results - def := false - if op == influxql.OR { - def = true - } - for _, id := range ids { // Get LHS and RHS filter expressions for this series ID. lfilter, rfilter := lfilters[id], rfilters[id] - // Set default filters if either LHS or RHS expressions were nil. + // Set filter to false if either LHS or RHS expressions were nil. if lfilter == nil { - lfilter = &influxql.BooleanLiteral{Val: def} + lfilter = &influxql.BooleanLiteral{Val: false} } if rfilter == nil { - rfilter = &influxql.BooleanLiteral{Val: def} + rfilter = &influxql.BooleanLiteral{Val: false} } // Create the intermediate filter expression for this series ID. @@ -717,7 +712,9 @@ func mergeSeriesFilters(op influxql.Token, ids SeriesIDs, lfilters, rfilters Fil } // Store the series ID and merged filter in the final results. - filters[id] = expr + if expr != nil { + filters[id] = expr + } series = append(series, id) } return series, filters @@ -749,6 +746,9 @@ func (m *Measurement) idsForExpr(n *influxql.BinaryExpr) (SeriesIDs, influxql.Ex tagVals, ok := m.seriesByTagKeyValue[name.Val] if name.Val != "_name" && !ok { + if n.Op == influxql.NEQ || n.Op == influxql.NEQREGEX { + return m.seriesIDs, &influxql.BooleanLiteral{Val: true}, nil + } return nil, nil, nil } @@ -804,6 +804,9 @@ func (m *Measurement) idsForExpr(n *influxql.BinaryExpr) (SeriesIDs, influxql.Ex return ids, &influxql.BooleanLiteral{Val: true}, nil } + if n.Op == influxql.NEQ || n.Op == influxql.NEQREGEX { + return m.seriesIDs, &influxql.BooleanLiteral{Val: true}, nil + } return nil, nil, nil }