reopen bug cloning norms for all fields

classic Classic list List threaded Threaded
3 messages Options
Reply | Threaded
Open this post in threaded view
|

reopen bug cloning norms for all fields

Yonik Seeley-2-2
After an NPE and a quick gander at SegmentReader, it looks like it's
trying to clone the norms for *all* fields, regardless of if they are
indexed.
The following is the simplest patch that seems to fix things, but I'm
wondering if other changes might be warranted (like where
fieldNormsChanged is set... should we only check for indexed fields
that have norms enabled?  or if we should have CHANGED/UNCHANGED/NONE
rather than true/false)

-Yonik

Index: src/java/org/apache/lucene/index/SegmentReader.java
===================================================================
--- src/java/org/apache/lucene/index/SegmentReader.java (revision 741813)
+++ src/java/org/apache/lucene/index/SegmentReader.java (working copy)
@@ -722,7 +722,8 @@
         if (doClone || !fieldNormsChanged[i]) {
           final String curField = fieldInfos.fieldInfo(i).name;
           Norm norm = (Norm) this.norms.get(curField);
-          clone.norms.put(curField, norm.clone());
+          if (norm != null)
+            clone.norms.put(curField, norm.clone());
         }
       }

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: reopen bug cloning norms for all fields

Yonik Seeley-2-2
It looks like this was introduced just recently in LUCENE-1314.
I've just committed this fix along with test modifications that fail
w/o this patch.

-Yonik

On Fri, Feb 6, 2009 at 10:15 PM, Yonik Seeley
<[hidden email]> wrote:

> After an NPE and a quick gander at SegmentReader, it looks like it's
> trying to clone the norms for *all* fields, regardless of if they are
> indexed.
> The following is the simplest patch that seems to fix things, but I'm
> wondering if other changes might be warranted (like where
> fieldNormsChanged is set... should we only check for indexed fields
> that have norms enabled?  or if we should have CHANGED/UNCHANGED/NONE
> rather than true/false)
>
> -Yonik
>
> Index: src/java/org/apache/lucene/index/SegmentReader.java
> ===================================================================
> --- src/java/org/apache/lucene/index/SegmentReader.java (revision 741813)
> +++ src/java/org/apache/lucene/index/SegmentReader.java (working copy)
> @@ -722,7 +722,8 @@
>         if (doClone || !fieldNormsChanged[i]) {
>           final String curField = fieldInfos.fieldInfo(i).name;
>           Norm norm = (Norm) this.norms.get(curField);
> -          clone.norms.put(curField, norm.clone());
> +          if (norm != null)
> +            clone.norms.put(curField, norm.clone());
>         }
>       }
>

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: reopen bug cloning norms for all fields

Michael McCandless-2

Woops, thanks Yonik.  The bug is real and your fix looks right,

Mike

Yonik Seeley wrote:

> It looks like this was introduced just recently in LUCENE-1314.
> I've just committed this fix along with test modifications that fail
> w/o this patch.
>
> -Yonik
>
> On Fri, Feb 6, 2009 at 10:15 PM, Yonik Seeley
> <[hidden email]> wrote:
>> After an NPE and a quick gander at SegmentReader, it looks like it's
>> trying to clone the norms for *all* fields, regardless of if they are
>> indexed.
>> The following is the simplest patch that seems to fix things, but I'm
>> wondering if other changes might be warranted (like where
>> fieldNormsChanged is set... should we only check for indexed fields
>> that have norms enabled?  or if we should have CHANGED/UNCHANGED/NONE
>> rather than true/false)
>>
>> -Yonik
>>
>> Index: src/java/org/apache/lucene/index/SegmentReader.java
>> ===================================================================
>> --- src/java/org/apache/lucene/index/SegmentReader.java (revision  
>> 741813)
>> +++ src/java/org/apache/lucene/index/SegmentReader.java (working  
>> copy)
>> @@ -722,7 +722,8 @@
>>        if (doClone || !fieldNormsChanged[i]) {
>>          final String curField = fieldInfos.fieldInfo(i).name;
>>          Norm norm = (Norm) this.norms.get(curField);
>> -          clone.norms.put(curField, norm.clone());
>> +          if (norm != null)
>> +            clone.norms.put(curField, norm.clone());
>>        }
>>      }
>>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [hidden email]
> For additional commands, e-mail: [hidden email]
>


---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]