From ac1fa668e73244fcf9c0d663d08daed89db5fd76 Mon Sep 17 00:00:00 2001
From: Barak Michener <me@barakmich.com>
Date: Wed, 12 Aug 2015 14:50:57 -0400
Subject: [PATCH] Add check for multiple all iterators

Update test to reflect that the two Alls will be collapsed,
also fix the test of tag optimization to use fixed iterators.
---
 graph/iterator/and_iterator_optimize.go      |  9 +++++-
 graph/iterator/and_iterator_optimize_test.go | 45 ++++++++++++++++++++++++++--
 2 files changed, 51 insertions(+), 3 deletions(-)

diff --git a/graph/iterator/and_iterator_optimize.go b/graph/iterator/and_iterator_optimize.go
index 05dc85d..db841dd 100644
--- a/graph/iterator/and_iterator_optimize.go
+++ b/graph/iterator/and_iterator_optimize.go
@@ -292,12 +292,15 @@ func hasAnyNullIterators(its []graph.Iterator) bool {
 // nothing, and graph.All which returns everything. Particularly, we want
 // to see if we're intersecting with a bunch of graph.All iterators, and,
 // if we are, then we have only one useful iterator.
+//
+// We already checked for hasAnyNullIteratators() -- so now we're considering
+// All iterators.
 func hasOneUsefulIterator(its []graph.Iterator) graph.Iterator {
 	usefulCount := 0
 	var usefulIt graph.Iterator
 	for _, it := range its {
 		switch it.Type() {
-		case graph.Null, graph.All:
+		case graph.All:
 			continue
 		case graph.Optional:
 			// Optional is weird -- it's not useful, but we can't optimize
@@ -313,6 +316,10 @@ func hasOneUsefulIterator(its []graph.Iterator) graph.Iterator {
 	if usefulCount == 1 {
 		return usefulIt
 	}
+	if usefulCount == 0 {
+		// It's full of All iterators. We can safely return one of them.
+		return its[0]
+	}
 	return nil
 }
 
diff --git a/graph/iterator/and_iterator_optimize_test.go b/graph/iterator/and_iterator_optimize_test.go
index 13ede49..4bf5b8a 100644
--- a/graph/iterator/and_iterator_optimize_test.go
+++ b/graph/iterator/and_iterator_optimize_test.go
@@ -73,7 +73,7 @@ func TestNullIteratorAnd(t *testing.T) {
 	}
 }
 
-func TestReorderWithTag(t *testing.T) {
+func TestAllPromotion(t *testing.T) {
 	qs := &store{
 		data: []string{},
 		iter: NewFixed(Identity),
@@ -91,6 +91,43 @@ func TestReorderWithTag(t *testing.T) {
 	if !changed {
 		t.Error("Expected new iterator")
 	}
+	if newIt.Type() != graph.All {
+		t.Error("Should have promoted the All iterator")
+	}
+	expectedTags := []string{"good", "slow"}
+	tagsOut := make([]string, 0)
+	for _, x := range newIt.Tagger().Tags() {
+		tagsOut = append(tagsOut, x)
+	}
+	sort.Strings(tagsOut)
+	if !reflect.DeepEqual(expectedTags, tagsOut) {
+		t.Fatalf("Tags don't match: expected: %#v, got: %#v", expectedTags, tagsOut)
+	}
+}
+
+func TestReorderWithTag(t *testing.T) {
+	qs := &store{
+		data: []string{},
+		iter: NewFixed(Identity),
+	}
+	all := NewFixed(Identity)
+	all.Add(3)
+	all.Tagger().Add("good")
+	all2 := NewFixed(Identity)
+	all2.Tagger().Add("slow")
+	all2.Add(3)
+	all2.Add(4)
+	all2.Add(5)
+	all2.Add(6)
+	a := NewAnd(qs)
+	// Make all2 the default iterator
+	a.AddSubIterator(all2)
+	a.AddSubIterator(all)
+
+	newIt, changed := a.Optimize()
+	if !changed {
+		t.Error("Expected new iterator")
+	}
 	expectedTags := []string{"good", "slow"}
 	tagsOut := make([]string, 0)
 	for _, sub := range newIt.SubIterators() {
@@ -98,8 +135,12 @@ func TestReorderWithTag(t *testing.T) {
 			tagsOut = append(tagsOut, x)
 		}
 	}
+	for _, x := range newIt.Tagger().Tags() {
+		tagsOut = append(tagsOut, x)
+	}
+	sort.Strings(tagsOut)
 	if !reflect.DeepEqual(expectedTags, tagsOut) {
-		t.Fatal("Tags don't match")
+		t.Fatalf("Tags don't match: expected: %#v, got: %#v", expectedTags, tagsOut)
 	}
 }