You are viewing a plain text version of this content. The canonical link for it is here.
Posted to solr-dev@lucene.apache.org by patrick o'leary <po...@aol.com> on 2007/11/20 03:01:10 UTC

QueryParsing.SortSpec

Hey

Is there a reason that QueryParsing.SortSpec has private constructors?
It's preventing me from having a custom sort parser for the ResponseBuilder
without a patch for solr.

Thanks
P
-- 

Patrick O'Leary

You see, wire telegraph is a kind of a very, very long cat. You pull his tail in New York and his head is meowing in Los Angeles.
 Do you understand this? 
And radio operates exactly the same way: you send signals here, they receive them there. The only difference is that there is no cat.
  - Albert Einstein

View Patrick O Leary's LinkedIn profileView Patrick O Leary's profile
<http://www.linkedin.com/in/pjaol>

Re: QueryParsing.SortSpec

Posted by Chris Hostetter <ho...@fucit.org>.
: While we are at it, we should limit consolidate the null sort checking.  See:
: http://www.nabble.com/Sorting-problem-tf4762114.html#a13627492
: 
: Currently for "score desc", the parsing utility returns a null SortSpec, then
: has to create a new one to attach rows/offset

+1 ... there's no reason the SolrSpec needs to be null, it can contain a 
null "Sort" object in the "score desc" case (or it can contain Sort.SCORE 
and the SOlrIndexSearcher can be the single place that optimizes that into 
a null sort)


-Hoss


Re: QueryParsing.SortSpec

Posted by Ryan McKinley <ry...@gmail.com>.
> 
> since 99% of hte use cases are pulling sort, rows, and start from some 
> SOlrParams, that use case should probably be refactored into a utility 
> method as well.
> 

While we are at it, we should limit consolidate the null sort checking. 
  See: http://www.nabble.com/Sorting-problem-tf4762114.html#a13627492

Currently for "score desc", the parsing utility returns a null SortSpec, 
then has to create a new one to attach rows/offset

ryan

Re: QueryParsing.SortSpec

Posted by Chris Hostetter <ho...@fucit.org>.
: I think it does make sense to keep them together.
: offset and length only make sense if an ordering is specified.

true.

: If we change the name, we should also move it to a top-level class
: (from a static inner).
: Any suggestions?

good point .. promoting it to first order is probably more important then 
renaming.  i don't have any good suggestions for a better name ... i nthe 
back of my mind the word "slice" keeps coming up since it effectively 
decides which "slice" of a DocList/DocSet you get in the result ... 
(SliceSortSpec?) but that may just be me.

since 99% of hte use cases are pulling sort, rows, and start from some 
SOlrParams, that use case should probably be refactored into a utility 
method as well.


-Hoss


Re: QueryParsing.SortSpec

Posted by patrick o'leary <po...@aol.com>.
I don't see an issue with retaining it, as long as it's got public
constructors.
The difference between
responseBuilder.setSort(customSort);  // if you deprecate SortSpec.
and
responseBuilder.setSortSpec (new QueryParser.SortSpec(customSort, -1));

isn't going to kill anyone.

P

Yonik Seeley wrote:
> On Nov 19, 2007 10:26 PM, Chris Hostetter <ho...@fucit.org> wrote:
>   
>> Last time i looked at most usages, the getOffset() and getCount() were
>> totally ignored
>>     
>
> Not any more... see QueryComponent.process():
>
>       results.docList = searcher.getDocList(
>           builder.getQuery(), builder.getFilters(),
> builder.getSortSpec().getSort(),
>           builder.getSortSpec().getOffset(), builder.getSortSpec().getCount(),
>           builder.getFieldFlags() );
>
>   
>> A quick grep indicates that the new QParser and QueryComponent stuff
>> (which i still haven't had time to look at) seems to be using it, but i
>> can't tell if that's just because that's what's parseSort returns, or if
>> it's becuse it's actually useful to have those three values bundled in
>> that way.
>>     
>
> I think it does make sense to keep them together.
> offset and length only make sense if an ordering is specified.
>
>   
>> if we're going to keep it, let's at least put some mutators on it ... and
>> maybe consider changing the name.
>>     
>
> If we change the name, we should also move it to a top-level class
> (from a static inner).
> Any suggestions?
>
> -Yonik
>
>   

-- 

Patrick O'Leary


You see, wire telegraph is a kind of a very, very long cat. You pull his tail in New York and his head is meowing in Los Angeles.
 Do you understand this? 
And radio operates exactly the same way: you send signals here, they receive them there. The only difference is that there is no cat.
  - Albert Einstein

View Patrick O Leary's LinkedIn profileView Patrick O Leary's profile
<http://www.linkedin.com/in/pjaol>

Re: QueryParsing.SortSpec

Posted by Yonik Seeley <yo...@apache.org>.
On Nov 19, 2007 10:26 PM, Chris Hostetter <ho...@fucit.org> wrote:
> Last time i looked at most usages, the getOffset() and getCount() were
> totally ignored

Not any more... see QueryComponent.process():

      results.docList = searcher.getDocList(
          builder.getQuery(), builder.getFilters(),
builder.getSortSpec().getSort(),
          builder.getSortSpec().getOffset(), builder.getSortSpec().getCount(),
          builder.getFieldFlags() );

> A quick grep indicates that the new QParser and QueryComponent stuff
> (which i still haven't had time to look at) seems to be using it, but i
> can't tell if that's just because that's what's parseSort returns, or if
> it's becuse it's actually useful to have those three values bundled in
> that way.

I think it does make sense to keep them together.
offset and length only make sense if an ordering is specified.

> if we're going to keep it, let's at least put some mutators on it ... and
> maybe consider changing the name.

If we change the name, we should also move it to a top-level class
(from a static inner).
Any suggestions?

-Yonik

Re: QueryParsing.SortSpec

Posted by Chris Hostetter <ho...@fucit.org>.
: Hmmm, no reason I see.
: Unless there are objections, I'll make them public.

-0 ... i'd much rather see us deprecate it and make it go away entirely.

it's an archaic throwback to the days when every option was going to 
be encoded into the "q" param instead of having "sort" "rows" and "start"   
(ie: q = solr rocks; price asc, score desc, skip 50, top 10 ... or 
something like that)

Last time i looked at most usages, the getOffset() and getCount() were 
totally ignored and the only reason any class refered to SortSpec was 
because they had to (since there is no parseSort that just returned a 
Sort object)

A quick grep indicates that the new QParser and QueryComponent stuff 
(which i still haven't had time to look at) seems to be using it, but i 
can't tell if that's just because that's what's parseSort returns, or if 
it's becuse it's actually useful to have those three values bundled in 
that way.

if we're going to keep it, let's at least put some mutators on it ... and 
maybe consider changing the name.



-Hoss


Re: QueryParsing.SortSpec

Posted by Yonik Seeley <yo...@apache.org>.
On Nov 19, 2007 9:01 PM, patrick o'leary <po...@aol.com> wrote:
>  Is there a reason that QueryParsing.SortSpec has private constructors?
>  It's preventing me from having a custom sort parser for the ResponseBuilder
>  without a patch for solr.

Hmmm, no reason I see.
Unless there are objections, I'll make them public.

-Yonik