Solr
  1. Solr
  2. SOLR-8287

TrieLongField and TrieDoubleField should override toNativeType

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 5.4, 6.0
    • Component/s: None
    • Labels:
      None

      Description

      Although the TrieIntField and TrieFloatField override the toNativeType() method, the TrieLongField and TrieDoubleField do not do so.

      This method is called during atomic updates by the AtomicUpdateDocumentMerger for the "set" operation.

      1. SOLR-8287.patch
        2 kB
        Ishan Chattopadhyaya
      2. SOLR-8287.patch
        2 kB
        Ishan Chattopadhyaya

        Activity

        Hide
        Ishan Chattopadhyaya added a comment -

        This patch adds the overridden methods to the field types.

        Show
        Ishan Chattopadhyaya added a comment - This patch adds the overridden methods to the field types.
        Hide
        Christine Poerschke added a comment -

        The TrieDoubleField change is similar to what TrieFloatField currently has, that looks good to me.

        The TrieLongField change is similar to what TrieIntField currently has, but I'm wondering:

        • TrieIntField.toNativeType attempts Float.parseFloat if the Integer.parseInt attempt throws a NumberFormatException
        • in the current patch TrieLongField.toNativeType attempts Float.parseFloat if the Long.parseLong attempt throws a NumberFormatException but might Double.parseDouble be attempted instead?
        Show
        Christine Poerschke added a comment - The TrieDoubleField change is similar to what TrieFloatField currently has, that looks good to me. The TrieLongField change is similar to what TrieIntField currently has, but I'm wondering: TrieIntField.toNativeType attempts Float.parseFloat if the Integer.parseInt attempt throws a NumberFormatException in the current patch TrieLongField.toNativeType attempts Float.parseFloat if the Long.parseLong attempt throws a NumberFormatException but might Double.parseDouble be attempted instead?
        Hide
        Ishan Chattopadhyaya added a comment -

        Thanks for looking at the patch, Christine Poerschke. I think that is a good point, would defend against any potential precision loss in future.

        Show
        Ishan Chattopadhyaya added a comment - Thanks for looking at the patch, Christine Poerschke . I think that is a good point, would defend against any potential precision loss in future.
        Hide
        ASF subversion and git services added a comment -

        Commit 1714226 from Christine Poerschke in branch 'dev/trunk'
        [ https://svn.apache.org/r1714226 ]

        SOLR-8287: TrieDoubleField and TrieLongField now override toNativeType

        Show
        ASF subversion and git services added a comment - Commit 1714226 from Christine Poerschke in branch 'dev/trunk' [ https://svn.apache.org/r1714226 ] SOLR-8287 : TrieDoubleField and TrieLongField now override toNativeType
        Hide
        ASF subversion and git services added a comment -

        Commit 1714243 from Christine Poerschke in branch 'dev/branches/branch_5x'
        [ https://svn.apache.org/r1714243 ]

        SOLR-8287: TrieDoubleField and TrieLongField now override toNativeType (merge in revision 1714226 from trunk)

        Show
        ASF subversion and git services added a comment - Commit 1714243 from Christine Poerschke in branch 'dev/branches/branch_5x' [ https://svn.apache.org/r1714243 ] SOLR-8287 : TrieDoubleField and TrieLongField now override toNativeType (merge in revision 1714226 from trunk)
        Hide
        Christine Poerschke added a comment -

        Thanks Ishan!

        Show
        Christine Poerschke added a comment - Thanks Ishan!
        Hide
        Ishan Chattopadhyaya added a comment -

        Thanks for the quick review and commit, Christine!

        Show
        Ishan Chattopadhyaya added a comment - Thanks for the quick review and commit, Christine!

          People

          • Assignee:
            Christine Poerschke
            Reporter:
            Ishan Chattopadhyaya
          • Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development