Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Changed logic of iterator termination #330

Merged
merged 2 commits into from
Jul 13, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -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}/... && \
Expand Down
5 changes: 0 additions & 5 deletions pkg/runtime/collections/collection.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
5 changes: 0 additions & 5 deletions pkg/runtime/collections/filter.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
2 changes: 1 addition & 1 deletion pkg/runtime/collections/filter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down
2 changes: 1 addition & 1 deletion pkg/runtime/collections/indexed.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,5 +62,5 @@ func (iterator *IndexedIterator) Next(_ context.Context, scope *core.Scope) (*co
return nextScope, nil
}

return nil, nil
return nil, core.ErrNoMoreData
}
28 changes: 17 additions & 11 deletions pkg/runtime/collections/indexed_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down Expand Up @@ -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))
Expand Down Expand Up @@ -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))
Expand All @@ -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()))
})

Expand All @@ -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)
})
}
8 changes: 4 additions & 4 deletions pkg/runtime/collections/iterator.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion pkg/runtime/collections/keyed.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,5 +60,5 @@ func (iterator *KeyedIterator) Next(_ context.Context, scope *core.Scope) (*core
return nextScope, nil
}

return nil, nil
return nil, core.ErrNoMoreData
}
12 changes: 7 additions & 5 deletions pkg/runtime/collections/keyed_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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() {
Expand All @@ -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)
})
}
13 changes: 6 additions & 7 deletions pkg/runtime/collections/limit.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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++
Expand Down
2 changes: 1 addition & 1 deletion pkg/runtime/collections/map.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
12 changes: 7 additions & 5 deletions pkg/runtime/collections/map_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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() {
Expand All @@ -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)
})
}
2 changes: 1 addition & 1 deletion pkg/runtime/collections/slice.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,5 +51,5 @@ func (iterator *SliceIterator) Next(_ context.Context, scope *core.Scope) (*core
return nextScope, nil
}

return nil, nil
return nil, core.ErrNoMoreData
}
12 changes: 7 additions & 5 deletions pkg/runtime/collections/slice_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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() {
Expand All @@ -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)
})
}
2 changes: 1 addition & 1 deletion pkg/runtime/collections/sort.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
4 changes: 0 additions & 4 deletions pkg/runtime/collections/tap.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
4 changes: 0 additions & 4 deletions pkg/runtime/collections/unique.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
5 changes: 5 additions & 0 deletions pkg/runtime/core/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -65,3 +66,7 @@ func Errors(err ...error) error {

return errors.New(message)
}

func IsNoMoreData(err error) bool {
return err == ErrNoMoreData
}
28 changes: 14 additions & 14 deletions pkg/runtime/expressions/clauses/collect_iterator.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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++
Expand Down Expand Up @@ -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
Expand Down
Loading