[ 
https://issues.apache.org/jira/browse/SOLR-2894?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Hoss Man updated SOLR-2894:
---------------------------

    Attachment: SOLR-2894.patch


hey guys, stoked to see all these tests passing!

I've been slowly working my way through Andrew's latest patch, reviewing all 
the code and making some tweaks/improvements as I go.  Here's a checkpointed 
update...

{panel}
Patch updates in attachment:

* fix FacetComponent to mirror refactoring done in SOLR-6216 
* fixed up the String.format calls in various classes so they specify 
Locale.ROOT
** removed some useless "toString()" calls in these format calls as well, 
particularly since it looked like they could cause NPEs
* PivotFacetField
** javadocs:
*** createFromListOfNamedLists
*** convertToListOfNamedLists
** eliminate call to {{PivotFacetFieldValueCollection.contains(...)}} (see 
below)
* PivotFacetValue...
** javadocs:
*** class
*** createFromNamedList
*** shardHasContributed
*** convertToNamedList
* PivotFacetFieldValueCollection...
** javadocs:
*** class
*** refinableCollection
*** refinableSubList
*** refinableSize
*** size
*** get
*** add
** remove unused methods
*** isEmpty()
*** getValue(Comparable)
*** contains(Comparable)
**** (this was used, but only in a case where it was immediately followed by a 
call {{get(Comparable)}} so i just optimized it away and replaced it with a 
null check.
** rename: "isSorted" -> "dirty"
** rename: "nullValue" -> "missingValue"
*** it was really confusing because "nullValue" could be null, or it could be a 
PivotFacetValue whose value was null
** fix {{add(PivotFacetValue)}} to set "dirty" directly
** lock down some stuff...
*** methods for accessing some vars so they don't need to be public
*** make some things specified in constructor final
*** make {{refinableCollection}} and {{refinableSubList}} return immutable lists
{panel}


Some things i'm either confused by and/or debating in my head ... 
comments/opinions from others would be apreciated:

* refinement and facet offset
** I haven't looed into this closely, but i noticed the refinement code seems 
to only refine things started at the "facetFieldOffset," of the current 
collection
** don't we need to refine all the values, starting from the beginging of the 
list?
** if if the offset is "1" and the first value X has a count of "100" and the 
second value Y has an initial count of "50" but a post-refinement count of 
"150" pushing itself prior to the offset and putting X into the window, then 
doesn't X miss out on refinement?

* {{refinableCollection()}}
** I think we probably want to rename {{refinableCollection()}} (and 
{{refinableSize()}}) to something more like "{{getExplicitValuesList()}} 
(compared to the {{getMissingValue()}} method I just added) to make it more 
clear what you are really getting form this method ... I recognize that this 
name comes from the fact that we don't ever really need to refine the count for 
the missing value, but that seems like an implementaion detail that doesn't 
affect a lot of places this method is called (and particularly since the 
childPivots of the missing value _do_ still need refined so even when it is 
relevant, it's still missleading from a recursion standpoint.)

* trim
** from what i can understand of the {{trim}} methods - these are typically 
destructive operations that: 
*** should only be called after all refinement is completed
*** prune things that are no longer needed based on the limit/offset params, 
making the objects unusable for any future modifications/refinement so that 
it's only good for...
*** should be called just prior to asking for the final NamedList response 
structure
** if my understanding is correct, then it seems like it might be safer & more 
straight forward to instead just refactor this functionality directly into the 
corrisponding methods for converting to a NamedList, and clearly document those 
methods as destructive?
*** or at the very least add a "trimmed" boolean and sprinkle arround some 
asserts in the various methods related to wether the object has/has not already 
been trimmed



> Implement distributed pivot faceting
> ------------------------------------
>
>                 Key: SOLR-2894
>                 URL: https://issues.apache.org/jira/browse/SOLR-2894
>             Project: Solr
>          Issue Type: Improvement
>            Reporter: Erik Hatcher
>            Assignee: Hoss Man
>             Fix For: 4.9, 5.0
>
>         Attachments: SOLR-2894-mincount-minification.patch, 
> SOLR-2894-reworked.patch, SOLR-2894.patch, SOLR-2894.patch, SOLR-2894.patch, 
> SOLR-2894.patch, SOLR-2894.patch, SOLR-2894.patch, SOLR-2894.patch, 
> SOLR-2894.patch, SOLR-2894.patch, SOLR-2894.patch, SOLR-2894.patch, 
> SOLR-2894.patch, SOLR-2894.patch, SOLR-2894.patch, SOLR-2894.patch, 
> SOLR-2894.patch, SOLR-2894.patch, SOLR-2894.patch, SOLR-2894.patch, 
> SOLR-2894.patch, SOLR-2894.patch, SOLR-2894.patch, SOLR-2894.patch, 
> SOLR-2894.patch, SOLR-2894.patch, SOLR-2894.patch, SOLR-2894.patch, 
> SOLR-2894.patch, SOLR-2894.patch, SOLR-2894.patch, SOLR-2894.patch, 
> SOLR-2894.patch, SOLR-2894.patch, SOLR-2894.patch, SOLR-2894.patch, 
> SOLR-2894.patch, SOLR-2894_cloud_test.patch, dateToObject.patch, 
> pivot_mincount_problem.sh
>
>
> Following up on SOLR-792, pivot faceting currently only supports 
> undistributed mode.  Distributed pivot faceting needs to be implemented.



--
This message was sent by Atlassian JIRA
(v6.2#6252)

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org

Reply via email to