lucene-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Shai Erera <>
Subject IndexReader.listCommits to return a List
Date Wed, 29 Sep 2010 07:22:49 GMT

IndexReader.listCommits returns a Collection, which not only does not define
and guarantee any ordering between the commits, but also makes it very hard
to know what is the latest commit. And we have some inconsistency, IMO, in
how we return the commits today, in the different API:

IndexDeletionPolicy receives a List<IndexCommit> and not Collection.
Furthermore, it relies on the commits being ordered oldest -> latest, and
it's somewhat documented.

listCommits, to my surprise, returns the latest commit as the first element
of the Collection, and the rest of the commits are unsorted. Having seen IDP
relying on the commits to be sorted, I thought that listCommits sorts them
too, but was surprised to find out that the latest one appears first.

The implementation already returns an ArrayList, and I wonder why do we need
the Collection abstraction? Why not returning a List, or even a LinkedList,
or an array - anything that entails ordering between them. And preferably
something which will allow me to access first/last really quickly, and
direct access can be a bonus for all I care.
And we should also document the ordering they are returned, and better make
it consistent w/ IDP's assumptions. It's a static method, so no one can
break by extending it. And committing to some ordering should be problematic
- we already do that with IDPs.

The only problem is that we cannot change the signature of the method, so
I'm ok w/ either break back-compat and document, or deprecate and create a
getCommits (or a better name).

While we're at it, it'd be great if we can consolidate ReaderCommit and
CommitPoint. The latter implements Comparable and supports deletion of
IndexCommits, while the former doesn't do both. Is there any reason why it
cannot support delete()?

What do you think?


View raw message