fix(storage): Ensure Tag(Keys|Values) APIs never return (nil, nil)

Formalized this post condition in the documentation and added additional
unit tests.

Added a nil guard and unit test to WriteStringIterator.

storage/engine_schema.go

@@ -10,6 +10,8 @@ import (
 
 // TagKeys returns an iterator where the values are tag keys for the bucket
 // matching the predicate within the time range (start, end].
+//
+// TagKeys will always return a StringIterator if there is no error.
 func (e *Engine) TagKeys(ctx context.Context, orgID, bucketID influxdb.ID, start, end int64, predicate influxql.Expr) (cursors.StringIterator, error) {
 	e.mu.RLock()
 	defer e.mu.RUnlock()
@@ -23,6 +25,8 @@ func (e *Engine) TagKeys(ctx context.Context, orgID, bucketID influxdb.ID, start
 // TagValues returns an iterator which enumerates the values for the specific
 // tagKey in the given bucket matching the predicate within the
 // time range (start, end].
+//
+// TagValues will always return a StringIterator if there is no error.
 func (e *Engine) TagValues(ctx context.Context, orgID, bucketID influxdb.ID, tagKey string, start, end int64, predicate influxql.Expr) (cursors.StringIterator, error) {
 	e.mu.RLock()
 	defer e.mu.RUnlock()

storage/reads/string_iterator_writer.go

@@ -38,6 +38,10 @@ func (w *StringIteratorWriter) WrittenN() int {
 }
 
 func (w *StringIteratorWriter) WriteStringIterator(si cursors.StringIterator) error {
+	if si == nil {
+		return nil
+	}
+
 	for si.Next() {
 		v := si.Value()
 		if v == "" {

storage/reads/string_iterator_writer_test.go

@@ -45,3 +45,12 @@ func TestStringIteratorWriter(t *testing.T) {
 		t.Errorf("expected %v got %v", expect, got)
 	}
 }
+
+func TestStringIteratorWriter_Nil(t *testing.T) {
+	w := reads.NewStringIteratorWriter(&mockStringValuesStream{})
+	err := w.WriteStringIterator(nil)
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	w.Flush()
+}

tsdb/tsm1/engine_schema.go

@@ -17,6 +17,8 @@ import (
 // TagValues returns an iterator which enumerates the values for the specific
 // tagKey in the given bucket matching the predicate within the
 // time range (start, end].
+//
+// TagValues will always return a StringIterator if there is no error.
 func (e *Engine) TagValues(ctx context.Context, orgID, bucketID influxdb.ID, tagKey string, start, end int64, predicate influxql.Expr) (cursors.StringIterator, error) {
 	encoded := tsdb.EncodeName(orgID, bucketID)
 
@@ -115,7 +117,7 @@ func (e *Engine) tagValuesPredicate(ctx context.Context, orgBucket, tagKeyBytes
 	}
 
 	if len(keys) == 0 {
-		return nil, nil
+		return cursors.EmptyStringIterator, nil
 	}
 
 	var files []TSMFile
@@ -226,6 +228,10 @@ func (e *Engine) findCandidateKeys(ctx context.Context, orgBucket []byte, predic
 	return keys, nil
 }
 
+// TagKeys returns an iterator which enumerates the tag keys for the given
+// bucket matching the predicate within the time range (start, end].
+//
+// TagKeys will always return a StringIterator if there is no error.
 func (e *Engine) TagKeys(ctx context.Context, orgID, bucketID influxdb.ID, start, end int64, predicate influxql.Expr) (cursors.StringIterator, error) {
 	encoded := tsdb.EncodeName(orgID, bucketID)
 
@@ -309,7 +315,7 @@ func (e *Engine) tagKeysPredicate(ctx context.Context, orgBucket []byte, start,
 	}
 
 	if len(keys) == 0 {
-		return nil, nil
+		return cursors.EmptyStringIterator, nil
 	}
 
 	var files []TSMFile

tsdb/tsm1/engine_schema_test.go

@@ -238,6 +238,23 @@ memB,host=EB,os=macOS value=1.3 201`)
 			exp:      []string{"0B", "AB", "BB", "CB", "DB", "EB"},
 			expStats: cursors.CursorStats{ScannedValues: 3, ScannedBytes: 24},
 		},
+
+		// ***********************
+		// * other scenarios
+		// ***********************
+		{
+			// ensure StringIterator is never nil
+			name: "predicate/no candidate series",
+			args: args{
+				org:  1,
+				key:  "host",
+				min:  0,
+				max:  1000,
+				expr: `foo = 'bar'`,
+			},
+			exp:      nil,
+			expStats: cursors.CursorStats{ScannedValues: 0, ScannedBytes: 0},
+		},
 	}
 	for _, tc := range tests {
 		t.Run(tc.name, func(t *testing.T) {
@@ -470,6 +487,22 @@ mem,mem1=v,mem2=v        f=1 201`)
 			exp:      []string{models.MeasurementTagKey, "cpu0", "cpu1", "cpu2", "cpu3", "cpu4", "cpu5", "mem0", "mem1", "mem2", models.FieldKeyTagKey},
 			expStats: cursors.CursorStats{ScannedValues: 2, ScannedBytes: 16},
 		},
+
+		// ***********************
+		// * other scenarios
+		// ***********************
+		{
+			// ensure StringIterator is never nil
+			name: "predicate/no candidate series",
+			args: args{
+				org:  0,
+				min:  0,
+				max:  300,
+				expr: "foo = 'bar'",
+			},
+			exp:      nil,
+			expStats: cursors.CursorStats{ScannedValues: 0, ScannedBytes: 0},
+		},
 	}
 	for _, tc := range tests {
 		t.Run(fmt.Sprintf("org%d/%s", tc.args.org, tc.name), func(t *testing.T) {