Solr
  1. Solr
  2. SOLR-2523

SolrJ QueryResponse doesn't support range facets

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Trivial Trivial
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 3.4, 4.0-ALPHA
    • Component/s: clients - java
    • Labels:
      None

      Description

      It is possible to get date facets and pivot facets in SolrJ.

      queryResponse.getFacetDate();
      queryResponse.getFacetPivot();
      

      Having this also for range fields would be nice. Adding this is trivial. Maybe we should deprecate date facet methods in QueryResponse class? Since it is superseded by range facets. Also some set / add / remove methods for setting facet range parameters on the SolrQuery class would be nice.

      1. SOLR-2523.patch
        18 kB
        Martijn van Groningen
      2. SOLR-2523.patch
        19 kB
        Martijn van Groningen

        Activity

        Hide
        Martijn van Groningen added a comment -

        Attached patch that adds response parsing for range facets and two methods for adding range facets on SolrQuery.

        On the QueryResponse class:

        List<RangeFacet> facets queryResponse.getFacetRanges()
        

        On the SolrQuery class:

        solrQuery.addNumericRangeFacet("field", 1, 10, 1);
        solrQuery.addDateRangeFacet("field", start, end, "+1MONTH");
        
        Show
        Martijn van Groningen added a comment - Attached patch that adds response parsing for range facets and two methods for adding range facets on SolrQuery. On the QueryResponse class: List<RangeFacet> facets queryResponse.getFacetRanges() On the SolrQuery class: solrQuery.addNumericRangeFacet( "field" , 1, 10, 1); solrQuery.addDateRangeFacet( "field" , start, end, "+1MONTH" );
        Hide
        Robert Muir added a comment -

        Hi Martijn, there are some fun things if you run some of the new tests under a thai locale (our tests randomize across all locales so eventually it will hit this one)

        ant test-core -Dtestcase=SolrQueryTest -Dtests.locale=th_TH_TH
        

        I was trying to ensure the test was correct if the default calendar is not gregorian, but it seems to fail already based on number format...

        Show
        Robert Muir added a comment - Hi Martijn, there are some fun things if you run some of the new tests under a thai locale (our tests randomize across all locales so eventually it will hit this one) ant test-core -Dtestcase=SolrQueryTest -Dtests.locale=th_TH_TH I was trying to ensure the test was correct if the default calendar is not gregorian, but it seems to fail already based on number format...
        Hide
        Robert Muir added a comment -

        Hi Martijn, there are some fun things if you run some of the new tests under a thai locale (our tests randomize across all locales so eventually it will hit this one)

        ant test-core -Dtestcase=SolrQueryTest -Dtests.locale=th_TH_TH
        

        I was trying to ensure the test was correct if the default calendar is not gregorian, but it seems to fail already based on number format...

        Show
        Robert Muir added a comment - Hi Martijn, there are some fun things if you run some of the new tests under a thai locale (our tests randomize across all locales so eventually it will hit this one) ant test-core -Dtestcase=SolrQueryTest -Dtests.locale=th_TH_TH I was trying to ensure the test was correct if the default calendar is not gregorian, but it seems to fail already based on number format...
        Hide
        Martijn van Groningen added a comment -

        Nice catch! I've updated the patch, so for the Thai locale all tests in the patch pass.

        The date formatting worked without changing the code. I'm using Solrj's DateUtil for that.

        Show
        Martijn van Groningen added a comment - Nice catch! I've updated the patch, so for the Thai locale all tests in the patch pass. The date formatting worked without changing the code. I'm using Solrj's DateUtil for that.
        Hide
        Martijn van Groningen added a comment -

        I'm currently adding getAsFilterQuery() method for range facets as described here:
        http://www.mail-archive.com/solr-user@lucene.apache.org/msg51871.html

        I'm currently having a problem with parsing date gaps. DateMathParser is part of Solr core, so I can't use it in SolrJ. I moved it to solr commons, which let me to another issue. The DateMathParser#getNow() method uses SolrRequestInfo which uses a lot of Solr core classes. So I thought I should introduce a CurrentDataSource callback, which decouples the DateMatchParser completely from core. Any thoughts about this approach?

        Maybe we should commit the current patch as it is now and open a new issue for adding getAsFilterQuery() method. We can then open another issue for moving the DateMathParser to commons, which I think is a good idea any way.

        Show
        Martijn van Groningen added a comment - I'm currently adding getAsFilterQuery() method for range facets as described here: http://www.mail-archive.com/solr-user@lucene.apache.org/msg51871.html I'm currently having a problem with parsing date gaps. DateMathParser is part of Solr core, so I can't use it in SolrJ. I moved it to solr commons, which let me to another issue. The DateMathParser#getNow() method uses SolrRequestInfo which uses a lot of Solr core classes. So I thought I should introduce a CurrentDataSource callback, which decouples the DateMatchParser completely from core. Any thoughts about this approach? Maybe we should commit the current patch as it is now and open a new issue for adding getAsFilterQuery() method. We can then open another issue for moving the DateMathParser to commons, which I think is a good idea any way.
        Hide
        Yonik Seeley added a comment -

        I'm currently having a problem with parsing date gaps.

        Hmmm, why do you need to parse the gap? The results returned include absolute times. For consistency, perhaps a filter query should also use those absolute times returned in the response to construct the filter rather than trying to use date math?

        Show
        Yonik Seeley added a comment - I'm currently having a problem with parsing date gaps. Hmmm, why do you need to parse the gap? The results returned include absolute times. For consistency, perhaps a filter query should also use those absolute times returned in the response to construct the filter rather than trying to use date math?
        Hide
        Martijn van Groningen added a comment -

        Maybe I don't get something then. Let's take a look at the following example.

             <lst name="facet_ranges">
              <lst name="manufacturedate_dt">
                <lst name="counts">
                  <int name="2005-02-13T15:26:37Z">4</int>
                  <int name="2006-02-13T15:26:37Z">7</int>
                  <int name="2007-02-13T15:26:37Z">1</int>
                </lst>
                <str name="gap">+1YEAR</str>
                <date name="start">2005-02-13T15:26:37Z</date>
                <date name="end">2009-02-13T15:26:37Z</date>
              </lst>
            </lst>
        

        For the first facet I need the following fq:
        manufacturedate_dt:[2005-02-13T15:26:37Z TO 2006-02-13T15:26:37Z}
        I can get this from parsing the date of the first facet and then the second one. This works fine for this example. However this wouldn't work in all cases. For example when dealing with last facet count or when not all facets are returned and the next facet is a few gaps ahead (for example when facet.mincount=1). I think I do need to use the gap then, right?

        Show
        Martijn van Groningen added a comment - Maybe I don't get something then. Let's take a look at the following example. <lst name= "facet_ranges" > <lst name= "manufacturedate_dt" > <lst name= "counts" > <int name= "2005-02-13T15:26:37Z" > 4 </int> <int name= "2006-02-13T15:26:37Z" > 7 </int> <int name= "2007-02-13T15:26:37Z" > 1 </int> </lst> <str name= "gap" > +1YEAR </str> <date name= "start" > 2005-02-13T15:26:37Z </date> <date name= "end" > 2009-02-13T15:26:37Z </date> </lst> </lst> For the first facet I need the following fq: manufacturedate_dt:[2005-02-13T15:26:37Z TO 2006-02-13T15:26:37Z} I can get this from parsing the date of the first facet and then the second one. This works fine for this example. However this wouldn't work in all cases. For example when dealing with last facet count or when not all facets are returned and the next facet is a few gaps ahead (for example when facet.mincount=1). I think I do need to use the gap then, right?
        Hide
        Yonik Seeley added a comment -

        For example when dealing with last facet count

        I had thought that "end" would deal with this... but it's actually the start of the last range.

        or when not all facets are returned and the next facet is a few gaps ahead

        Ah, right - I hadn't considered mincount>0

        I'm not a date-math expert, but is there a problem with using the gap w/o having to parse it (i.e. can we always append it?)

        manufacturedate_dt:[2007-02-13T15:26:37Z TO 2007-02-13T15:26:37Z+1YEAR]
        

        If the date stuff is too hard to use, we should consider making the interface easier for all clients instead of just trying to deal with the consequences only in SolrJ.

        Show
        Yonik Seeley added a comment - For example when dealing with last facet count I had thought that "end" would deal with this... but it's actually the start of the last range. or when not all facets are returned and the next facet is a few gaps ahead Ah, right - I hadn't considered mincount>0 I'm not a date-math expert, but is there a problem with using the gap w/o having to parse it (i.e. can we always append it?) manufacturedate_dt:[2007-02-13T15:26:37Z TO 2007-02-13T15:26:37Z+1YEAR] If the date stuff is too hard to use, we should consider making the interface easier for all clients instead of just trying to deal with the consequences only in SolrJ.
        Hide
        Martijn van Groningen added a comment -

        I'm not a date-math expert, but is there a problem with using the gap w/o having to parse it (i.e. can we always append it?)

        Good idea! This would be really useful for any client. I think we can change this in SolrQueryParser#getRangeQuery(....) method or in DateField#parseMath(...).

        BTW do you think it is a good idea to commit what we have now here? We can then address appending the gap to a date for range queries and the RangeFacet#getAsFilterQuery() method in other issues.

        Show
        Martijn van Groningen added a comment - I'm not a date-math expert, but is there a problem with using the gap w/o having to parse it (i.e. can we always append it?) Good idea! This would be really useful for any client. I think we can change this in SolrQueryParser#getRangeQuery(....) method or in DateField#parseMath(...). BTW do you think it is a good idea to commit what we have now here? We can then address appending the gap to a date for range queries and the RangeFacet#getAsFilterQuery() method in other issues.
        Hide
        Yonik Seeley added a comment -

        BTW do you think it is a good idea to commit what we have now here? We can then address appending the gap to a date for range queries and the RangeFacet#getAsFilterQuery() method in other issues.

        Sounds fine in general, but if you're looking for a review of the API here, I've never done much with the higher-level (type safe) SolrJ stuff myself.

        Show
        Yonik Seeley added a comment - BTW do you think it is a good idea to commit what we have now here? We can then address appending the gap to a date for range queries and the RangeFacet#getAsFilterQuery() method in other issues. Sounds fine in general, but if you're looking for a review of the API here, I've never done much with the higher-level (type safe) SolrJ stuff myself.
        Hide
        Hoss Man added a comment -

        I'm not a date-math expert, but is there a problem with using the gap w/o having to parse it (i.e. can we always append it?)

        that is exactly how it was designed to be used. But ultimately i really want to implement SOLR-1896 so no client (in any language) ever has to think about any of this.

        Good idea! This would be really useful for any client. I think we can change this in SolrQueryParser#getRangeQuery(....) method or in DateField#parseMath(...).

        i'm a little lost ... I don't understand what "change" is being suggested in this sentence ... can't the client already access both the values and the gap and concat them?

        Show
        Hoss Man added a comment - I'm not a date-math expert, but is there a problem with using the gap w/o having to parse it (i.e. can we always append it?) that is exactly how it was designed to be used. But ultimately i really want to implement SOLR-1896 so no client (in any language) ever has to think about any of this. Good idea! This would be really useful for any client. I think we can change this in SolrQueryParser#getRangeQuery(....) method or in DateField#parseMath(...). i'm a little lost ... I don't understand what "change" is being suggested in this sentence ... can't the client already access both the values and the gap and concat them?
        Hide
        Martijn van Groningen added a comment -

        Solr-1896 looks like what this issue needs. The change that I suggested involves having the datemath syntax in regular Solr query parser. But I think solr-1896 is just fine for this issue. Maybe changing the regular query parser is a bit too much....

        Show
        Martijn van Groningen added a comment - Solr-1896 looks like what this issue needs. The change that I suggested involves having the datemath syntax in regular Solr query parser. But I think solr-1896 is just fine for this issue. Maybe changing the regular query parser is a bit too much....
        Hide
        Martijn van Groningen added a comment -

        I'll commit this patch in the coming day or so. So this is without the getAsFilterQuery() method, since this is best fix in SOLR-1896 for all clients.

        Show
        Martijn van Groningen added a comment - I'll commit this patch in the coming day or so. So this is without the getAsFilterQuery() method, since this is best fix in SOLR-1896 for all clients.
        Hide
        Martijn van Groningen added a comment -

        Committed.
        3x branch in revision 1150364
        trunk in revision 1150365

        Show
        Martijn van Groningen added a comment - Committed. 3x branch in revision 1150364 trunk in revision 1150365
        Hide
        Robert Muir added a comment -

        bulk close for 3.4

        Show
        Robert Muir added a comment - bulk close for 3.4

          People

          • Assignee:
            Martijn van Groningen
            Reporter:
            Martijn van Groningen
          • Votes:
            1 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development