Mailing List Archive

Proposed changes to SegmentsReader and TermInfosReader
I'm getting a NullPointerException when a requested Term is greater than
(using compareTo) all existing Terms in a given segment. Here are my
proposed changes:

[Index: SegmentsReader.java]
[===================================================================]
[RCS file:
/home/cvs/jakarta-lucene/src/java/org/apache/lucene/index/SegmentsReader.jav
a,v]
[retrieving revision 1.1.1.1]
[diff -r1.1.1.1 SegmentsReader.java]
[181,191c182,195]
[< if (t != null) {]
[< termEnum = (SegmentTermEnum)reader.terms(t);]
[< } else]
[< termEnum = (SegmentTermEnum)reader.terms();]
[< ]
[< SegmentMergeInfo smi = new SegmentMergeInfo(starts[i], termEnum,
reader);]
[.< if (t == null ? smi.next() : termEnum.term() != null)]
[< queue.put(smi); // initialize queue]
[< else]
[< smi.close();]
[< }]
[---]
[> if (t != null) {]
[> termEnum = (SegmentTermEnum)reader.terms(t);]
[> } else {]
[> termEnum = (SegmentTermEnum)reader.terms();]
[> }]
[> ]
[> if (termEnum != null) {]
[> SegmentMergeInfo smi = new SegmentMergeInfo(starts[i],
termEnum, reader);]
[.> if (t == null ? smi.next() : termEnum.term() != null)]
[> queue.put(smi); //
initialize queue]
[> else]
[> smi.close();]
[> }]
[> }]


[Index: TermInfosReader.java]
[===================================================================]
[RCS file:
/home/cvs/jakarta-lucene/src/java/org/apache/lucene/index/TermInfosReader.ja
va,v]
[retrieving revision 1.1.1.1]
[diff -r1.1.1.1 TermInfosReader.java]
[217c217]
[< get(term); // seek enum to
term]
[---]
[.> if (get(term) == null) return null; // seek enum to term]


Please let me know if you see a problem with the change. Otherwise, I'll
check it in tomorrow.

Thanks,
Scott
RE: Proposed changes to SegmentsReader and TermInfosReader [ In reply to ]
1. Your diffs are hard to read! Why the square brackets? Also, it looks
like you re-indented and added brackets to some code that you did not
change, so that it shows up in the diffs. That's a no-no in my book.
Personally I prefer the '-u' and '-w' flags to 'diff'. If you're using 'cvs
diff' to generate diffs from the checked-in version, then you can just put
the line 'diff -uw' in ~/.cvsrc.

2. This doesn't look very risky. If I'm parsing the diffs correctly, it's
really just two one-line changes to check for null pointers. So I'm
inclined to let it into 1.2RC2. Objections?

Doug

> -----Original Message-----
> From: Scott Ganyo [mailto:scott.ganyo@eTapestry.com]
> Sent: Wednesday, October 10, 2001 9:10 AM
> To: Lucene-Dev (E-mail)
> Subject: Proposed changes to SegmentsReader and TermInfosReader
>
>
> I'm getting a NullPointerException when a requested Term is
> greater than
> (using compareTo) all existing Terms in a given segment. Here are my
> proposed changes:
>
> [Index: SegmentsReader.java]
> [===================================================================]
> [RCS file:
> /home/cvs/jakarta-lucene/src/java/org/apache/lucene/index/Segm
> entsReader.jav
> a,v]
> [retrieving revision 1.1.1.1]
> [diff -r1.1.1.1 SegmentsReader.java]
> [181,191c182,195]
> [< if (t != null) {]
> [< termEnum = (SegmentTermEnum)reader.terms(t);]
> [< } else]
> [< termEnum = (SegmentTermEnum)reader.terms();]
> [< ]
> [< SegmentMergeInfo smi = new
> SegmentMergeInfo(starts[i], termEnum,
> reader);]
> [.< if (t == null ? smi.next() : termEnum.term() != null)]
> [< queue.put(smi); // initialize queue]
> [< else]
> [< smi.close();]
> [< }]
> [---]
> [> if (t != null) {]
> [> termEnum = (SegmentTermEnum)reader.terms(t);]
> [> } else {]
> [> termEnum = (SegmentTermEnum)reader.terms();]
> [> }]
> [> ]
> [> if (termEnum != null) {]
> [> SegmentMergeInfo smi = new
> SegmentMergeInfo(starts[i],
> termEnum, reader);]
> [.> if (t == null ? smi.next() :
> termEnum.term() != null)]
> [> queue.put(smi); //
> initialize queue]
> [> else]
> [> smi.close();]
> [> }]
> [> }]
>
>
> [Index: TermInfosReader.java]
> [===================================================================]
> [RCS file:
> /home/cvs/jakarta-lucene/src/java/org/apache/lucene/index/Term
> InfosReader.ja
> va,v]
> [retrieving revision 1.1.1.1]
> [diff -r1.1.1.1 TermInfosReader.java]
> [217c217]
> [< get(term); //
> seek enum to
> term]
> [---]
> [.> if (get(term) == null) return null; // seek enum to term]
>
>
> Please let me know if you see a problem with the change.
> Otherwise, I'll
> check it in tomorrow.
>
> Thanks,
> Scott
>
RE: Proposed changes to SegmentsReader and TermInfosReader [ In reply to ]
Sorry about the confusion, I'll fix up my diffs in the future.

Anyway, I'm not convinced that this is a non-risky change. The reason being
that the one-line change in the TermInfosReader will now allow a null to be
passed back from the method where none was expected. Instead, I'll propose
a different fix...

The original problem was that SegmentTermEnum.java:51 was getting a
NullPointerException because the term was null. So now instead of passing a
null up the chain, I'd like to instead make the SegmentTermEnum clone() work
regardless of the Term being null. I think the only change that needs to be
done then is to make this change:

Index: SegmentTermEnum.java
===================================================================
RCS file:
/home/cvs/jakarta-lucene/src/java/org/apache/lucene/index/SegmentTermEnum.ja
va,v
retrieving revision 1.1.1.1
diff -u -w -r1.1.1.1 SegmentTermEnum.java
--- SegmentTermEnum.java 2001/09/18 16:29:54 1.1.1.1
+++ SegmentTermEnum.java 2001/10/10 19:14:54
@@ -88,7 +88,7 @@

clone.input = (InputStream)input.clone();
clone.termInfo = new TermInfo(termInfo);
- clone.growBuffer(term.text.length());
+ if (term != null) clone.growBuffer(term.text.length());

return clone;
}

This seems much safer to me as the resulting enum will be valid, but empty.

Scott