RE: Reverse sort facet query [SOLR-1672]

[Chris Hostetter <ho...@fucit.org>]

: Yes, I thought about adding some 'new syntax', but I opted for a separate 'facet.sortorder' parameter,
: 
: mainly because I'm not familiar enough with the codebase to know what effect this might have on
: 
: backward compatibility. It would be easy enough to modify the patch I created to do it this way.

it shouldn't really affect anything -- it wouldn't really be new syntax, 
just extending hte existing "sort" param syntax to apply to the 
"facet.sort" param.  The only back compat concern is making sure we 
continue to support true/false as aliases, and having the default order 
match the current bahvior if asc/desc aren't specified.


-Hoss

RE: Reverse sort facet query [SOLR-1672]

[Chris Hostetter <ho...@fucit.org>]

: i.e. just extend facet.sort to allow a 'count desc'. By convention, ok 
: to use a a space in the name? - or would count.desc (and count.asc as 
: alias for count) be more compliant?

i would use space to remain consistent with the existing "sort" 
param. 

it might even make sense to refactor (re/ab)use the existing "sort" 
parsing code in QueryParsing.parseSort ... but now that that also know 
about parsing functions it's a bit hairry ... so that does seem a little 
crazy.




: 
:  
: 
: Peter
:  
: 
:  		 	   		  
: _________________________________________________________________
: We want to hear all your funny, exciting and crazy Hotmail stories. Tell us now
: http://clk.atdmt.com/UKM/go/195013117/direct/01/



-Hoss

RE: Reverse sort facet query [SOLR-1672]

[Peter S <pe...@hotmail.com>]

> 
> now i'm totally confused: what are you suggesting this new param would 
> do, what does the name mean?
> 
Sorry, I wan't clear - there isn't a new parameter, except the one added in the patch. What I was suggesting here is to do the work
to remove the new parameter I just put in (facet.sortorder), and do it in exactly the way you mentioned - 
i.e. just extend facet.sort to allow a 'count desc'. By convention, ok to use a a space in the name? - or would count.desc (and count.asc as alias for count) be more compliant?

 

Peter
 

 		 	   		  
_________________________________________________________________
We want to hear all your funny, exciting and crazy Hotmail stories. Tell us now
http://clk.atdmt.com/UKM/go/195013117/direct/01/

RE: Reverse sort facet query [SOLR-1672]

[Chris Hostetter <ho...@fucit.org>]

: working well. The only caveat to this is that the reverse sort results
: don't include 0-count facets (see notes in SOLR-1672), so reverse sort 
	...
: believe patching to include 0 counts could open a can of worms in terms 
: of b/w compat and performance, as 0 counts look to be skipped (by 
: default). I could be wrong, and you may know better how changes to 

Hmmm... that behavior should all be driven by facet.mincount.  i haven't 
look at that code in a long time, so an optimization may have been added 
to not bother trying to "sort" all of the 0s ... but the default for 
facet.mincount is 0 (ie: show everything)

: Would you like me to go ahead and amend the patch (w/o 0-counts) to define a new 'sort' parameter? 
: 
: For naming, I would propose an extension of FacetParams.FACET_SORT_COUNT ala:
: 
: public static final String FACET_SORT_COUNT_REVERSE = "count.reverse";

now i'm totally confused: what are you suggesting this new param would 
do, what does the name mean?

my original point was that we probably don't need any new params at all: 
just change facet.sort to accept "count desc" and "count asc" in addition 
to "count" (which would become an alias for "count desc").


-Hoss

RE: Reverse sort facet query [SOLR-1672]

[Peter 4U <pe...@hotmail.com>]

 

> Date: Sun, 3 Jan 2010 22:18:33 -0800
> From: hossman_lucene@fucit.org
> To: solr-user@lucene.apache.org
> Subject: RE: Reverse sort facet query [SOLR-1672]
> 
> 
> : Yes, I thought about adding some 'new syntax', but I opted for a separate 'facet.sortorder' parameter,
> : 
> : mainly because I'm not familiar enough with the codebase to know what effect this might have on
> : 
> : backward compatibility. It would be easy enough to modify the patch I created to do it this way.
> 
> it shouldn't really affect anything -- it wouldn't really be new syntax, 
> just extending hte existing "sort" param syntax to apply to the 
> "facet.sort" param. The only back compat concern is making sure we 
> continue to support true/false as aliases, and having the default order 
> match the current bahvior if asc/desc aren't specified.
> 
> 
> -Hoss
> 


Yes, agreed. The current patch doesn't touch the b/w true/false aliasing, and any move to adding a new attr can keep all that intact.

I've been using the current patch extensively in our testing, and that's working well. The only caveat to this is that the reverse sort results

don't include 0-count facets (see notes in SOLR-1672), so reverse sort results start with the first count=1. This could be confusing as

there could well be many facets whose count is 0, and it might be expected that these be returned in the first instance.

>From my admittedly cursory look into the codebase regading this, I believe patching to include 0 counts could open a can of worms in terms

of b/w compat and performance, as 0 counts look to be skipped (by default). I could be wrong, and you may know better how changes to SimpleFacets/UnInvertedField would affect performance and compatibility.

If there is indeed a performance optimization in facet counting iteration, it would, imo, be preferable to have the optimization, rather than the 0-counts.

 

Would you like me to go ahead and amend the patch (w/o 0-counts) to define a new 'sort' parameter? 

For naming, I would propose an extension of FacetParams.FACET_SORT_COUNT ala:

 

public static final String FACET_SORT_COUNT_REVERSE = "count.reverse";

 

I can then easily modify the patch to detect/use this value to invoke the new behaviour.

Comments? Suggestions?

 

Thanks,

Peter

 

 

 

 
 		 	   		  
_________________________________________________________________
Have more than one Hotmail account? Link them together to easily access both
 http://clk.atdmt.com/UKM/go/186394591/direct/01/