[ 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