Uploaded image for project: 'Solr'
  1. Solr
  2. SOLR-2523

SolrJ QueryResponse doesn't support range facets

Details

    • Improvement
    • Status: Closed
    • Trivial
    • Resolution: Fixed
    • None
    • 3.4, 4.0-ALPHA
    • clients - java
    • 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.

      Attachments

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

        Activity

          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");
          
          martijn.v.groningen 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" );
          rcmuir 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...

          rcmuir 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...
          rcmuir 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...

          rcmuir 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...

          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.

          martijn.v.groningen 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.

          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.

          martijn.v.groningen 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.
          yseeley@gmail.com 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?

          yseeley@gmail.com 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?

          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?

          martijn.v.groningen 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?
          yseeley@gmail.com 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.

          yseeley@gmail.com 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.

          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.

          martijn.v.groningen 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.
          yseeley@gmail.com 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.

          yseeley@gmail.com 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.

          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?

          hossman Chris M. Hostetter 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?

          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....

          martijn.v.groningen 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....

          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.

          martijn.v.groningen 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.

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

          martijn.v.groningen Martijn van Groningen added a comment - Committed. 3x branch in revision 1150364 trunk in revision 1150365
          rcmuir Robert Muir added a comment -

          bulk close for 3.4

          rcmuir Robert Muir added a comment - bulk close for 3.4

          For generating queries based on range facet results, yes, Solr-1896 will do the right thing.

          For generating labels for the UI, we still need access to DateMathParser on the client side. Shouldn't be too tricky to de-couple from SolrRequestInfo ... (he says hopefully).

          Do you see what I mean?

          creitzel Charles Reitzel added a comment - For generating queries based on range facet results, yes, Solr-1896 will do the right thing. For generating labels for the UI, we still need access to DateMathParser on the client side. Shouldn't be too tricky to de-couple from SolrRequestInfo ... (he says hopefully). Do you see what I mean?

          People

            martijn.v.groningen Martijn van Groningen
            martijn.v.groningen Martijn van Groningen
            Votes:
            1 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

              Created:
              Updated:
              Resolved: