Implemented the tag values iterator for SHOW TAG VALUES

CHANGELOG.md

@@ -19,6 +19,7 @@
 - [#5691](https://github.com/influxdata/influxdb/pull/5691): Remove associated shard data when retention policies are dropped.
 - [#5758](https://github.com/influxdata/influxdb/pull/5758): TSM engine stats for cache, WAL, and filestore. Thanks @jonseymour
 - [#5844](https://github.com/influxdata/influxdb/pull/5844): Tag TSM engine stats with database and retention policy
+- [#5593](https://github.com/influxdata/influxdb/issues/5593): Modify `SHOW TAG VALUES` output for the new query engine to normalize the output.
 
 ### Bugfixes
 

cmd/influxd/run/server_test.go

@@ -4973,37 +4973,37 @@ func TestServer_Query_ShowTagKeys(t *testing.T) {
 		&Query{
 			name:    "show tag values with key",
 			command: "SHOW TAG VALUES WITH KEY = host",
-			exp:     `{"results":[{"series":[{"name":"hostTagValues","columns":["host"],"values":[["server01"],["server02"],["server03"]]}]}]}`,
+			exp:     `{"results":[{"series":[{"name":"cpu","columns":["key","value"],"values":[["host","server01"],["host","server02"]]},{"name":"disk","columns":["key","value"],"values":[["host","server03"]]},{"name":"gpu","columns":["key","value"],"values":[["host","server02"],["host","server03"]]}]}]}`,
 			params:  url.Values{"db": []string{"db0"}},
 		},
 		&Query{
 			name:    `show tag values with key and where`,
 			command: `SHOW TAG VALUES FROM cpu WITH KEY = host WHERE region = 'uswest'`,
-			exp:     `{"results":[{"series":[{"name":"hostTagValues","columns":["host"],"values":[["server01"]]}]}]}`,
+			exp:     `{"results":[{"series":[{"name":"cpu","columns":["key","value"],"values":[["host","server01"]]}]}]}`,
 			params:  url.Values{"db": []string{"db0"}},
 		},
 		&Query{
-			name:    `show tag values with key and where matches regular expression`,
+			name:    `show tag values with key and where matches the regular expression`,
 			command: `SHOW TAG VALUES WITH KEY = host WHERE region =~ /ca.*/`,
-			exp:     `{"results":[{"series":[{"name":"hostTagValues","columns":["host"],"values":[["server03"]]}]}]}`,
+			exp:     `{"results":[{"series":[{"name":"disk","columns":["key","value"],"values":[["host","server03"]]},{"name":"gpu","columns":["key","value"],"values":[["host","server03"]]}]}]}`,
 			params:  url.Values{"db": []string{"db0"}},
 		},
 		&Query{
-			name:    `show tag values with key and where does not matche regular expression`,
+			name:    `show tag values with key and where does not match the regular expression`,
 			command: `SHOW TAG VALUES WITH KEY = region WHERE host !~ /server0[12]/`,
-			exp:     `{"results":[{"series":[{"name":"regionTagValues","columns":["region"],"values":[["caeast"]]}]}]}`,
+			exp:     `{"results":[{"series":[{"name":"disk","columns":["key","value"],"values":[["region","caeast"]]}]}]}`,
 			params:  url.Values{"db": []string{"db0"}},
 		},
 		&Query{
-			name:    `show tag values with key in and where does not matche regular expression`,
+			name:    `show tag values with key in and where does not match the regular expression`,
 			command: `SHOW TAG VALUES FROM cpu WITH KEY IN (host, region) WHERE region = 'uswest'`,
-			exp:     `{"results":[{"series":[{"name":"hostTagValues","columns":["host"],"values":[["server01"]]},{"name":"regionTagValues","columns":["region"],"values":[["uswest"]]}]}]}`,
+			exp:     `{"results":[{"series":[{"name":"cpu","columns":["key","value"],"values":[["host","server01"],["region","uswest"]]}]}]}`,
 			params:  url.Values{"db": []string{"db0"}},
 		},
 		&Query{
 			name:    `show tag values with key and measurement matches regular expression`,
 			command: `SHOW TAG VALUES FROM /[cg]pu/ WITH KEY = host`,
-			exp:     `{"results":[{"series":[{"name":"hostTagValues","columns":["host"],"values":[["server01"],["server02"],["server03"]]}]}]}`,
+			exp:     `{"results":[{"series":[{"name":"cpu","columns":["key","value"],"values":[["host","server01"],["host","server02"]]},{"name":"gpu","columns":["key","value"],"values":[["host","server02"],["host","server03"]]}]}]}`,
 			params:  url.Values{"db": []string{"db0"}},
 		},
 		&Query{

influxql/ast.go

@@ -3440,6 +3440,36 @@ type rewriterFunc func(Node) Node
 
 func (fn rewriterFunc) Rewrite(n Node) Node { return fn(n) }
 
+// RewriteExpr recursively invokes the function to replace each expr.
+// Nodes are traversed depth-first and rewritten from leaf to root.
+func RewriteExpr(expr Expr, fn func(Expr) Expr) Expr {
+	switch e := expr.(type) {
+	case *BinaryExpr:
+		e.LHS = RewriteExpr(e.LHS, fn)
+		e.RHS = RewriteExpr(e.RHS, fn)
+		if e.LHS != nil && e.RHS == nil {
+			expr = e.LHS
+		} else if e.RHS != nil && e.LHS == nil {
+			expr = e.RHS
+		} else if e.LHS == nil && e.RHS == nil {
+			return nil
+		}
+
+	case *ParenExpr:
+		e.Expr = RewriteExpr(e.Expr, fn)
+		if e.Expr == nil {
+			return nil
+		}
+
+	case *Call:
+		for i, expr := range e.Args {
+			e.Args[i] = RewriteExpr(expr, fn)
+		}
+	}
+
+	return fn(expr)
+}
+
 // Eval evaluates expr against a map.
 func Eval(expr Expr, m map[string]interface{}) interface{} {
 	if expr == nil {

influxql/ast_test.go

@@ -861,6 +861,27 @@ func TestRewrite(t *testing.T) {
 	}
 }
 
+// Ensure an Expr can be rewritten handling nils.
+func TestRewriteExpr(t *testing.T) {
+	expr := MustParseExpr(`(time > 1 AND time < 10) OR foo = 2`)
+
+	// Remove all time expressions.
+	act := influxql.RewriteExpr(expr, func(e influxql.Expr) influxql.Expr {
+		switch e := e.(type) {
+		case *influxql.BinaryExpr:
+			if lhs, ok := e.LHS.(*influxql.VarRef); ok && lhs.Val == "time" {
+				return nil
+			}
+		}
+		return e
+	})
+
+	// Verify that everything is flipped.
+	if act := act.String(); act != `foo = 2.000` {
+		t.Fatalf("unexpected result: %s", act)
+	}
+}
+
 // Ensure that the String() value of a statement is parseable
 func TestParseString(t *testing.T) {
 	var tests = []struct {

influxql/statement_rewriter.go

@@ -1,8 +1,6 @@
 package influxql
 
-import (
-	"errors"
-)
+import "errors"
 
 // RewriteStatement rewrites stmt into a new statement, if applicable.
 func RewriteStatement(stmt Statement) (Statement, error) {
@@ -13,6 +11,8 @@ func RewriteStatement(stmt Statement) (Statement, error) {
 		return rewriteShowMeasurementsStatement(stmt)
 	case *ShowTagKeysStatement:
 		return rewriteShowTagKeysStatement(stmt)
+	case *ShowTagValuesStatement:
+		return rewriteShowTagValuesStatement(stmt)
 	default:
 		return stmt, nil
 	}
@@ -61,28 +61,8 @@ func rewriteShowMeasurementsStatement(stmt *ShowMeasurementsStatement) (Statemen
 	}
 
 	condition := stmt.Condition
-	if source, ok := stmt.Source.(*Measurement); ok {
-		var expr Expr
-		if source.Regex != nil {
-			expr = &BinaryExpr{
-				Op:  EQREGEX,
-				LHS: &VarRef{Val: "name"},
-				RHS: &RegexLiteral{Val: source.Regex.Val},
-			}
-		} else if source.Name != "" {
-			expr = &BinaryExpr{
-				Op:  EQ,
-				LHS: &VarRef{Val: "name"},
-				RHS: &StringLiteral{Val: source.Name},
-			}
-		}
-
-		// Set condition or "AND" together.
-		if condition == nil {
-			condition = expr
-		} else {
-			condition = &BinaryExpr{Op: AND, LHS: expr, RHS: condition}
-		}
+	if stmt.Source != nil {
+		condition = rewriteSourcesCondition(Sources([]Source{stmt.Source}), stmt.Condition)
 	}
 
 	return &SelectStatement{
@@ -107,39 +87,70 @@ func rewriteShowTagKeysStatement(stmt *ShowTagKeysStatement) (Statement, error)
 		return nil, errors.New("SHOW TAG KEYS doesn't support time in WHERE clause")
 	}
 
+	condition := rewriteSourcesCondition(stmt.Sources, stmt.Condition)
+	return &SelectStatement{
+		Fields: []*Field{
+			{Expr: &VarRef{Val: "tagKey"}},
+		},
+		Sources: []Source{
+			&Measurement{Name: "_tagKeys"},
+		},
+		Condition:  condition,
+		Offset:     stmt.Offset,
+		Limit:      stmt.Limit,
+		SortFields: stmt.SortFields,
+		OmitTime:   true,
+		Dedupe:     true,
+	}, nil
+}
+
+func rewriteShowTagValuesStatement(stmt *ShowTagValuesStatement) (Statement, error) {
+	// Check for time in WHERE clause (not supported).
+	if HasTimeExpr(stmt.Condition) {
+		return nil, errors.New("SHOW TAG VALUES doesn't support time in WHERE clause")
+	}
+
 	condition := stmt.Condition
-	if len(stmt.Sources) > 0 {
-		if source, ok := stmt.Sources[0].(*Measurement); ok {
-			var expr Expr
-			if source.Regex != nil {
-				expr = &BinaryExpr{
-					Op:  EQREGEX,
-					LHS: &VarRef{Val: "name"},
-					RHS: &RegexLiteral{Val: source.Regex.Val},
-				}
-			} else if source.Name != "" {
+	if len(stmt.TagKeys) > 0 {
+		var expr Expr
+		for _, tagKey := range stmt.TagKeys {
+			tagExpr := &BinaryExpr{
+				Op:  EQ,
+				LHS: &VarRef{Val: "_tagKey"},
+				RHS: &StringLiteral{Val: tagKey},
+			}
+
+			if expr != nil {
 				expr = &BinaryExpr{
-					Op:  EQ,
-					LHS: &VarRef{Val: "name"},
-					RHS: &StringLiteral{Val: source.Name},
+					Op:  OR,
+					LHS: expr,
+					RHS: tagExpr,
 				}
+			} else {
+				expr = tagExpr
 			}
+		}
 
-			// Set condition or "AND" together.
-			if condition == nil {
-				condition = expr
-			} else {
-				condition = &BinaryExpr{Op: AND, LHS: expr, RHS: condition}
+		// Set condition or "AND" together.
+		if condition == nil {
+			condition = expr
+		} else {
+			condition = &BinaryExpr{
+				Op:  AND,
+				LHS: &ParenExpr{Expr: condition},
+				RHS: &ParenExpr{Expr: expr},
 			}
 		}
 	}
+	condition = rewriteSourcesCondition(stmt.Sources, condition)
 
 	return &SelectStatement{
 		Fields: []*Field{
-			{Expr: &VarRef{Val: "tagKey"}},
+			{Expr: &VarRef{Val: "_tagKey"}, Alias: "key"},
+			{Expr: &VarRef{Val: "value"}},
 		},
 		Sources: []Source{
-			&Measurement{Name: "_tagKeys"},
+			&Measurement{Name: "_tags"},
 		},
 		Condition:  condition,
 		Offset:     stmt.Offset,
@@ -149,3 +160,52 @@ func rewriteShowTagKeysStatement(stmt *ShowTagKeysStatement) (Statement, error)
 		Dedupe:     true,
 	}, nil
 }
+
+// rewriteSourcesCondition rewrites sources into `name` expressions.
+// Merges with cond and returns a new condition.
+func rewriteSourcesCondition(sources Sources, cond Expr) Expr {
+	if len(sources) == 0 {
+		return cond
+	}
+
+	// Generate an OR'd set of filters on source name.
+	var scond Expr
+	for _, source := range sources {
+		mm := source.(*Measurement)
+
+		// Generate a filtering expression on the measurement name.
+		var expr Expr
+		if mm.Regex != nil {
+			expr = &BinaryExpr{
+				Op:  EQREGEX,
+				LHS: &VarRef{Val: "name"},
+				RHS: &RegexLiteral{Val: mm.Regex.Val},
+			}
+		} else if mm.Name != "" {
+			expr = &BinaryExpr{
+				Op:  EQ,
+				LHS: &VarRef{Val: "name"},
+				RHS: &StringLiteral{Val: mm.Name},
+			}
+		}
+
+		if scond == nil {
+			scond = expr
+		} else {
+			scond = &BinaryExpr{
+				Op:  OR,
+				LHS: scond,
+				RHS: expr,
+			}
+		}
+	}
+
+	if cond != nil {
+		return &BinaryExpr{
+			Op:  AND,
+			LHS: &ParenExpr{Expr: scond},
+			RHS: &ParenExpr{Expr: cond},
+		}
+	}
+	return scond
+}

influxql/statement_rewriter_test.go

@@ -41,7 +41,7 @@ func TestRewriteStatement(t *testing.T) {
 		},
 		{
 			stmt: `SHOW MEASUREMENTS WITH MEASUREMENT = cpu WHERE region = 'uswest'`,
-			s:    `SELECT "name" FROM _measurements WHERE "name" = 'cpu' AND region = 'uswest'`,
+			s:    `SELECT "name" FROM _measurements WHERE ("name" = 'cpu') AND (region = 'uswest')`,
 		},
 		{
 			stmt: `SHOW TAG KEYS`,
@@ -57,7 +57,19 @@ func TestRewriteStatement(t *testing.T) {
 		},
 		{
 			stmt: `SHOW TAG KEYS FROM cpu WHERE region = 'uswest'`,
-			s:    `SELECT tagKey FROM _tagKeys WHERE "name" = 'cpu' AND region = 'uswest'`,
+			s:    `SELECT tagKey FROM _tagKeys WHERE ("name" = 'cpu') AND (region = 'uswest')`,
+		},
+		{
+			stmt: `SHOW TAG VALUES WITH KEY = region`,
+			s:    `SELECT _tagKey AS "key", value FROM _tags WHERE _tagKey = 'region'`,
+		},
+		{
+			stmt: `SHOW TAG VALUES FROM cpu WITH KEY = region`,
+			s:    `SELECT _tagKey AS "key", value FROM _tags WHERE ("name" = 'cpu') AND (_tagKey = 'region')`,
+		},
+		{
+			stmt: `SHOW TAG VALUES FROM cpu WITH KEY IN (region, host)`,
+			s:    `SELECT _tagKey AS "key", value FROM _tags WHERE ("name" = 'cpu') AND (_tagKey = 'region' OR _tagKey = 'host')`,
 		},
 		{
 			stmt: `SELECT value FROM cpu`,