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 Wed, 21 Nov 2012 22:00:52 GMT
I have added a proposed patch to the jira issue, at
https://issues.apache.org/jira/browse/SOLR-3926?focusedCommentId=13502346&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-13502346
.

The proposed satisfies my original needs, using a symbolic api as proposed
by Yonik. I'm hoping this is now sufficiently well developed to be applied
to both the 4x-branch and the trunk moving forward.


On Mon, Nov 19, 2012 at 5:09 PM, Eirik Lygre <eirik.lygre@gmail.com> wrote:

>
>
>
> On Mon, Nov 19, 2012 at 3:23 PM, Eirik Lygre <eirik.lygre@gmail.com>wrote:
>>
>>
>> On Sat, Nov 17, 2012 at 11:41 AM, Eirik Lygre <eirik.lygre@gmail.com>wrote:
>>
>>>
>>>   // Functional, symbolic api. Changes are serialized immediately
>>>   Map<String, ORDER> getSort()  // Returns immmutable, ordered map
>>>   setSort(LinkedHashMap<S,O>);
>>>   addSort(String, ORDER);
>>>   removeSort();
>>>   removeSort(String);
>>>
>>> So, a couple of questions out of this:
>>> - Do we want to maintain backward compatibility  i.e. the use case with
>>> setting the string and then manipulating it?
>>> - Is there a clear point for serialization, so that we can serialize "at
>>> the end" only?
>>>
>>
>> Two comments for the api:
>>
>> 1) There is a use case that might be fairly common, in which first a
>> number of sorts are set, and then the user wants to change the direction of
>> that sort. Presumably this happens over time, and over several user
>> interactions, but it would probably look like this:
>>
>> q.addSort ("a", ASC).addSort("b", ASC).addSort("c", ASC);
>> // Now change the sort order of a, while maintaining a as the first field
>> in the sort list
>> q.setSort("a", DESC); // Don't like; setSort() feels like it should
>> remove others
>> q.toggleSort("a"); // Works, but I'd prefer an api where I decide the
>> direction (e.g. not toggle)
>> q.toggleSort("a", DESC) // Works, but the word toggle is not really good
>> q.changeSort("a", DESC); // Works, though it is not clear what to do if
>> "a" is not already in the sort list
>> q.changeOrAddSort("a", DESC); // This is my preferred semantics, but what
>> about the method name?
>> q.setSort(new LinkedHashMap(q.getSort())
>>
>> 2) Also, I'm not sure I like the method name "removeSort()" to remove
>> all. Either "removeAllSorts()" or "clearSort()", with a preference to the
>> latter (it matches "clear()" which is used to clear the entire
>> SolrQuery/ModifiableSolrParams)
>>
>> // Functional, symbolic api. Changes are serialized immediately
>> clearSort();
>> Map<String, ORDER> getSort()  // Returns immutable, ordered map
>>  setSort(LinkedHashMap<S,O>);
>> addSort(String, ORDER);
>> changeOrAddSort (String, ORDER)
>> removeSort(String);
>>
>
> Attached is a patch; I'll put it up on jira once we're kinda happy with it
> :-)
>
> 1) I renamed the new method (#1 above) to "addOrUpdateSort()". Probably
> not a particularly good parallel, but I found
> org.apache.lucene.index.FieldInfos.addOrUpdate(), and reused that convention
> 2) I removed the new raw string methods getSortString() and
> setSortString(). The point of this exercise is to use a symbolic api, and
> if you want the raw field, use get()/set() with CommonParams.SORT.
>
> The attached patch now contains a few fixes to the old api, tests for
> those fixes, the new api with javadoc, and tests for the new api.
>
>
> --
> 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