Solr
  1. Solr
  2. SOLR-4138

currency field doesn't work with functions (ie: isn't compatible with frange query)

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 4.0
    • Fix Version/s: 4.2, Trunk
    • Component/s: search
    • Labels:
      None

      Description

      In general, using CurrencyField with FunctionQueries doesn't work

      In particular, as originally reported...

      Filtering using

      {!frange}

      syntax isn't work properly. (rather at all)

      1. SOLR-4135-test.patch
        0.7 kB
        Grzegorz Sobczyk
      2. SOLR-4138.patch
        16 kB
        Hoss Man
      3. SOLR-4138.patch
        3 kB
        Hoss Man

        Issue Links

          Activity

          Hide
          Grzegorz Sobczyk added a comment -

          example patch with test for CurrencyFieldTest

          Show
          Grzegorz Sobczyk added a comment - example patch with test for CurrencyFieldTest
          Hide
          Hoss Man added a comment -

          Thanks for reporting this Grzegorz,

          The general problem seems to be that while CurrencyField has a CurrencyValueSource that it uses for range queries and sorting, it doesn't override FieldType.getValueSource() to return one of these in cases when people use currency fields in functions.

          Even with this simple addition however, your test as written still doesn't pass, because CurrencyValueSource operates using the internal "long" values of the currency, based on Currency.getDefaultFractionDigits() for the default currency .. so if USD is the configured default, you get a value of "500.0" (cents) from the value source instead of "5.0" when a document is indexed with a value of "5,USD".

          I've attached a patch with the trivial getVaueSource() and tests showing the current behavior, but i'm not entirely certain if this is really the behavior we want ... changing it would probably require some meaty rewrites to CurrencyValueSource to take in options about whether the getDefaultFractionDigits() logic should be "undone" to produce the more human friendly values ... or perhaps just wrapping it in a special ValueSource that applies that logic in the getValueSource case so existing usages in getRangeQuery would be unaffected.

          Show
          Hoss Man added a comment - Thanks for reporting this Grzegorz, The general problem seems to be that while CurrencyField has a CurrencyValueSource that it uses for range queries and sorting, it doesn't override FieldType.getValueSource() to return one of these in cases when people use currency fields in functions. Even with this simple addition however, your test as written still doesn't pass, because CurrencyValueSource operates using the internal "long" values of the currency, based on Currency.getDefaultFractionDigits() for the default currency .. so if USD is the configured default, you get a value of "500.0" (cents) from the value source instead of "5.0" when a document is indexed with a value of "5,USD". I've attached a patch with the trivial getVaueSource() and tests showing the current behavior, but i'm not entirely certain if this is really the behavior we want ... changing it would probably require some meaty rewrites to CurrencyValueSource to take in options about whether the getDefaultFractionDigits() logic should be "undone" to produce the more human friendly values ... or perhaps just wrapping it in a special ValueSource that applies that logic in the getValueSource case so existing usages in getRangeQuery would be unaffected.
          Hide
          Hoss Man added a comment -

          enhancing summary & description to reflect broader scope of issue

          Show
          Hoss Man added a comment - enhancing summary & description to reflect broader scope of issue
          Hide
          Hoss Man added a comment -

          Thinking about it some more ...

          The current behavior of being as "raw" as possible when using a currency field in a function probably makes the most sense – and then we should implement something like SOLR-3239 as a wrapper function people can use when they want to be able to apply functions (or frange) over the converted values.

          so you can choose between...

          q={!frange u=500}price_field
          
            or
          
          q={!frange u=5}currency(price_field,USD)
          
          Show
          Hoss Man added a comment - Thinking about it some more ... The current behavior of being as "raw" as possible when using a currency field in a function probably makes the most sense – and then we should implement something like SOLR-3239 as a wrapper function people can use when they want to be able to apply functions (or frange) over the converted values. so you can choose between... q={!frange u=500}price_field or q={!frange u=5}currency(price_field,USD)
          Hide
          Hoss Man added a comment -

          Updated previous patch to work with trunk, and to include a new currency(field_name,[currency_code]) function like the one i hypothesized in my previous comment.

          still several nocommit's related to the docs to make it clear what's what – but the tests all pass and the code seems complete to me.

          Show
          Hoss Man added a comment - Updated previous patch to work with trunk, and to include a new currency(field_name, [currency_code] ) function like the one i hypothesized in my previous comment. still several nocommit's related to the docs to make it clear what's what – but the tests all pass and the code seems complete to me.
          Hide
          Commit Tag Bot added a comment -

          [trunk commit] Chris M. Hostetter
          http://svn.apache.org/viewvc?view=revision&revision=1452483

          SOLR-4138: CurrencyField fields can now be used in a ValueSources. There is also a new currency(field,[CODE]) function

          Show
          Commit Tag Bot added a comment - [trunk commit] Chris M. Hostetter http://svn.apache.org/viewvc?view=revision&revision=1452483 SOLR-4138 : CurrencyField fields can now be used in a ValueSources. There is also a new currency(field, [CODE] ) function
          Hide
          Hoss Man added a comment -

          Committed revision 1452483.
          Committed revision 1452508.
          Committed revision 1452527.

          Show
          Hoss Man added a comment - Committed revision 1452483. Committed revision 1452508. Committed revision 1452527.
          Hide
          Commit Tag Bot added a comment -

          [branch_4x commit] Chris M. Hostetter
          http://svn.apache.org/viewvc?view=revision&revision=1452527

          SOLR-4138: CurrencyField fields can now be used in a ValueSources. There is also a new currency(field,[CODE]) function (merge r1452483 and r1452508)

          Show
          Commit Tag Bot added a comment - [branch_4x commit] Chris M. Hostetter http://svn.apache.org/viewvc?view=revision&revision=1452527 SOLR-4138 : CurrencyField fields can now be used in a ValueSources. There is also a new currency(field, [CODE] ) function (merge r1452483 and r1452508)
          Hide
          Commit Tag Bot added a comment -

          [trunk commit] Chris M. Hostetter
          http://svn.apache.org/viewvc?view=revision&revision=1452508

          SOLR-4138: doc typos

          Show
          Commit Tag Bot added a comment - [trunk commit] Chris M. Hostetter http://svn.apache.org/viewvc?view=revision&revision=1452508 SOLR-4138 : doc typos
          Hide
          Uwe Schindler added a comment -

          Closed after release.

          Show
          Uwe Schindler added a comment - Closed after release.

            People

            • Assignee:
              Hoss Man
              Reporter:
              Grzegorz Sobczyk
            • Votes:
              0 Vote for this issue
              Watchers:
              1 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development