Solr
  1. Solr
  2. SOLR-3878

NPE in CurrencyValue.parse() while issuing wildcard range query on a CurrencyField

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Critical Critical
    • Resolution: Fixed
    • Affects Version/s: 3.6.1, 4.0-BETA
    • Fix Version/s: 4.0, 4.1, 6.0
    • Component/s: query parsers
    • Labels:
      None

      Description

      According to the wiki wildcard range queries are supported. In reality either of the following queries result in NPE using the example schema.

      • price_c:[* TO 1000]
      • price_c:[* TO 1000,USD]
      • price_c:[*,USD TO 1000,USD]
      • price_c:[1000 TO *]
      • price_c:[1000,USD TO *]
      • price_c:[1000,USD TO *]
      1. SOLR-3878.patch
        4 kB
        Jan Høydahl

        Activity

        Hide
        Jan Høydahl added a comment -

        Confirmed. The code does not support open-ended ranges Starting a fix.

        Show
        Jan Høydahl added a comment - Confirmed. The code does not support open-ended ranges Starting a fix.
        Hide
        Jan Høydahl added a comment -

        Aiming for the 4.0 respin

        Show
        Jan Høydahl added a comment - Aiming for the 4.0 respin
        Hide
        Jan Høydahl added a comment -

        Patch for trunk attached, including tests

        Show
        Jan Høydahl added a comment - Patch for trunk attached, including tests
        Hide
        Hoss Man added a comment -

        Jan: i've only skimmed the patch, but would it be better/simpler if CurrencyValue.parse(...) always returned a non null CurrencyValue object, but that CurrencyValue.getAmmount() could return null (ie: make it return Long instead of long)?

        that seems like it would simply some of the null checks you had to add in your patch.

        either way:

        • CurrencyValue.parse should have some javadocs talking about how it deals with "*" as input
        • other uses of CurrencyValue.parse need to be checked to ensure the behavior appropriately if "*" is specified as a value (ie: will i get a meaningful error if I try to add a document with a value of "*" ? what happens if i do a query on currency_field:*
        Show
        Hoss Man added a comment - Jan: i've only skimmed the patch, but would it be better/simpler if CurrencyValue.parse(...) always returned a non null CurrencyValue object, but that CurrencyValue.getAmmount() could return null (ie: make it return Long instead of long)? that seems like it would simply some of the null checks you had to add in your patch. either way: CurrencyValue.parse should have some javadocs talking about how it deals with "*" as input other uses of CurrencyValue.parse need to be checked to ensure the behavior appropriately if "*" is specified as a value (ie: will i get a meaningful error if I try to add a document with a value of "*" ? what happens if i do a query on currency_field:*
        Hide
        Miklós Márton added a comment -

        Hoss says something... Either way: I'm pretty happy that it's already solved. :]

        I hate to nitpick, but as I checked the source (always happy to learn something), I noticed a typo in the block comment above CurrencyValue.parse():

           * <p/>
           * Currency values are expected to be in the format &lt;amount&gt;,&lt;currency code&gt;,
           * for example, "500,USD" would represent 5 U.S. Dollars.
           * <p/>
        

        I assume it should be "... 500 U.S. Dollars ..."

        Show
        Miklós Márton added a comment - Hoss says something... Either way: I'm pretty happy that it's already solved. :] I hate to nitpick, but as I checked the source (always happy to learn something), I noticed a typo in the block comment above CurrencyValue.parse(): * <p/> * Currency values are expected to be in the format &lt;amount&gt;,&lt;currency code&gt;, * for example, "500,USD" would represent 5 U.S. Dollars. * <p/> I assume it should be "... 500 U.S. Dollars ..."
        Hide
        Jan Høydahl added a comment -

        Committed to trunk (1389659), 4.x (1389675) and 4.0.0 (1389679)

        Hoss, I kept the implementation returning null as CurrencyValue as this actually means fewer changes, and is more inline with the other range methods.
        Regarding price_c:*, that did not work in 3.6 either (returns 0 hits), and does not work now. Will file a separate issue.
        Also, polishing JavaDocs can be dealt with post 4.0.0 release.

        Show
        Jan Høydahl added a comment - Committed to trunk (1389659), 4.x (1389675) and 4.0.0 (1389679) Hoss, I kept the implementation returning null as CurrencyValue as this actually means fewer changes, and is more inline with the other range methods. Regarding price_c:*, that did not work in 3.6 either (returns 0 hits), and does not work now. Will file a separate issue. Also, polishing JavaDocs can be dealt with post 4.0.0 release.
        Hide
        Commit Tag Bot added a comment -

        [branch_4x commit] Jan Høydahl
        http://svn.apache.org/viewvc?view=revision&revision=1389675

        SOLR-3878: NPE in CurrencyValue.parse() while issuing wildcard range query on a CurrencyField

        Show
        Commit Tag Bot added a comment - [branch_4x commit] Jan Høydahl http://svn.apache.org/viewvc?view=revision&revision=1389675 SOLR-3878 : NPE in CurrencyValue.parse() while issuing wildcard range query on a CurrencyField

          People

          • Assignee:
            Jan Høydahl
            Reporter:
            Miklós Márton
          • Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development