Skip to content

Commit

Permalink
Fix where filters when a OR is used and when a tag does not exist
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
jsternberg committed Mar 15, 2016
1 parent c5bd96d commit e38b326
Show file tree
Hide file tree
Showing 4 changed files with 46 additions and 20 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@

## v0.11.0 [unreleased]

- [#5152](https://github.com/influxdata/influxdb/issues/5152): Fix where filters when a tag and a filter are combined with OR.

### Features

- [#5596](https://github.com/influxdata/influxdb/pull/5596): Build improvements for ARM architectures. Also removed `--goarm` and `--pkgarch` build flags.
Expand Down
18 changes: 18 additions & 0 deletions cmd/influxd/run/server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4273,6 +4273,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"}},
Expand Down
25 changes: 14 additions & 11 deletions tsdb/engine/tsm1/engine.go
Original file line number Diff line number Diff line change
Expand Up @@ -769,8 +769,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.
Expand All @@ -782,17 +782,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 {
Expand Down
21 changes: 12 additions & 9 deletions tsdb/meta.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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
Expand Down Expand Up @@ -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
}

Expand Down Expand Up @@ -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
}

Expand Down

0 comments on commit e38b326

Please sign in to comment.