MemoryIndex NPE with Point

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
3 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

MemoryIndex NPE with Point

jmlucjav
Hi,

I have this piece of code working fine in 6.4.2

MemoryIndex mi = new MemoryIndex();
float fval = 3.3f;
final String floatfield = "floatfield";
DoublePoint ffield = new DoublePoint(floatfield, fval);
mi.addField(floatfield, ffield.tokenS


If I upgrade to 6.5.1 or 6.6.0 I get a NPE here:
Exception in thread "main" java.lang.NullPointerException
at org.apache.lucene.index.memory.MemoryIndex.storeTerms(MemoryIndex.java:619)
at org.apache.lucene.index.memory.MemoryIndex.addField(MemoryIndex.java:509)
at org.apache.lucene.index.memory.MemoryIndex.addField(MemoryIndex.java:484)
at org.apache.lucene.index.memory.MemoryIndex.addField(MemoryIndex.java:459)
at org.apache.lucene.index.memory.MemoryIndex.addField(MemoryIndex.java:370)


Looking at the code, it looks like in the code path it is being followed, before, it was checked if teh tokenStream was null or not before calling MemoryIndex.storeTerms. Now this check is gone, so the NPE.

Is this intended and I should create the field in some other way now? Or is this just a bug? 

xavier
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: MemoryIndex NPE with Point

david.w.smiley@gmail.com
Hi Xavier,

BTW the lucene-user list is probably the best place to report this.

I looked closer at this.  This aspect of the MemoryIndex API is debatable.... wether a MemoryIndex.addField(...TokenStream...)  should be allowed to be null or not.  Apparently Analyzer.tokenStream(field,...) can return null if the field doesn't have a terms index, so I suppose we should allow null TokenStream because otherwise it could be trappy (unexpected NPE).  Obviously this would be a simple short-circuit implementation.  On the other hand, passing in null may point to bugs in your expectation of how to use MemoryIndex.  That appears to be the case for you -- you are supposed to call MemoryIndex.addField( ffield, ...)   where ffield is your local variable of type DoublePoint which implements IndexableField.  It was an incorrect assumption to think that MemoryIndex requires that you grab the tokenStream from the field.  Those addField convenience methods that take TokenStream are only appropriate for fields with a terms index, i.e.  only works for fields in which IndexOptions != NONE.  DoublePoint in particular does not have this but instead has a points-index (aka PointValues) which is seen by calling field.fieldType().pointDimensionCount() > 0.

So I think I lean towards the current behavior, albeit with a check for a null tokenStream to throw an error that clarifies the contract.  More Javadocs too of course.

~ David

On Sat, Aug 12, 2017 at 5:21 AM xavier jmlucjav <[hidden email]> wrote:
Hi,

I have this piece of code working fine in 6.4.2

MemoryIndex mi = new MemoryIndex();
float fval = 3.3f;
final String floatfield = "floatfield";
DoublePoint ffield = new DoublePoint(floatfield, fval);
mi.addField(floatfield, ffield.tokenS


If I upgrade to 6.5.1 or 6.6.0 I get a NPE here:
Exception in thread "main" java.lang.NullPointerException
at org.apache.lucene.index.memory.MemoryIndex.storeTerms(MemoryIndex.java:619)
at org.apache.lucene.index.memory.MemoryIndex.addField(MemoryIndex.java:509)
at org.apache.lucene.index.memory.MemoryIndex.addField(MemoryIndex.java:484)
at org.apache.lucene.index.memory.MemoryIndex.addField(MemoryIndex.java:459)
at org.apache.lucene.index.memory.MemoryIndex.addField(MemoryIndex.java:370)


Looking at the code, it looks like in the code path it is being followed, before, it was checked if teh tokenStream was null or not before calling MemoryIndex.storeTerms. Now this check is gone, so the NPE.

Is this intended and I should create the field in some other way now? Or is this just a bug? 

xavier
--
Lucene/Solr Search Committer, Consultant, Developer, Author, Speaker
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: MemoryIndex NPE with Point

jmlucjav
David,

On Sat, Aug 12, 2017 at 4:43 PM, David Smiley <[hidden email]> wrote:
Hi Xavier,

BTW the lucene-user list is probably the best place to report this.

Certainly, I intended to post to that list, but sent it to dev one instead, sorry.

 

I looked closer at this.  This aspect of the MemoryIndex API is debatable.... wether a MemoryIndex.addField(...TokenStream...)  should be allowed to be null or not.  Apparently Analyzer.tokenStream(field,...) can return null if the field doesn't have a terms index, so I suppose we should allow null TokenStream because otherwise it could be trappy (unexpected NPE).  Obviously this would be a simple short-circuit implementation.  On the other hand, passing in null may point to bugs in your expectation of how to use MemoryIndex.  That appears to be the case for you -- you are supposed to call MemoryIndex.addField( ffield, ...)   where ffield is your local variable of type DoublePoint which implements IndexableField.  It was an incorrect assumption to think that MemoryIndex requires that you grab the tokenStream from the field.  Those addField convenience methods that take TokenStream are only appropriate for fields with a terms index, i.e.  only works for fields in which IndexOptions != NONE.  DoublePoint in particular does not have this but instead has a points-index (aka PointValues) which is seen by calling field.fieldType().pointDimensionCount() > 0.

 ok, makes sense. At the time I implemented this code there were not convenience method in MemoryIndex to add a field without the tokenStream. I modified my code to do so, and it works fine now with 6.6.0.

thanks!
xavier


So I think I lean towards the current behavior, albeit with a check for a null tokenStream to throw an error that clarifies the contract.  More Javadocs too of course.

~ David

On Sat, Aug 12, 2017 at 5:21 AM xavier jmlucjav <[hidden email]> wrote:
Hi,

I have this piece of code working fine in 6.4.2

MemoryIndex mi = new MemoryIndex();
float fval = 3.3f;
final String floatfield = "floatfield";
DoublePoint ffield = new DoublePoint(floatfield, fval);
mi.addField(floatfield, ffield.tokenS


If I upgrade to 6.5.1 or 6.6.0 I get a NPE here:
Exception in thread "main" java.lang.NullPointerException
at org.apache.lucene.index.memory.MemoryIndex.storeTerms(MemoryIndex.java:619)
at org.apache.lucene.index.memory.MemoryIndex.addField(MemoryIndex.java:509)
at org.apache.lucene.index.memory.MemoryIndex.addField(MemoryIndex.java:484)
at org.apache.lucene.index.memory.MemoryIndex.addField(MemoryIndex.java:459)
at org.apache.lucene.index.memory.MemoryIndex.addField(MemoryIndex.java:370)


Looking at the code, it looks like in the code path it is being followed, before, it was checked if teh tokenStream was null or not before calling MemoryIndex.storeTerms. Now this check is gone, so the NPE.

Is this intended and I should create the field in some other way now? Or is this just a bug? 

xavier
--
Lucene/Solr Search Committer, Consultant, Developer, Author, Speaker

Loading...