From 2909927ab0eaa2943905f5beab5bbb8d71ca18b8 Mon Sep 17 00:00:00 2001
From: jlapacik <joshua.lapacik@pacbell.net>
Date: Thu, 4 Apr 2019 11:13:09 -0700
Subject: [PATCH] fix(reads): HasFieldValueKey searches for "$", not "_value"
 (#13138)

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.
---
 storage/reads/predicate.go      |  7 +--
 storage/reads/predicate_test.go | 84 +++++++++++++++++++++++++++++++++
 2 files changed, 88 insertions(+), 3 deletions(-)

diff --git a/storage/reads/predicate.go b/storage/reads/predicate.go
index b7b42fc9ebd..f70d3cda971 100644
--- a/storage/reads/predicate.go
+++ b/storage/reads/predicate.go
@@ -18,6 +18,7 @@ const (
 	fieldKey       = "_field"
 	measurementKey = "_measurement"
 	valueKey       = "_value"
+	fieldRef       = "$"
 )
 
 // NodeVisitor can be called by Walk to traverse the Node hierarchy.
@@ -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:
@@ -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}
 				}
 			}
@@ -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]
 }
diff --git a/storage/reads/predicate_test.go b/storage/reads/predicate_test.go
index dcdeeaa9152..c300e663470 100644
--- a/storage/reads/predicate_test.go
+++ b/storage/reads/predicate_test.go
@@ -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)
+			}
+		})
+	}
+}