Changed logic of iterator termination (#330)

MontFerret · Jul 13, 2019 · 9756f0d · 9756f0d
1 parent b1e9505
commit 9756f0d
Show file tree

Hide file tree

Showing 21 changed files with 87 additions and 86 deletions.
diff --git a/Makefile b/Makefile
@@ -23,7 +23,7 @@ compile:
 	./main.go
 
 test:
-	go test -race -v ${DIR_PKG}/...
+	go test -race ${DIR_PKG}/...
 
 cover:
 	go test -race -coverprofile=coverage.txt -covermode=atomic ${DIR_PKG}/... && \

diff --git a/pkg/runtime/collections/collection.go b/pkg/runtime/collections/collection.go
@@ -46,11 +46,6 @@ func (iterator *coreIterator) Next(ctx context.Context, scope *core.Scope) (*cor
 		return nil, err
 	}
 
-	// end of iteration
-	if val == values.None {
-		return nil, nil
-	}
-
 	nextScope := scope.Fork()
 
 	if err := nextScope.SetVariable(iterator.valVar, val); err != nil {

diff --git a/pkg/runtime/collections/filter.go b/pkg/runtime/collections/filter.go
@@ -35,11 +35,6 @@ func (iterator *FilterIterator) Next(ctx context.Context, scope *core.Scope) (*c
 			return nil, err
 		}
 
-		if nextScope == nil {
-			return nil, nil
-		}
-
-		// TODO: test case when predicate return not nil
 		take, err := iterator.predicate(ctx, nextScope)
 
 		if err != nil {

diff --git a/pkg/runtime/collections/filter_test.go b/pkg/runtime/collections/filter_test.go
@@ -161,7 +161,7 @@ func TestFilter(t *testing.T) {
 		item, err := iter.Next(context.Background(), scope)
 
 		So(item, ShouldBeNil)
-		So(err, ShouldBeNil)
+		So(err, ShouldEqual, core.ErrNoMoreData)
 	})
 
 	Convey("Should iterate over nested filter", t, func() {

diff --git a/pkg/runtime/collections/indexed.go b/pkg/runtime/collections/indexed.go
@@ -62,5 +62,5 @@ func (iterator *IndexedIterator) Next(_ context.Context, scope *core.Scope) (*co
 		return nextScope, nil
 	}
 
-	return nil, nil
+	return nil, core.ErrNoMoreData
 }
diff --git a/pkg/runtime/collections/indexed_test.go b/pkg/runtime/collections/indexed_test.go
@@ -37,10 +37,12 @@ func TestArrayIterator(t *testing.T) {
 		for {
 			nextScope, err := iter.Next(ctx, scope.Fork())
 
-			So(err, ShouldBeNil)
+			if err != nil {
+				if core.IsNoMoreData(err) {
+					break
+				}
 
-			if nextScope == nil {
-				break
+				So(err, ShouldBeNil)
 			}
 
 			res = append(res, nextScope.MustGetVariable(collections.DefaultValueVar))
@@ -70,10 +72,12 @@ func TestArrayIterator(t *testing.T) {
 		for {
 			nextScope, err := iter.Next(ctx, scope.Fork())
 
-			So(err, ShouldBeNil)
+			if err != nil {
+				if core.IsNoMoreData(err) {
+					break
+				}
 
-			if nextScope == nil {
-				break
+				So(err, ShouldBeNil)
 			}
 
 			res = append(res, nextScope.MustGetVariable(collections.DefaultValueVar))
@@ -107,10 +111,12 @@ func TestArrayIterator(t *testing.T) {
 		for {
 			nextScope, err := iter.Next(ctx, scope.Fork())
 
-			So(err, ShouldBeNil)
+			if err != nil {
+				if core.IsNoMoreData(err) {
+					break
+				}
 
-			if nextScope == nil {
-				break
+				So(err, ShouldBeNil)
 			}
 
 			res = append(res, nextScope.MustGetVariable(collections.DefaultValueVar))
@@ -119,7 +125,7 @@ func TestArrayIterator(t *testing.T) {
 		item, err := iter.Next(ctx, scope)
 
 		So(item, ShouldBeNil)
-		So(err, ShouldBeNil)
+		So(err, ShouldEqual, core.ErrNoMoreData)
 		So(res, ShouldHaveLength, int(arr.Length()))
 	})
 
@@ -133,7 +139,7 @@ func TestArrayIterator(t *testing.T) {
 
 		nextScope, err := iter.Next(ctx, scope)
 
-		So(err, ShouldBeNil)
+		So(err, ShouldEqual, core.ErrNoMoreData)
 		So(nextScope, ShouldBeNil)
 	})
 }
diff --git a/pkg/runtime/collections/iterator.go b/pkg/runtime/collections/iterator.go
@@ -22,11 +22,11 @@ func ToSlice(ctx context.Context, scope *core.Scope, iterator Iterator) ([]*core
 		nextScope, err := iterator.Next(ctx, scope.Fork())
 
 		if err != nil {
-			return nil, err
-		}
+			if core.IsNoMoreData(err) {
+				return res, nil
+			}
 
-		if nextScope == nil {
-			return res, nil
+			return nil, err
 		}
 
 		res = append(res, nextScope)

diff --git a/pkg/runtime/collections/keyed.go b/pkg/runtime/collections/keyed.go
@@ -60,5 +60,5 @@ func (iterator *KeyedIterator) Next(_ context.Context, scope *core.Scope) (*core
 		return nextScope, nil
 	}
 
-	return nil, nil
+	return nil, core.ErrNoMoreData
 }
diff --git a/pkg/runtime/collections/keyed_test.go b/pkg/runtime/collections/keyed_test.go
@@ -35,10 +35,12 @@ func TestObjectIterator(t *testing.T) {
 		for {
 			nextScope, err := iter.Next(ctx, scope)
 
-			So(err, ShouldBeNil)
+			if err != nil {
+				if core.IsNoMoreData(err) {
+					break
+				}
 
-			if nextScope == nil {
-				break
+				So(err, ShouldBeNil)
 			}
 
 			key := nextScope.MustGetVariable(collections.DefaultKeyVar)
@@ -77,7 +79,7 @@ func TestObjectIterator(t *testing.T) {
 		nextScope, err := iter.Next(ctx, scope)
 
 		So(nextScope, ShouldBeNil)
-		So(err, ShouldBeNil)
+		So(err, ShouldEqual, core.ErrNoMoreData)
 	})
 
 	Convey("Should NOT iterate over a empty map", t, func() {
@@ -91,6 +93,6 @@ func TestObjectIterator(t *testing.T) {
 		nextScope, err := iter.Next(ctx, scope)
 
 		So(nextScope, ShouldBeNil)
-		So(err, ShouldBeNil)
+		So(err, ShouldEqual, core.ErrNoMoreData)
 	})
 }
diff --git a/pkg/runtime/collections/limit.go b/pkg/runtime/collections/limit.go
@@ -31,7 +31,7 @@ func (iterator *LimitIterator) Next(ctx context.Context, scope *core.Scope) (*co
 		return iterator.values.Next(ctx, scope)
 	}
 
-	return nil, nil
+	return nil, core.ErrNoMoreData
 }
 
 func (iterator *LimitIterator) counter() int {
@@ -44,15 +44,14 @@ func (iterator *LimitIterator) verifyOffset(ctx context.Context, scope *core.Sco
 	}
 
 	for iterator.offset > iterator.currCount {
-		nextScope, err := iterator.values.Next(ctx, scope.Fork())
+		_, err := iterator.values.Next(ctx, scope.Fork())
 
 		if err != nil {
-			return err
-		}
+			if core.IsNoMoreData(err) {
+				iterator.currCount = iterator.offset
+			}
 
-		if nextScope == nil {
-			iterator.currCount = iterator.offset
-			return nil
+			return err
 		}
 
 		iterator.currCount++

diff --git a/pkg/runtime/collections/map.go b/pkg/runtime/collections/map.go
@@ -69,5 +69,5 @@ func (iterator *MapIterator) Next(_ context.Context, scope *core.Scope) (*core.S
 		return nextScope, nil
 	}
 
-	return nil, nil
+	return nil, core.ErrNoMoreData
 }
diff --git a/pkg/runtime/collections/map_test.go b/pkg/runtime/collections/map_test.go
@@ -34,10 +34,12 @@ func TestMapIterator(t *testing.T) {
 		for {
 			nextScope, err := iter.Next(ctx, scope)
 
-			So(err, ShouldBeNil)
+			if err != nil {
+				if core.IsNoMoreData(err) {
+					break
+				}
 
-			if nextScope == nil {
-				break
+				So(err, ShouldBeNil)
 			}
 
 			key := nextScope.MustGetVariable(collections.DefaultKeyVar)
@@ -73,7 +75,7 @@ func TestMapIterator(t *testing.T) {
 		item, err := iter.Next(ctx, scope)
 
 		So(item, ShouldBeNil)
-		So(err, ShouldBeNil)
+		So(err, ShouldEqual, core.ErrNoMoreData)
 	})
 
 	Convey("Should NOT iterate over a empty map", t, func() {
@@ -87,6 +89,6 @@ func TestMapIterator(t *testing.T) {
 		item, err := iter.Next(ctx, scope)
 
 		So(item, ShouldBeNil)
-		So(err, ShouldBeNil)
+		So(err, ShouldEqual, core.ErrNoMoreData)
 	})
 }
diff --git a/pkg/runtime/collections/slice.go b/pkg/runtime/collections/slice.go
@@ -51,5 +51,5 @@ func (iterator *SliceIterator) Next(_ context.Context, scope *core.Scope) (*core
 		return nextScope, nil
 	}
 
-	return nil, nil
+	return nil, core.ErrNoMoreData
 }
diff --git a/pkg/runtime/collections/slice_test.go b/pkg/runtime/collections/slice_test.go
@@ -36,10 +36,12 @@ func TestSliceIterator(t *testing.T) {
 		for {
 			nextScope, err := iter.Next(ctx, scope)
 
-			So(err, ShouldBeNil)
+			if err != nil {
+				if core.IsNoMoreData(err) {
+					break
+				}
 
-			if nextScope == nil {
-				break
+				So(err, ShouldBeNil)
 			}
 
 			key := nextScope.MustGetVariable(collections.DefaultKeyVar)
@@ -101,7 +103,7 @@ func TestSliceIterator(t *testing.T) {
 		item, err := iter.Next(ctx, scope)
 
 		So(item, ShouldBeNil)
-		So(err, ShouldBeNil)
+		So(err, ShouldEqual, core.ErrNoMoreData)
 	})
 
 	Convey("Should NOT iterate over an empty slice", t, func() {
@@ -114,6 +116,6 @@ func TestSliceIterator(t *testing.T) {
 		item, err := iter.Next(ctx, scope)
 
 		So(item, ShouldBeNil)
-		So(err, ShouldBeNil)
+		So(err, ShouldEqual, core.ErrNoMoreData)
 	})
 }
diff --git a/pkg/runtime/collections/sort.go b/pkg/runtime/collections/sort.go
@@ -104,7 +104,7 @@ func (iterator *SortIterator) Next(ctx context.Context, scope *core.Scope) (*cor
 		return val, nil
 	}
 
-	return nil, nil
+	return nil, core.ErrNoMoreData
 }
 
 func (iterator *SortIterator) sort(ctx context.Context, scope *core.Scope) ([]*core.Scope, error) {

diff --git a/pkg/runtime/collections/tap.go b/pkg/runtime/collections/tap.go
@@ -29,10 +29,6 @@ func (iterator *TapIterator) Next(ctx context.Context, scope *core.Scope) (*core
 		return nil, err
 	}
 
-	if nextScope == nil {
-		return nil, nil
-	}
-
 	_, err = iterator.predicate.Exec(ctx, nextScope)
 
 	if err != nil {

diff --git a/pkg/runtime/collections/unique.go b/pkg/runtime/collections/unique.go
@@ -33,10 +33,6 @@ func (iterator *UniqueIterator) Next(ctx context.Context, scope *core.Scope) (*c
 			return nil, err
 		}
 
-		if nextScope == nil {
-			return nil, nil
-		}
-
 		v, err := nextScope.GetVariable(iterator.hashKey)
 
 		if err != nil {

diff --git a/pkg/runtime/core/errors.go b/pkg/runtime/core/errors.go
@@ -20,6 +20,7 @@ var (
 	ErrTimeout               = errors.New("operation timed out")
 	ErrNotImplemented        = errors.New("not implemented")
 	ErrNotSupported          = errors.New("not supported")
+	ErrNoMoreData            = errors.New("no more data")
 )
 
 const typeErrorTemplate = "expected %s, but got %s"
@@ -65,3 +66,7 @@ func Errors(err ...error) error {
 
 	return errors.New(message)
 }
+
+func IsNoMoreData(err error) bool {
+	return err == ErrNoMoreData
+}
diff --git a/pkg/runtime/expressions/clauses/collect_iterator.go b/pkg/runtime/expressions/clauses/collect_iterator.go
@@ -97,7 +97,7 @@ func (iterator *CollectIterator) Next(ctx context.Context, scope *core.Scope) (*
 		return val, nil
 	}
 
-	return nil, nil
+	return nil, core.ErrNoMoreData
 }
 
 func (iterator *CollectIterator) init(ctx context.Context, scope *core.Scope) ([]*core.Scope, error) {
@@ -137,11 +137,11 @@ func (iterator *CollectIterator) group(ctx context.Context, scope *core.Scope) (
 		dataSourceScope, err := iterator.dataSource.Next(ctx, scope.Fork())
 
 		if err != nil {
-			return nil, err
-		}
+			if core.IsNoMoreData(err) {
+				break
+			}
 
-		if dataSourceScope == nil {
-			break
+			return nil, err
 		}
 
 		// this data dataSourceScope represents a data of a given iteration with values retrieved by selectors
@@ -328,14 +328,14 @@ func (iterator *CollectIterator) count(ctx context.Context, scope *core.Scope) (
 	for {
 		// keep all defined variables in forked scopes
 		// all those variables should not be available for further executions
-		os, err := iterator.dataSource.Next(ctx, scope.Fork())
+		_, err := iterator.dataSource.Next(ctx, scope.Fork())
 
 		if err != nil {
-			return nil, err
-		}
+			if core.IsNoMoreData(err) {
+				break
+			}
 
-		if os == nil {
-			break
+			return nil, err
 		}
 
 		counter++
@@ -368,11 +368,11 @@ func (iterator *CollectIterator) aggregate(ctx context.Context, scope *core.Scop
 		os, err := iterator.dataSource.Next(ctx, scope.Fork())
 
 		if err != nil {
-			return nil, err
-		}
+			if core.IsNoMoreData(err) {
+				break
+			}
 
-		if os == nil {
-			break
+			return nil, err
 		}
 
 		// iterate over each selector for a current data set