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 Sat, 17 Nov 2012 10:41:46 GMT
On Fri, Nov 16, 2012 at 4:57 PM, Yonik Seeley <yonik@lucidworks.com> wrote:

> On Fri, Nov 16, 2012 at 11:56 AM, Eirik Lygre <eirik.lygre@gmail.com>
> wrote:
> > 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.
>
> That seems reasonable to me.  Something like this perhaps?
>
>   Map<String, SORT_ORDER> getSort()
>   setSort(Map<String, SORT_ORDER>)
>   String getSortString()
>   void setSortString(String sortString)  // throw exception if the
> internal Map isn't null?
>
> and add/removeSortField would only work on the internal Map (and throw
> an exception if setSortString had been used?)
>

A couple of thoughts:

1) The old methods work on the serialized string. If we want to maintain
backward compatibility (do we?), we can't really change that, because that
supports a use case where you first set the full sort string, and then
start manipulating them.

2) I'm not sure about the map-idea, but not all against it either :-)

a) If we expose the Map as the api, we should probably specify that this is
an ordered map; otherwise, people will make the common mistake of using a
HashMap, inserting values in the order they want fields sorted, and get the
unexpected result. As far as I know, that means specifying LinkedHashMap,
right?

b) We also need to decide on the point of serialization, and the mutability
of the map returned from getSort().
- If it is mutable, I expect to be able to say
"query.getSort().put("field", ORDER.asc). For that to work, we need to have
a clear serialization entry point, and I haven't found that.
- If it is immutable, there will be some amount of copying maps which
really isn't needed, perhaps

c) I'm not sure about the value of the exception from setSortString;
set(CommonParams.SORT, str) would presumably bypass that anyway, unless we
override that, too. And, if we are to serialize the string as the map
changes (which I think we might have to), we'll probably need some local
(but simple) bookkeeping in order to keep track of the source of the sort
string (symbolic vs raw api).

If we were to maintain backward compatibility, I would think of something
like this:

  // 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);

  // Raw string api, shortcuts to get()/set() with CommonParams.SORT
  void setSortString(String sortString)
  String getSortString()

  // @Deprecated api, parses the serialized string
  setSortField(String, ORDER)
  addSortField(String, ORDER)
  removeSortField(String, ORDER)
  String[] getSortFields()
  String getSortField()  // Same as getSortString()

If we skip backward compatibility (e.g. no use case with setting the string
and then manipulating it), we get off easier, much like your suggestion:

  // Raw string api, shortcuts to get()/set() with CommonParams.SORT
  void setSortField(String sortString)  // New
  String getSortField()

  // Functional, symbolic api, changes the serialized string
  Map<String, ORDER> getSort()  // Returns immmutable, ordered map
  setSort(LinkedHashMap<>)
  setSortField(String, ORDER)
  addSortField(String, ORDER)
  removeSortField(String, ORDER)
  String[] getSortFields() // Array of serialized entries

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?

-- 
Eirik

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

Mime
View raw message