storage: optimize queries for label presence

The query term "key>" can be used to search for records that have any value for the key. This optimizes the query with a separate SQL expression and adds tests for this behavior. Change-Id: I5e2734734e0911a3ed12c87e7ada776ad309a90d Reviewed-on: https://go-review.googlesource.com/36590 Reviewed-by: Russ Cox <[email protected]>
golang · Feb 9, 2017 · 07bde3b · 07bde3b
1 parent c133759
commit 07bde3b
Show file tree

Hide file tree

Showing 2 changed files with 25 additions and 4 deletions.
diff --git a/storage/db/db.go b/storage/db/db.go
@@ -377,6 +377,10 @@ func parseQueryPart(part string) (sql string, args []interface{}, err error) {
 		}
 		return "SELECT UploadID, RecordID FROM RecordLabels WHERE Name = ? AND Value = ?", []interface{}{key, value}, nil
 	case '>', '<':
+		if sep == '>' && value == "" {
+			// Simplify queries for any value.
+			return "SELECT UploadID, RecordID FROM RecordLabels WHERE Name = ?", []interface{}{key}, nil
+		}
 		return fmt.Sprintf("SELECT UploadID, RecordID FROM RecordLabels WHERE Name = ? AND Value %c ?", sep), []interface{}{key, value}, nil
 	}
 	return "", nil, fmt.Errorf("query part %q has invalid key", part)

diff --git a/storage/db/db_test.go b/storage/db/db_test.go
@@ -12,6 +12,7 @@ import (
 	"os"
 	"os/exec"
 	"reflect"
+	"sort"
 	"strconv"
 	"strings"
 	"testing"
@@ -273,8 +274,10 @@ func TestQuery(t *testing.T) {
 		{"label0:0 label0:5", []int{}},
 		{"bogus query", nil},
 		{"label1<2 label3:0", []int{0, 1, 2, 3}},
-		{"label1>510", []int{1022, 1023}},
+		{"label1>510 label1<52", []int{1022, 1023}},
 		{"", allRecords},
+		{"missing>", []int{}},
+		{"label0>", allRecords},
 	}
 	for _, test := range tests {
 		t.Run("query="+test.q, func(t *testing.T) {
@@ -293,21 +296,35 @@ func TestQuery(t *testing.T) {
 					t.Errorf("Close: %v", err)
 				}
 			}()
-			for i, num := range test.want {
+			var have []int
+			for i := range test.want {
 				if !q.Next() {
 					t.Fatalf("#%d: Next() = false", i)
 				}
 				r := q.Result()
-				if r.Labels["label0"] != fmt.Sprintf("%d", num) {
-					t.Errorf("result[%d].label0 = %q, want %d", i, r.Labels["label0"], num)
+				n, err := strconv.Atoi(r.Labels["label0"])
+				if err != nil {
+					t.Fatalf("unexpected label0 value %q: %v", r.Labels["label0"], err)
 				}
+				have = append(have, n)
 				if r.NameLabels["name"] != "Name" {
 					t.Errorf("result[%d].name = %q, want %q", i, r.NameLabels["name"], "Name")
 				}
 			}
+			for q.Next() {
+				r := q.Result()
+				t.Errorf("Next() = true, want false (got labels %v)", r.Labels)
+			}
 			if err := q.Err(); err != nil {
 				t.Errorf("Err() = %v, want nil", err)
 			}
+			sort.Ints(have)
+			if len(have) == 0 {
+				have = []int{}
+			}
+			if !reflect.DeepEqual(have, test.want) {
+				t.Errorf("label0[] = %v, want %v", have, test.want)
+			}
 		})
 	}
 }