[
https://issues.apache.org/jira/browse/SOLR-4212?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Shalin Shekhar Mangar updated SOLR-4212:
----------------------------------------
Attachment: SOLR-6353-4212.patch
{quote}
On the flip side: PivotFacetHelper.mergeQueryCounts does have javadocs, but it
doesn't look like they match what the method does – I'm pretty sure either the
javadocs are wrong, or the method has a bug in it and isn't working as
documented.
{quote}
Fixed. This method was refactored and I forgot to update the javadocs.
{quote}
I've updated your patch to add a request like this to
DistributedFacetPivotLargeTest.
{quote}
Thanks!
{quote}
...wouldn't both of those usages be much simplier if instead of this Map there
was just a List<RangeFacetRequest> that could be iterated over when the caller
cares about looping over all RangeFacetRequest instances?
{quote}
I changed the Map to a List.
{quote}
I think you should revive your earlier idea of having a "FacetContext" object
to put in the request context and use it to track the RangeFacetRequests and
FacetBase (FACET_QUERY) objects.
{quote}
I have created a new FacetContext class which holds all parsed range facets and
query facets and also has a map of tag vs list of parsed facets such that we
can do quick lookups by tag.
{quote}
(Either that, or just use the existing FacetInfo class for this since that's
pretty much it's current purpose. Is the reason you didn't already do that
because FacetInfo isn't currently in the single node code path?)
{quote}
Yeah, that’s pretty much it. The current FacetInfo is exclusively used for
merging distributed results which makes it a bit inflexible for our
requirements.
{quote}
The way RangeFacetProcessor deals with FacetRangeOther seems really confusing
and sketchy. It took me a while to understand why those Exceptions were "safe
to ignore"
{quote}
I refactored as you suggested. This is much cleaner, thank you!
{quote}
I added a nocommit here to ensure we don't forget to file a jira to track this.
It's not just an issue of localparams, but also the parsing & construction of
the Query object – doing that once per request (just like the way you've
changed the range faceting to do all the bucket calculation once) could give us
some serious performance savings when facet.query params are hung of of many
and/or large pivot trees. (should just mean a pretty trivial new subclass of
FacetBase that also has a public Query getQuery() accessor)
{quote}
I opened SOLR-7753
{quote}
It's really weird that none of the public accessor methods (getStart, getEnd,
getGap, getGapObj, etc...) have any javadocs, even though many of the protected
variables they return do have jdocs (and the info in those jdocs seems pretty
important to the method callers – like the distinction betwen end vs endObj).
{quote}
Done. Initially I had kept the fields as public final but later I changed them
to private and added accessor methods. Didn’t remember to move the javadocs. I
have added copious amounts of javadocs to each one of them now.
{quote}
The method signature & jdocs for createCalculator look dangerous
{quote}
You’re right. I changed it to a private method which operates on “this”.
{quote}
There's a weird bit of "format and immediately reparse" logic in
RangeFacetRequest + RangeEndpointCalculator that makes no sense to me
{quote}
Fixed, as you suggested.
{quote}
PivotFacetProcessor.addPivotQueriesAndRanges concerns...
this method was doing a lot of null checking against the facetQueries and
facetRanges Lists – even though there was never any possibility of those lists
being null
(partly because of these null checks that always passed) things like
SimpleFacets instances and RangeFacetProcessors were getting constructed for
every Pivot value (even on refinement) even when there are no ranges or facet
queries hanging off of a pivot.
So in the updated patch I also cleaned this method up a bit to assert the lists
are non null and keep the logicc minimal when they are empty. (had to try it
myself to be certain i wans't missing anything)
I also tightened up the error handling to be more specific about where the
SyntaxErrors are coming from.
{quote}
Thank you, looks really good.
I think this is ready. Please give it another look and let me know if it's good
to go in.
> Let facet queries hang off of pivots
> ------------------------------------
>
> Key: SOLR-4212
> URL: https://issues.apache.org/jira/browse/SOLR-4212
> Project: Solr
> Issue Type: Sub-task
> Components: search
> Affects Versions: 4.0
> Reporter: Steve Molloy
> Assignee: Shalin Shekhar Mangar
> Fix For: 5.3, Trunk
>
> Attachments: SOLR-4212-multiple-q.patch, SOLR-4212-multiple-q.patch,
> SOLR-4212.patch, SOLR-4212.patch, SOLR-4212.patch, SOLR-4212.patch,
> SOLR-4212.patch, SOLR-4212.patch, SOLR-6353-4212.patch, SOLR-6353-4212.patch,
> SOLR-6353-4212.patch, SOLR-6353-6686-4212.patch, SOLR-6353-6686-4212.patch,
> SOLR-6353-6686-4212.patch, SOLR-6353-6686-4212.patch,
> SOLR-6353-6686-4212.patch, patch-4212.txt
>
>
> Facet pivot provide hierarchical support for computing data used to populate
> a treemap or similar visualization. TreeMaps usually offer users extra
> information by applying an overlay color on top of the existing square sizes
> based on hierarchical counts. This second count is based on user choices,
> representing, usually with gradient, the proportion of the square that fits
> the user's choices.
> The proposition is to use local parameters to specify facet query to apply
> for pivot which matches a tag set on facet query. Parameter format would look
> like:
> facet.pivot={!query=r1}category,manufacturer
> facet.query={!tag=r1}somequery
> facet.query={!tag=r1}somedate:[NOW-1YEAR TO NOW]
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
