I checked this: > We can also add IOE to iterator()?
We cannot, because "Fields implements Iterable<String>" and Iterable is not allowed to throw Exceptions. This feels bad, but current implementations only return an Iterator over a TreeSet, loaded before. It looks just wrong because all other methodsa are allowed to throw IOEx (none of them do!), but this one not. If we would have a codec, that would throw Exception in the other methods, it would for sure also throw ex in the iterator. I fixed the JavaDocs already and improved FilterAtomicReader to use a generic Iterator from the stored fields branch - see my commit. Uwe ----- Uwe Schindler H.-H.-Meier-Allee 63, D-28213 Bremen http://www.thetaphi.de eMail: [email protected] > -----Original Message----- > From: Michael McCandless [mailto:[email protected]] > Sent: Monday, August 20, 2012 2:37 PM > To: [email protected] > Subject: Re: svn commit: r1374647 - in /lucene/dev/branches/branch_4x: ./ > lucene/ lucene/test-framework/ lucene/test- > framework/src/java/org/apache/lucene/index/FieldFilterAtomicReader.java > > +1 to fix the javadocs: it's completely wrong now! > > We can also add IOE to iterator()? > > Mike McCandless > > http://blog.mikemccandless.com > > On Sat, Aug 18, 2012 at 6:13 PM, Uwe Schindler <[email protected]> wrote: > > Hm, > > > > sorry for noise, I think Javadocs are wrong. BlockTermsReader > > implements this as fields.keySet().size(), so its correct what my impl > > returns, but the Javadocs in Fields.java are wrong. > > > > Mike - I have no idea. What is the correct thing? Something is inconsistent. > > > > I fixed Fields.size(9 javadocs to: > > /** Returns the number of fields or -1 if the number of > > * distinct field names is unknown. If >= 0, > > * {@link #iterator} will return as many field names. */ > > public abstract int size() throws IOException; > > > > The second strange thing is that Fields.size() and all other Fields > > methods throw IOExcepotion, but iterator() not! > > Should I commit the javadocs fix? > > > > Uwe > > > > ----- > > Uwe Schindler > > H.-H.-Meier-Allee 63, D-28213 Bremen > > http://www.thetaphi.de > > eMail: [email protected] > > > > > >> -----Original Message----- > >> From: Uwe Schindler [mailto:[email protected]] > >> Sent: Saturday, August 18, 2012 11:55 PM > >> To: [email protected] > >> Subject: RE: svn commit: r1374647 - in > >> /lucene/dev/branches/branch_4x: ./ lucene/ lucene/test-framework/ > >> lucene/test- > >> framework/src/java/org/apache/lucene/index/FieldFilterAtomicReader.ja > >> va > >> > >> And its worse: this method returns int, while Terms.size() returns long! > >> > >> ----- > >> Uwe Schindler > >> H.-H.-Meier-Allee 63, D-28213 Bremen > >> http://www.thetaphi.de > >> eMail: [email protected] > >> > >> > >> > -----Original Message----- > >> > From: Uwe Schindler [mailto:[email protected]] > >> > Sent: Saturday, August 18, 2012 11:52 PM > >> > To: [email protected] > >> > Subject: RE: svn commit: r1374647 - in /lucene/dev/branches/branch_4x: > >> > ./ lucene/ lucene/test-framework/ lucene/test- > >> > framework/src/java/org/apache/lucene/index/FieldFilterAtomicReader. > >> > jav > >> > a > >> > > >> > I checked again: > >> > > >> > /** Returns the number of terms for all fields, or -1 if this > >> > * measure isn't stored by the codec. Note that, just like > >> > * other term measures, this measure does not take deleted > >> > * documents into account. */ > >> > public abstract int size() throws IOException; > >> > > >> > So this method is returning nonsense. Should return -1, as we don't > >> > know > >> this > >> > information, right? > >> > > >> > ----- > >> > Uwe Schindler > >> > H.-H.-Meier-Allee 63, D-28213 Bremen http://www.thetaphi.de > >> > eMail: [email protected] > >> > > >> > > >> > > -----Original Message----- > >> > > From: Uwe Schindler [mailto:[email protected]] > >> > > Sent: Saturday, August 18, 2012 11:27 PM > >> > > To: [email protected] > >> > > Subject: RE: svn commit: r1374647 - in /lucene/dev/branches/branch_4x: > >> > > ./ lucene/ lucene/test-framework/ lucene/test- > >> > > framework/src/java/org/apache/lucene/index/FieldFilterAtomicReader.j > >> > > av > >> > > a > >> > > > >> > > No problem, I wrote the original code, so I was fixing it in parallel! > >> > Sorry for > >> > > changing your fix :-) > >> > > > >> > > We should maybe improve the iterator at all (the StoredFields branch > >> > > has a FilterIterator already, which just has a matches() method) and > >> > > make the > >> > whole > >> > > class available outside of tests? Its as useful as the other > >> > > FilterReaders > >> > in > >> > > contrib/misc. You can use it to split an index vertically (remove > >> > > fields > >> > from an > >> > > index by the usual IW.addIndexes(new > >> > > FieldFilterAtomicReader(...)) > >> > > > >> > > Uwe > >> > > > >> > > ----- > >> > > Uwe Schindler > >> > > H.-H.-Meier-Allee 63, D-28213 Bremen http://www.thetaphi.de > >> > > eMail: [email protected] > >> > > > >> > > > >> > > > -----Original Message----- > >> > > > From: Michael McCandless [mailto:[email protected]] > >> > > > Sent: Saturday, August 18, 2012 11:22 PM > >> > > > To: [email protected] > >> > > > Cc: [email protected] > >> > > > Subject: Re: svn commit: r1374647 - in > > /lucene/dev/branches/branch_4x: > >> > > > ./ lucene/ lucene/test-framework/ lucene/test- > >> > > > > framework/src/java/org/apache/lucene/index/FieldFilterAtomicReader.j > >> > > > av > >> > > > a > >> > > > > >> > > > Woops, thanks Uwe! > >> > > > > >> > > > Mike McCandless > >> > > > > >> > > > http://blog.mikemccandless.com > >> > > > > >> > > > On Sat, Aug 18, 2012 at 3:56 PM, <[email protected]> wrote: > >> > > > > Author: uschindler > >> > > > > Date: Sat Aug 18 19:56:33 2012 > >> > > > > New Revision: 1374647 > >> > > > > > >> > > > > URL: http://svn.apache.org/viewvc?rev=1374647&view=rev > >> > > > > Log: > >> > > > > Merged revision(s) 1374646 from lucene/dev/trunk: > >> > > > > 2nd fix: The same problem had size() > >> > > > > > >> > > > > Modified: > >> > > > > lucene/dev/branches/branch_4x/ (props changed) > >> > > > > lucene/dev/branches/branch_4x/lucene/ (props changed) > >> > > > > lucene/dev/branches/branch_4x/lucene/test-framework/ (props > >> > > changed) > >> > > > > > >> > > > > lucene/dev/branches/branch_4x/lucene/test- > framework/src/java/org/a > >> > > > > pa ch e/lucene/index/FieldFilterAtomicReader.java > >> > > > > > >> > > > > Modified: > >> > > > > lucene/dev/branches/branch_4x/lucene/test- > framework/src/java/org/a > >> > > > > pa ch e/lucene/index/FieldFilterAtomicReader.java > >> > > > > URL: > >> > > > > > http://svn.apache.org/viewvc/lucene/dev/branches/branch_4x/lucene/ > >> > > > > te > >> > > > > st > >> > > > > - > >> framework/src/java/org/apache/lucene/index/FieldFilterAtomicReader. > >> > > > > ja va?rev=1374647&r1=1374646&r2=1374647&view=diff > >> > > > > > >> > > > > >> > > > >> > > >> > ================================================================ > >> > > > ====== > >> > > > > ======== > >> > > > > --- > >> > > > > lucene/dev/branches/branch_4x/lucene/test- > framework/src/java/org/a > >> > > > > pa ch e/lucene/index/FieldFilterAtomicReader.java (original) > >> > > > > +++ lucene/dev/branches/branch_4x/lucene/test- > framework/src/java/o > >> > > > > +++ rg /a pache/lucene/index/FieldFilterAtomicReader.java Sat Aug > >> > > > > +++ 18 > >> > > > > +++ 19:56:33 2012 > >> > > > > @@ -64,11 +64,7 @@ public final class FieldFilterAtomicRead > >> > > > > f = new FieldFilterFields(f); > >> > > > > // we need to check for emptyness, so we can return > >> > > > > // null: > >> > > > > - if (f.iterator().hasNext()) { > >> > > > > - return f; > >> > > > > - } else { > >> > > > > - return null; > >> > > > > - } > >> > > > > + return f.iterator().hasNext() ? f : null; > >> > > > > } > >> > > > > > >> > > > > @Override > >> > > > > @@ -146,7 +142,8 @@ public final class FieldFilterAtomicRead > >> > > > > // TODO: add faster implementation! > >> > > > > int c = 0; > >> > > > > final Iterator<String> it = iterator(); > >> > > > > - while (it.next() != null) { > >> > > > > + while (it.hasNext()) { > >> > > > > + it.next(); > >> > > > > c++; > >> > > > > } > >> > > > > return c; > >> > > > > @@ -156,7 +153,7 @@ public final class FieldFilterAtomicRead > >> > > > > public Iterator<String> iterator() { > >> > > > > final Iterator<String> in = super.iterator(); > >> > > > > return new Iterator<String>() { > >> > > > > - String cached = null; > >> > > > > + private String cached = null; > >> > > > > > >> > > > > @Override > >> > > > > public String next() { > >> > > > > > >> > > > > > >> > > > > >> > > > -------------------------------------------------------------------- > >> > > > - To unsubscribe, e-mail: [email protected] For > >> > > > additional commands, e-mail: [email protected] > >> > > > >> > > > >> > > --------------------------------------------------------------------- > >> > > To unsubscribe, e-mail: [email protected] For > >> > > additional commands, e-mail: [email protected] > >> > > >> > > >> > --------------------------------------------------------------------- > >> > To unsubscribe, e-mail: [email protected] For > additional > >> > commands, e-mail: [email protected] > >> > >> > >> --------------------------------------------------------------------- > >> To unsubscribe, e-mail: [email protected] > >> For additional commands, e-mail: [email protected] > > > > > > --------------------------------------------------------------------- > > To unsubscribe, e-mail: [email protected] > > For additional commands, e-mail: [email protected] > > > > --------------------------------------------------------------------- > To unsubscribe, e-mail: [email protected] > For additional commands, e-mail: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
