[jira] [Updated] (SOLR-3218) Range faceting support for CurrencyField

["Hoss Man (JIRA)" <ji...@apache.org>]

     [ https://issues.apache.org/jira/browse/SOLR-3218?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Hoss Man updated SOLR-3218:
---------------------------
    Attachment: SOLR-3218.patch

I'm going to try to see this through -- but at this point I don't think it's a good idea to add {{facet.range}} support w/o also adding {{json.facet}} {{type:range}} support as well -- so i'm working on that at the same time.

Here's what i've got so far...

* massaged the latest path to apply to master
** along the way i droped the StatsValuesFactory changes since:
*** they were causing a lot of headaches
*** they weren't directly related to the objective
*** they didn't have any distrib tests and i was 99% certain at a glance that the distrib stats code wasn't going to work as is since it seemed to expect {{CurrencyValue}} objects from the shards
* beefed up the existing tests a bit and added some cloud based test equivilents
* adapted the minimal changes to the {{json.facet}} {{FacetRange}} class...
** added a new {{CurrencyCalc}} impl
** added a new {{buildRangeLabel}} method to the {{Calc}} API since the existing logic (to always use the "low" value as the {{Range.label}} ) isn't viable for {{CurrencyCalc}} because the {{CurrencyValue}} objects aren't suitable for being included directly in the response writer.
** added some very basic variants on the (new) tests for the {{json.facet}} code.

There are still more tests I want to write related to the {{json.facet}} code -- partly because of how {{type:range}} can be used in broader ways in the {{json.facet}} then the equivilent {{facet.range}} code (see nocommits in the test) ... but more notably because of the following points of confusion I have about the code in the existing patch that I'm hoping Yonik can provide some guidance on...


# The {{Calc}} API includes 2 "bits" related methods whose purpose I don't understand and at a glance I don't see used anywhere.  I'm not sure if they are dead code, but with these impls in {{CurrencyCalc}} that do nothing but throw Exceptions I'm not getting any errors -- so if they aren't dead code then they only get used via some code path I haven't yet identified/tested...{code}
    @Override
    public Comparable bitsToValue(long bits) {
      throw new RuntimeException("nocommit: dead code??"); // return bits;
    }

    @Override
    public long bitsToSortableBits(long bits) {
      throw new RuntimeException("nocommit: dead code??"); // return bits;
    }
{code}
# even though the {{FacetRange.Range}} class already has an existing {{Object label}} variable that's used in most places as the {{"val"}} when generating the {{"buckets"}} SimpleOrderedMap for the response, there is one code path in the {{refineBucket}} method that goes out of it's way to use {{FacetRange.Range.low}} instead of {{FacetRange.Range.label}} with a perplexing comment...{code}
bucket.add("val", range.low); // use "low" instead of bucketVal because it will be the right type (we may have been passed back long instead of int for example)
{code}
#* It's not clear to me what the meaning of this comment is and how/why using {{range.low}} is better then using {{range.label}} here since in every (pre-patch) code path I see {{range.low == range.label}}
#** When would the {{label}} ever have the wrong type?!?!?
#* As things stand right now, the {{CurencyValue}} objects that get put into the {{range.low}} variable (when {{CurrencyCalc}} is used) should never be included directly in the response -- and yet nothing in my currency cloud test seems to trigger this code path (yet)
#* Given that the "buckets" of a range facet should always be deterministic, I'm not actually clear on when/how/why a FacetRange bucket would ever need to be "refined" ?


[~yonik@apache.org] can you help shed some light on when/how these methods are used to save me some time in figuring out how to write additional test cases explicitly targeting these code paths?


> Range faceting support for CurrencyField
> ----------------------------------------
>
>                 Key: SOLR-3218
>                 URL: https://issues.apache.org/jira/browse/SOLR-3218
>             Project: Solr
>          Issue Type: Improvement
>          Components: Schema and Analysis
>            Reporter: Jan Høydahl
>         Attachments: SOLR-3218-1.patch, SOLR-3218-2.patch, SOLR-3218.patch, SOLR-3218.patch, SOLR-3218.patch, SOLR-3218.patch, SOLR-3218.patch, SOLR-3218.patch, SOLR-3218.patch
>
>
> Spinoff from SOLR-2202. Need to add range faceting capabilities for CurrencyField



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@lucene.apache.org
For additional commands, e-mail: dev-help@lucene.apache.org