lucene-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Eirik Lygre <eirik.ly...@gmail.com>
Subject Re: Patch for SOLR-3926: A better way to get current sort information from solrj.SolrQuery
Date Fri, 16 Nov 2012 10:56:20 GMT
(I wrote some more comments on the SolrQuery sorting functions in the jira
itself; that should probably have been on the mailing list; sorry about
that).

The current state (described in
https://issues.apache.org/jira/browse/SOLR-3926) is that there is a nice
and clean (thanks, Otis!) little patch to extend SolrQuery's sorting
methods. In the process of writing, testing and discussing that, I've found
a couple of bugs in the original SolrQuery sorting methods, and Yonik had
comments on the entire (current) approach to sort fields.

The current SolrQuery implementations of addSortField()/removeSortField()
use the "sort" parameter string stored in SolrQuery (really the parent
ModifiableSolrParams), parsing it and changing it as needed. The methods
assume that the string has a syntax of "field direction,field
direction,field direction", where comma is used to separate the entries.
While this obviously covers the current use cases (or else they wouldn't
work), it will fail as soon as we get into advanced functional sort, for
example the sort string "sum(x_f, y_f) desc, price asc" would break
SolrQuery.removeSortField() and SolrQuery.getSortFields() due to the comma
inside the function.

An alternative approach is suggested by Yonik: "Splitting on whitespace
isn't likely to work to well with everything we can put in the field list
these days (functions, augmenters, etc).
Why not keep sort information in the client code in symbolic form (i.e. not
a serialized string), manipulate it there, and then set to the SolrQuery
right before submitting it?". That should solve the problems outlined
above. However, it would also probably not work well with a pattern that
works today, where you can actually set the full sort string *as a string*
on the SolrQuery, and then manipulate it. So, that will be a regression
that may not be desirable. One way out would be to have a parallel set of
methods that work on symbolic form: These new functions would not be able
to parse a sort string, but they would handle more complex sort
specifications. As a user of SolrQuery, you would use one, but not both, of
these apis.

think I would hope for the following approach:

- Fist, I complete a patch that fixes the current bugs (with the current
implementation) as well as the enhancement request in SOLR-3926, and we get
that commited.
- Afterwards, if we the people of this list thinks we should extend
SolrQuery with better sort manipulation support, I'll volunteer to write
additional methods as described above.

Comments? And it would be nice to have a committer "signed up" for taking
my patch in, when it's done.

Eirik


On Mon, Nov 12, 2012 at 9:35 PM, Eirik Lygre <eirik.lygre@gmail.com> wrote:

> Our search client needs to render the current search information from a
> SolrQuery object. The existing SolrQuery.getSortFields() method returns an
> array of strings of the form "fieldname direction", which the client must
> parse itself. While this is not very difficult, it is orthogonal to the
> concept used for setting sort information, which is based on methods taking
> two parameters: fieldname and an enum indicating sort direction
> (SolrQuery.ORDER).
>
> The issue is logged as https://issues.apache.org/jira/browse/SOLR-3926. I
> have created a patch with a new method "Map<String, ORDER>
> getSortFieldMap()". This method will parse the current search settings, and
> return a typed object which is easier to work with. The patch is created
> against the branch_4x code.
>
> It would be cool if a commiter could look at the issue and the code, and
> give some feedback, and hopefully promote the code into repository.
>
> If required, I will also create a similar patch for trunk.
>
> --
> Eirik
>
> There is no high like a tango high
> There is no low like a tango low
>



-- 
Eirik

There is no high like a tango high
There is no low like a tango low

Mime
View raw message