Issue Details (XML | Word | Printable)

Key: CODEBASE-215
Type: Bug Bug
Status: Closed Closed
Resolution: Fixed
Priority: Major Major
Assignee: Jonathan Rochkind
Reporter: Jonathan Rochkind
Votes: 0
Watchers: 0
Operations

Clone this issue
If you were logged in you would be able to see more operations.
Blacklight Plugin

change how facet limits are supplied to solr

Created: 16/Apr/10 11:26 AM   Updated: 29/Apr/10 02:20 PM
Return to search
Issue 18 of 72 issue(s)
<< Previous | CODEBASE-215 | Next >>
Component/s: None
Affects Version/s: 2.4
Fix Version/s: 2.5


 Description  « Hide
The current way of supplying facet limits to Solr in BL will fail if the facet value being used as a limit has chars in it that need to be escaped. Erik Hatcher suggests a better way as below:

jrochkind: erikhatcher: ping?
[12:14pm] jrochkind: erikhatcher: if you see this. A couple times we've talked about changing the way BL supplies facet limits, using the {} syntax thing that I don't completely understand, to deal with escaping issues etc. If you file a JIRA ticket in the BL jira, assign it to me, and supply a couple lines of detials (like a sample of what it should look like), that would be very helpful!
[12:22pm] erikhatcher: jrochkind: you mean instead of doing fq=some_facet:"some crazy value"?
[12:22pm] jrochkind: i think so, yeah. I have trouble keeping track of the details, is why I could use a JIRA, heh.
[12:22pm] erikhatcher: yes, do this instead: fq={!raw f=some_facet}some crazy value
[12:22pm] jrochkind: yes, that's it!
[12:23pm] jrochkind: you can still have multiple fq's?
[12:23pm] erikhatcher: it's in my preconf slides, i posted those, right?
[12:23pm] erikhatcher: yeah, all that does is change how the query string is parsed
[12:23pm] jrochkind: Probably. If you don't have time, that's cool, but if you can make a JIRA in BL jira and assign it to me, it would be super helpful.
[12:23pm] jrochkind: I guess I can do it myself right now and just put the cut-and-paste of this IRC log in there, heh.
[12:23pm] jrochkind: okay, I'll do that.

 All   Comments   Change History      Sort Order: Ascending order - Click to sort in descending order
Jonathan Rochkind added a comment - 19/Apr/10 10:38 AM
Afraid I can't figure out how to do this. May require some changes to rsolr? Or other parts of Blacklight? Can't figure out how to send rsolr multiple "fq" parameters that it will simply send to solr.

Any advice, Matt or anyone else?

Jonathan Rochkind added a comment - 19/Apr/10 11:36 AM
So I actually did find the part of Blacklight code to be easily modified for this. ApplicationHelper#solr_search_params. (A method that is getting rather HUGE and should probably be broken up at some point, but not this point!).

That method is already doing a whole bunch of Solr request param mapping. So doing one more piece here is not out of place. It's just one more piece BL will do itself instead of relying on rsolr-ext magic, making me and Erik and happy, making the code somewhat more transparent, and using the Erik-recommended way of sending fq to avoid escaping issues.

ApplicationHelper#solr_search_params will generate Solr "fq" as appropriate, using the {!raw} format, instead of sending :phrase_filters to rsolr-ext.

If anyone has a concern with this, let me know, otherwise I'll commit if there seem to be no concerns? It's just a few lines of code, and a few rspec tests changed to spec fq instead of phrase_filters.

Jonathan Rochkind added a comment - 19/Apr/10 12:11 PM
More from Erik on why switching to {!raw} here really makes sense:

(1:06:39 PM) erikhatcher: it is because of escaping... because that is ugly. but also because drilling into a facet generally should simply be a TermQuery ultimately
(1:06:53 PM) jrochkind: so escaping, plus possibly somewhat more efficient execution?
(1:07:16 PM) erikhatcher: and using the lucene query parser with field:"whatever \" you want" is potentially mangling what you meant
(1:07:31 PM) jrochkind: mangling in ways that do not have to do with escaping? Weird.
(1:07:45 PM) erikhatcher: it is analyzing the text, dealing with position stuff as a phrase query, etc
(1:08:11 PM) erikhatcher: you don't need analysis involved when it's a direct exact term value you're querying for

Jonathan Rochkind added a comment - 19/Apr/10 12:31 PM
Changed and tested in: http://github.com/jrochkind/blacklight/tree/better_solr_fq

I'll give it a day or two to see if anyone has concerns, if not, I'll commit (can always be revert'ed if for some reason it's a problem)

Jonathan Rochkind added a comment - 29/Apr/10 02:20 PM
Commitred to master.