From 1b6395ed0af3c1bfc53d1f644f5dbd4d79803b54 Mon Sep 17 00:00:00 2001 From: Andrew Dunham Date: Tue, 14 Apr 2015 20:17:31 -0700 Subject: [PATCH] Make Close() method on Iterators return an error --- graph/bolt/all_iterator.go | 3 ++- graph/bolt/iterator.go | 3 ++- graph/gaedatastore/iterator.go | 3 ++- graph/iterator.go | 2 +- graph/iterator/all_iterator.go | 4 +++- graph/iterator/and_iterator.go | 16 +++++++++++++--- graph/iterator/fixed_iterator.go | 4 +++- graph/iterator/hasa_iterator.go | 12 +++++++++--- graph/iterator/iterator.go | 4 +++- graph/iterator/linksto_iterator.go | 14 +++++++++++--- graph/iterator/materialize_iterator.go | 4 ++-- graph/iterator/not_iterator.go | 16 +++++++++++++--- graph/iterator/optional_iterator.go | 4 ++-- graph/iterator/or_iterator.go | 15 ++++++++++++--- graph/iterator/value_comparison_iterator.go | 4 ++-- graph/leveldb/all_iterator.go | 3 ++- graph/leveldb/iterator.go | 3 ++- graph/memstore/iterator.go | 4 +++- graph/mongo/iterator.go | 4 ++-- 19 files changed, 89 insertions(+), 33 deletions(-) diff --git a/graph/bolt/all_iterator.go b/graph/bolt/all_iterator.go index 4ae6fca..d8fae27 100644 --- a/graph/bolt/all_iterator.go +++ b/graph/bolt/all_iterator.go @@ -174,10 +174,11 @@ func (it *AllIterator) Contains(v graph.Value) bool { return true } -func (it *AllIterator) Close() { +func (it *AllIterator) Close() error { it.result = nil it.buffer = nil it.done = true + return nil } func (it *AllIterator) Size() (int64, bool) { diff --git a/graph/bolt/iterator.go b/graph/bolt/iterator.go index 50b2588..6a28422 100644 --- a/graph/bolt/iterator.go +++ b/graph/bolt/iterator.go @@ -106,10 +106,11 @@ func (it *Iterator) Clone() graph.Iterator { return out } -func (it *Iterator) Close() { +func (it *Iterator) Close() error { it.result = nil it.buffer = nil it.done = true + return nil } func (it *Iterator) isLiveValue(val []byte) bool { diff --git a/graph/gaedatastore/iterator.go b/graph/gaedatastore/iterator.go index 316207d..8f3ef18 100644 --- a/graph/gaedatastore/iterator.go +++ b/graph/gaedatastore/iterator.go @@ -130,12 +130,13 @@ func (it *Iterator) Reset() { it.result = nil } -func (it *Iterator) Close() { +func (it *Iterator) Close() error { it.buffer = nil it.offset = 0 it.done = true it.last = "" it.result = nil + return nil } func (it *Iterator) Tagger() *graph.Tagger { diff --git a/graph/iterator.go b/graph/iterator.go index f9edd52..63ccade 100644 --- a/graph/iterator.go +++ b/graph/iterator.go @@ -136,7 +136,7 @@ type Iterator interface { Describe() Description // Close the iterator and do internal cleanup. - Close() + Close() error // UID returns the unique identifier of the iterator. UID() uint64 diff --git a/graph/iterator/all_iterator.go b/graph/iterator/all_iterator.go index 0fc2694..3e37563 100644 --- a/graph/iterator/all_iterator.go +++ b/graph/iterator/all_iterator.go @@ -55,7 +55,9 @@ func (it *Int64) Reset() { it.at = it.min } -func (it *Int64) Close() {} +func (it *Int64) Close() error { + return nil +} func (it *Int64) Clone() graph.Iterator { out := NewInt64(it.min, it.max) diff --git a/graph/iterator/and_iterator.go b/graph/iterator/and_iterator.go index 0cd6cfb..956c21c 100644 --- a/graph/iterator/and_iterator.go +++ b/graph/iterator/and_iterator.go @@ -263,12 +263,22 @@ func (it *And) cleanUp() {} // Close this iterator, and, by extension, close the subiterators. // Close should be idempotent, and it follows that if it's subiterators // follow this contract, the And follows the contract. -func (it *And) Close() { +// +// Note: as this potentially involves closing multiple subiterators, only +// the first error encountered while closing will be reported (if any). +func (it *And) Close() error { it.cleanUp() - it.primaryIt.Close() + + var ret error + + ret = it.primaryIt.Close() for _, sub := range it.internalIterators { - sub.Close() + if err := sub.Close(); err != nil && ret != nil { + ret = err + } } + + return ret } // Register this as an "and" iterator. diff --git a/graph/iterator/fixed_iterator.go b/graph/iterator/fixed_iterator.go index cef8e1b..35c2690 100644 --- a/graph/iterator/fixed_iterator.go +++ b/graph/iterator/fixed_iterator.go @@ -63,7 +63,9 @@ func (it *Fixed) Reset() { it.lastIndex = 0 } -func (it *Fixed) Close() {} +func (it *Fixed) Close() error { + return nil +} func (it *Fixed) Tagger() *graph.Tagger { return &it.tags diff --git a/graph/iterator/hasa_iterator.go b/graph/iterator/hasa_iterator.go index 879e031..031c3b0 100644 --- a/graph/iterator/hasa_iterator.go +++ b/graph/iterator/hasa_iterator.go @@ -247,11 +247,17 @@ func (it *HasA) Stats() graph.IteratorStats { } // Close the subiterator, the result iterator (if any) and the HasA. -func (it *HasA) Close() { +func (it *HasA) Close() error { + var ret error + if it.resultIt != nil { - it.resultIt.Close() + ret = it.resultIt.Close() } - it.primaryIt.Close() + if err := it.primaryIt.Close(); err != nil && ret != nil { + ret = err + } + + return ret } // Register this iterator as a HasA. diff --git a/graph/iterator/iterator.go b/graph/iterator/iterator.go index e822e55..371025c 100644 --- a/graph/iterator/iterator.go +++ b/graph/iterator/iterator.go @@ -114,7 +114,9 @@ func (it *Null) Size() (int64, bool) { func (it *Null) Reset() {} -func (it *Null) Close() {} +func (it *Null) Close() error { + return nil +} // A null iterator costs nothing. Use it! func (it *Null) Stats() graph.IteratorStats { diff --git a/graph/iterator/linksto_iterator.go b/graph/iterator/linksto_iterator.go index cc1cfa5..bf51293 100644 --- a/graph/iterator/linksto_iterator.go +++ b/graph/iterator/linksto_iterator.go @@ -181,9 +181,17 @@ func (it *LinksTo) Result() graph.Value { } // Close our subiterators. -func (it *LinksTo) Close() { - it.nextIt.Close() - it.primaryIt.Close() +func (it *LinksTo) Close() error { + var ret error + + if err := it.nextIt.Close(); err != nil && ret != nil { + ret = err + } + if err := it.primaryIt.Close(); err != nil && ret != nil { + ret = err + } + + return ret } // We won't ever have a new result, but our subiterators might. diff --git a/graph/iterator/materialize_iterator.go b/graph/iterator/materialize_iterator.go index f771e89..871df88 100644 --- a/graph/iterator/materialize_iterator.go +++ b/graph/iterator/materialize_iterator.go @@ -69,11 +69,11 @@ func (it *Materialize) Reset() { it.index = -1 } -func (it *Materialize) Close() { - it.subIt.Close() +func (it *Materialize) Close() error { it.containsMap = nil it.values = nil it.hasRun = false + return it.subIt.Close() } func (it *Materialize) Tagger() *graph.Tagger { diff --git a/graph/iterator/not_iterator.go b/graph/iterator/not_iterator.go index 7a9ea21..3bee077 100644 --- a/graph/iterator/not_iterator.go +++ b/graph/iterator/not_iterator.go @@ -123,9 +123,19 @@ func (it *Not) NextPath() bool { return false } -func (it *Not) Close() { - it.primaryIt.Close() - it.allIt.Close() +// Close closes the primary and all iterators. If an error occurs, only the +// first one will be returned. +func (it *Not) Close() error { + var ret error + + if err := it.primaryIt.Close(); err != nil && ret != nil { + ret = err + } + if err := it.allIt.Close(); err != nil && ret != nil { + ret = err + } + + return ret } func (it *Not) Type() graph.Type { return graph.Not } diff --git a/graph/iterator/optional_iterator.go b/graph/iterator/optional_iterator.go index bc65acc..f39f57d 100644 --- a/graph/iterator/optional_iterator.go +++ b/graph/iterator/optional_iterator.go @@ -57,8 +57,8 @@ func (it *Optional) Reset() { it.lastCheck = false } -func (it *Optional) Close() { - it.subIt.Close() +func (it *Optional) Close() error { + return it.subIt.Close() } func (it *Optional) Tagger() *graph.Tagger { diff --git a/graph/iterator/or_iterator.go b/graph/iterator/or_iterator.go index 48875dc..0093eb5 100644 --- a/graph/iterator/or_iterator.go +++ b/graph/iterator/or_iterator.go @@ -241,12 +241,21 @@ func (it *Or) cleanUp() {} // Close this graph.iterator, and, by extension, close the subiterators. // Close should be idempotent, and it follows that if it's subiterators -// follow this contract, the And follows the contract. -func (it *Or) Close() { +// follow this contract, the Or follows the contract. +// +// Note: as this potentially involves closing multiple subiterators, only +// the first error encountered while closing will be reported (if any). +func (it *Or) Close() error { it.cleanUp() + + var ret error for _, sub := range it.internalIterators { - sub.Close() + if err := sub.Close(); err != nil && ret != nil { + ret = err + } } + + return ret } func (it *Or) Optimize() (graph.Iterator, bool) { diff --git a/graph/iterator/value_comparison_iterator.go b/graph/iterator/value_comparison_iterator.go index 19abab7..0299fdd 100644 --- a/graph/iterator/value_comparison_iterator.go +++ b/graph/iterator/value_comparison_iterator.go @@ -92,8 +92,8 @@ func (it *Comparison) doComparison(val graph.Value) bool { } } -func (it *Comparison) Close() { - it.subIt.Close() +func (it *Comparison) Close() error { + return it.subIt.Close() } func RunIntOp(a int64, op Operator, b int64) bool { diff --git a/graph/leveldb/all_iterator.go b/graph/leveldb/all_iterator.go index 2940a0d..e0d1bdc 100644 --- a/graph/leveldb/all_iterator.go +++ b/graph/leveldb/all_iterator.go @@ -145,11 +145,12 @@ func (it *AllIterator) Contains(v graph.Value) bool { return true } -func (it *AllIterator) Close() { +func (it *AllIterator) Close() error { if it.open { it.iter.Release() it.open = false } + return nil } func (it *AllIterator) Size() (int64, bool) { diff --git a/graph/leveldb/iterator.go b/graph/leveldb/iterator.go index 3a77188..83321ad 100644 --- a/graph/leveldb/iterator.go +++ b/graph/leveldb/iterator.go @@ -109,11 +109,12 @@ func (it *Iterator) Clone() graph.Iterator { return out } -func (it *Iterator) Close() { +func (it *Iterator) Close() error { if it.open { it.iter.Release() it.open = false } + return nil } func (it *Iterator) isLiveValue(val []byte) bool { diff --git a/graph/memstore/iterator.go b/graph/memstore/iterator.go index 96911af..17bd8e2 100644 --- a/graph/memstore/iterator.go +++ b/graph/memstore/iterator.go @@ -105,7 +105,9 @@ func (it *Iterator) Clone() graph.Iterator { return m } -func (it *Iterator) Close() {} +func (it *Iterator) Close() error { + return nil +} func (it *Iterator) checkValid(index int64) bool { return it.qs.log[index].DeletedBy == 0 diff --git a/graph/mongo/iterator.go b/graph/mongo/iterator.go index bbe1b4f..6409901 100644 --- a/graph/mongo/iterator.go +++ b/graph/mongo/iterator.go @@ -99,8 +99,8 @@ func (it *Iterator) Reset() { } -func (it *Iterator) Close() { - it.iter.Close() +func (it *Iterator) Close() error { + return it.iter.Close() } func (it *Iterator) Tagger() *graph.Tagger {