Lucene - Core
  1. Lucene - Core
  2. LUCENE-4218

contrary to documentation Document.get(field) on numeric field returns null

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Critical Critical
    • Resolution: Fixed
    • Affects Version/s: 4.0-ALPHA
    • Fix Version/s: 4.0, 6.0
    • Component/s: core/search
    • Labels:
    • Environment:

      Darwin e4-ce-8f-0f-c2-b0.dummy.porta.siemens.net 10.8.0 Darwin Kernel Version 10.8.0: Tue Jun 7 16:32:41 PDT 2011; root:xnu-1504.15.3~1/RELEASE_X86_64 x86_64

    • Lucene Fields:
      New

      Description

      A call to Numeric num = indexableField.numericValue() comes up with a correct value, whereas Document.get(field) yields null.

      1. LUCENE-4218.patch
        2 kB
        Robert Muir
      2. LUCENE-4218.patch
        4 kB
        Robert Muir

        Activity

        Hide
        Hoss Man added a comment -

        4.0.0-APHA docs for Document.get say (emphasis mine)...

        Returns the string value of the field with the given name if any exist in this document, or null. If multiple fields exist with this name, this method returns the first value added. If only binary fields with this name exist, returns null. For IntField, LongField, FloatField and DoubleField it returns the string value of the number. If you want the actual numeric field instance back, use getField(java.lang.String).

        ...not sure if the problem is the docs, or if the method is broken, but either way we should fix

        Show
        Hoss Man added a comment - 4.0.0-APHA docs for Document.get say (emphasis mine)... Returns the string value of the field with the given name if any exist in this document, or null. If multiple fields exist with this name, this method returns the first value added. If only binary fields with this name exist, returns null. For IntField, LongField, FloatField and DoubleField it returns the string value of the number. If you want the actual numeric field instance back, use getField(java.lang.String). ...not sure if the problem is the docs, or if the method is broken, but either way we should fix
        Hide
        Uwe Schindler added a comment -

        Before 4.0, this was exactly the case like documented. So we should either fix the docs or let it return string representation. I prefer the latter, as Document.get() is a very generic name.

        Show
        Uwe Schindler added a comment - Before 4.0, this was exactly the case like documented. So we should either fix the docs or let it return string representation. I prefer the latter, as Document.get() is a very generic name.
        Hide
        Robert Muir added a comment -

        rmuir20120906-bulk-40-change

        Show
        Robert Muir added a comment - rmuir20120906-bulk-40-change
        Hide
        Robert Muir added a comment -

        This just confused me too writing a simple test. we need to decide which way to go. If nobody speaks up, I will proceed with making a patch
        that fixes the code to match the docs.

        Show
        Robert Muir added a comment - This just confused me too writing a simple test. we need to decide which way to go. If nobody speaks up, I will proceed with making a patch that fixes the code to match the docs.
        Hide
        Michael McCandless added a comment -

        +1 to return string-ified number.

        Show
        Michael McCandless added a comment - +1 to return string-ified number.
        Hide
        Uwe Schindler added a comment -

        I complained already, because in 3.x (3.2+ when i added native numeric values) it behaves exactly like that and I did this with backwards-compatibility in mind! This may not be true anymore in 4.x, but should still be supported.

        Oh, I commented that already on that issue

        Show
        Uwe Schindler added a comment - I complained already, because in 3.x (3.2+ when i added native numeric values) it behaves exactly like that and I did this with backwards-compatibility in mind! This may not be true anymore in 4.x, but should still be supported. Oh, I commented that already on that issue
        Hide
        Robert Muir added a comment -

        simple patch

        Show
        Robert Muir added a comment - simple patch
        Hide
        Uwe Schindler added a comment -

        I think this patch should be fine, but its much more stupid than 3.x. In 3.x the stringValue() method was also implemented for NumericField, and that one returned the stringfied value. This was changed in 4/5, so breaking this. We might think about this and think of changing to stringValue() on Field handle this. By the way, stringValue() on 3.x was simply returning fieldsData.toString() -> very generic

        Show
        Uwe Schindler added a comment - I think this patch should be fine, but its much more stupid than 3.x. In 3.x the stringValue() method was also implemented for NumericField, and that one returned the stringfied value. This was changed in 4/5, so breaking this. We might think about this and think of changing to stringValue() on Field handle this. By the way, stringValue() on 3.x was simply returning fieldsData.toString() -> very generic
        Hide
        Robert Muir added a comment -

        The next question: should the fix really be in Document.java or Field.java:

        In 3.x, the stringValue() of a numeric field returned its string representation.
        in trunk/4.x, this isnt true: and it makes some things confusing, like the javadocs
        of stringValue itself (clearly bogus)

          /**
           * The value of the field as a String, or null. If null, the Reader value or
           * binary value is used. Exactly one of stringValue(), readerValue(), and
           * getBinaryValue() must be set.
           */
          public String stringValue() {
            return fieldsData instanceof String ? (String) fieldsData : null;
          }
        
        Show
        Robert Muir added a comment - The next question: should the fix really be in Document.java or Field.java: In 3.x, the stringValue() of a numeric field returned its string representation. in trunk/4.x, this isnt true: and it makes some things confusing, like the javadocs of stringValue itself (clearly bogus) /** * The value of the field as a String , or null . If null , the Reader value or * binary value is used. Exactly one of stringValue(), readerValue(), and * getBinaryValue() must be set. */ public String stringValue() { return fieldsData instanceof String ? ( String ) fieldsData : null ; }
        Hide
        Robert Muir added a comment -

        Revised patch fixing Field.java, so it acts like 3.x

        I think this is better.

        Show
        Robert Muir added a comment - Revised patch fixing Field.java, so it acts like 3.x I think this is better.
        Hide
        Uwe Schindler added a comment -

        I like that better, too! This is the way how 3.x worked since addition of numeric fields.

        Show
        Uwe Schindler added a comment - I like that better, too! This is the way how 3.x worked since addition of numeric fields.
        Hide
        Michael McCandless added a comment -

        +1 to fix Field.

        Show
        Michael McCandless added a comment - +1 to fix Field.
        Hide
        Robert Muir added a comment -

        Thanks for reporting this Jamie! Sorry it took so long!

        Show
        Robert Muir added a comment - Thanks for reporting this Jamie! Sorry it took so long!
        Hide
        Uwe Schindler added a comment -

        Closed after release.

        Show
        Uwe Schindler added a comment - Closed after release.

          People

          • Assignee:
            Robert Muir
            Reporter:
            Jamie
          • Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development