ZODB icon indicating copy to clipboard operation
ZODB copied to clipboard

FileStorage: Discrepancy between `iterator()` and `record_iternext()` on empty storages (`record_iternext` underspecified)

Open jamadden opened this issue 3 years ago • 0 comments

In ZODB 5.6 (and almost certainly previous versions), or any version of Python, FileStorage.iterator gracefully handles an empty storage, while record_iternext raises an exception.

>>> from ZODB import config
>>> conf_str = """
... <filestorage>
...    path /tmp/Data.fs
... </filestorage>
... """
>>> storage = config.storageFromString(conf_str)
>>> list(storage.iterator())
[]
>>> storage.record_iternext(None)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/ZODB/src/ZODB/FileStorage/FileStorage.py", line 1409, in record_iternext
    oid = index.minKey(next)
  File "//ZODB/src/ZODB/fsIndex.py", line 236, in minKey
    smallest_prefix = self._data.minKey()
ValueError: empty tree

This is a problem because record_iternext doesn't specify any behaviour around empty storages; it doesn't mention exceptions. I would expect record_iternext to behave like iterator and return a slate of None values, or specify what exception to watch for in that specific case (StopIteration?).

The existing documentation and example, and the iteration-cookie design of record_iternext, suggest that returning a bunch of Nones is likely to break existing clients just as badly as a new exception, so perhaps just specifying the ValueError is the most practical approach.

This comes up in the context of RelStorage and copyTransactionsFrom: For history-free RelStorage instances, it's substantially faster to implement copyTransactionsFrom using record_iternext than iterator().

jamadden avatar Oct 09 '20 14:10 jamadden