[
https://issues.apache.org/jira/browse/SOLR-6351?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14158661#comment-14158661
]
Hoss Man commented on SOLR-6351:
--------------------------------
i haven't done an extensive review, but here's some quick comments/questions
based on skim of the latest patch...
1) this block of new code in PivotFacetProcessor (which pops up twice at diff
points in the patch?) doesn't make any sense to me ... the StatsValues returned
from {{computeLocalStatsValues()}} is totally ignored ?
{noformat}
+ for(StatsField statsField : statsFields) {
+ statsField.computeLocalStatsValues(docSet);
+ }
{noformat}
2) i don't think {{StatsValues.hasValues()}} really makes sense ... we
shouldn't be computing StatsValues against the subset and then adding them to
the response if and only if they {{hasValues()}} -- we should instead be
skipping the computation of the StatsValues completely unless the pivot subset
is non-empty.
this isn't just a question of optimizing away the stats computation -- it's a
very real difference in the fundamental logic. there could be a non-empty set
of documents (ie: pivot count > 0), but the stats we've been asked to compute
(ie: over some field X) might result in a stats count that's 0 (if none of hte
docs in that set have a value in field X) in which case we should still include
the stats in the response.
3) why is a {{CommonParams.STATS}} constant being added? isn't this what
{{StatsParams.STATS}} is for?
4) i'm not really understanding the point of the two new
{{SolrExampleTests.testPivotFacetsStatsNotSupported*}} methods ... what's the
goal behind having these tests?
If nothing else, as things stand right now, these seem like they make really
brittle assumptions about the _exact_ error message they expect -- we should
change them to use substring/regex to sanity check just the key pieces of
information we care about finding in the error message.
We should also assert that the error code on these exceptions is definitely a
4xx error and not a 5xx
> Let Stats Hang off of Pivots (via 'tag')
> ----------------------------------------
>
> Key: SOLR-6351
> URL: https://issues.apache.org/jira/browse/SOLR-6351
> Project: Solr
> Issue Type: Sub-task
> Reporter: Hoss Man
> Attachments: SOLR-6351.patch, SOLR-6351.patch, SOLR-6351.patch,
> SOLR-6351.patch
>
>
> he goal here is basically flip the notion of "stats.facet" on it's head, so
> that instead of asking the stats component to also do some faceting
> (something that's never worked well with the variety of field types and has
> never worked in distributed mode) we instead ask the PivotFacet code to
> compute some stats X for each leaf in a pivot. We'll do this with the
> existing {{stats.field}} params, but we'll leverage the {{tag}} local param
> of the {{stats.field}} instances to be able to associate which stats we want
> hanging off of which {{facet.pivot}}
> Example...
> {noformat}
> facet.pivot={!stats=s1}category,manufacturer
> stats.field={!key=avg_price tag=s1 mean=true}price
> stats.field={!tag=s1 min=true max=true}user_rating
> {noformat}
> ...with the request above, in addition to computing the min/max user_rating
> and mean price (labeled "avg_price") over the entire result set, the
> PivotFacet component will also include those stats for every node of the tree
> it builds up when generating a pivot of the fields "category,manufacturer"
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
