Skip to content

Commit

Permalink
fix(reads): HasFieldValueKey searches for "$", not "_value" (#13138)
Browse files Browse the repository at this point in the history
Storage converts references to _value in filter predicates to $.
It then considers any predicate that does not reference $ to be
a tag predicate. Tag predicates are used to construct series index
cursors.

This commit fixes a bug where field comparisons were being included
in tag predicates due to the HasFieldValueKey function searching
for comparison expressions referencing _value instead of $. Because
all references to _value had already been replaced. An expression
of the form '$ < 3000' would be considered a tag expression and
therefore would be mistakenly included as a tag predicate.

Fixes #13159.
  • Loading branch information
jlapacik authored Apr 4, 2019
1 parent c0bf96c commit 2909927
Show file tree
Hide file tree
Showing 2 changed files with 88 additions and 3 deletions.
7 changes: 4 additions & 3 deletions storage/reads/predicate.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ const (
fieldKey = "_field"
measurementKey = "_measurement"
valueKey = "_value"
fieldRef = "$"
)

// NodeVisitor can be called by Walk to traverse the Node hierarchy.
Expand Down Expand Up @@ -454,7 +455,7 @@ func (v *nodeToExprVisitor) Visit(n *datatypes.Node) NodeVisitor {
return nil

case datatypes.NodeTypeFieldRef:
v.exprs = append(v.exprs, &influxql.VarRef{Val: "$"})
v.exprs = append(v.exprs, &influxql.VarRef{Val: fieldRef})
return nil

case datatypes.NodeTypeLiteral:
Expand Down Expand Up @@ -533,7 +534,7 @@ func RewriteExprRemoveFieldValue(expr influxql.Expr) influxql.Expr {
return influxql.RewriteExpr(expr, func(expr influxql.Expr) influxql.Expr {
if be, ok := expr.(*influxql.BinaryExpr); ok {
if ref, ok := be.LHS.(*influxql.VarRef); ok {
if ref.Val == "$" {
if ref.Val == fieldRef {
return &influxql.BooleanLiteral{Val: true}
}
}
Expand Down Expand Up @@ -576,7 +577,7 @@ func (v *hasRefs) Visit(node influxql.Node) influxql.Visitor {
}

func HasFieldValueKey(expr influxql.Expr) bool {
refs := hasRefs{refs: []string{valueKey}, found: make([]bool, 1)}
refs := hasRefs{refs: []string{fieldRef}, found: make([]bool, 1)}
influxql.Walk(&refs, expr)
return refs.found[0]
}
84 changes: 84 additions & 0 deletions storage/reads/predicate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,3 +56,87 @@ func TestPredicateToExprString(t *testing.T) {
})
}
}

func TestHasFieldValueKey(t *testing.T) {
predicates := []*datatypes.Node{
{
NodeType: datatypes.NodeTypeComparisonExpression,
Value: &datatypes.Node_Comparison_{
Comparison: datatypes.ComparisonLess,
},
Children: []*datatypes.Node{
{
NodeType: datatypes.NodeTypeFieldRef,
Value: &datatypes.Node_FieldRefValue{
FieldRefValue: "_value",
},
},
{
NodeType: datatypes.NodeTypeLiteral,
Value: &datatypes.Node_IntegerValue{
IntegerValue: 3000,
},
},
},
},
{
NodeType: datatypes.NodeTypeLogicalExpression,
Value: &datatypes.Node_Logical_{
Logical: datatypes.LogicalAnd,
},
Children: []*datatypes.Node{
{
NodeType: datatypes.NodeTypeComparisonExpression,
Value: &datatypes.Node_Comparison_{
Comparison: datatypes.ComparisonEqual,
},
Children: []*datatypes.Node{
{
NodeType: datatypes.NodeTypeTagRef,
Value: &datatypes.Node_TagRefValue{
TagRefValue: "_measurement",
},
},
{
NodeType: datatypes.NodeTypeLiteral,
Value: &datatypes.Node_StringValue{
StringValue: "cpu",
},
},
},
},
{
NodeType: datatypes.NodeTypeComparisonExpression,
Value: &datatypes.Node_Comparison_{
Comparison: datatypes.ComparisonLess,
},
Children: []*datatypes.Node{
{
NodeType: datatypes.NodeTypeFieldRef,
Value: &datatypes.Node_FieldRefValue{
FieldRefValue: "_value",
},
},
{
NodeType: datatypes.NodeTypeLiteral,
Value: &datatypes.Node_IntegerValue{
IntegerValue: 3000,
},
},
},
},
},
},
}
for _, predicate := range predicates {
t.Run("", func(t *testing.T) {
expr, err := reads.NodeToExpr(predicate, nil)
if err != nil {
t.Fatalf("unexpected error converting predicate to InfluxQL expression: %v", err)
}
if !reads.HasFieldValueKey(expr) {
t.Fatalf("did not find a field reference in %v", expr)
}
})
}
}

0 comments on commit 2909927

Please sign in to comment.