lucene-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From xavier jmlucjav <jmluc...@gmail.com>
Subject Re: MemoryIndex NPE with Point
Date Sat, 12 Aug 2017 15:38:48 GMT
David,

On Sat, Aug 12, 2017 at 4:43 PM, David Smiley <david.w.smiley@gmail.com>
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 <jmlucjav@gmail.com>
> 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(Memory
>> Index.java:619)
>> at org.apache.lucene.index.memory.MemoryIndex.addField(MemoryIn
>> dex.java:509)
>> at org.apache.lucene.index.memory.MemoryIndex.addField(MemoryIn
>> dex.java:484)
>> at org.apache.lucene.index.memory.MemoryIndex.addField(MemoryIn
>> dex.java:459)
>> at org.apache.lucene.index.memory.MemoryIndex.addField(MemoryIn
>> dex.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
> LinkedIn: http://linkedin.com/in/davidwsmiley | Book:
> http://www.solrenterprisesearchserver.com
>

Mime
View raw message