[
https://issues.apache.org/jira/browse/LUCENE-4619?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13530175#comment-13530175
]
Michael McCandless commented on LUCENE-4619:
--------------------------------------------
bq. You can remove TestDemoFacets from the patch? (since you committed it)
Will do.
bq. DEFAULT_INDEXING_PARAMS is a good idea, but it can change (it's mutable).
However, I plan to add freeze() to it in LUCENE-4621, so maybe update the
nocommit to call freeze() on it?
OK.
bq. Adding the field more than once ... that's tricky. It will of course create
multiple positions, but the entire code now assumes a single position for
facets. Is there any way we can prevent it? I guess not. Maybe put a WARNING
for now?
Yeah I think a big warning in the javadocs is all we can do for now ... would
be nice to somehow catch it but I can't think of a way now.
Separately this duality of dimension/field is sort of confusing. Shouldn't I
be adding FacetField("Author") and FacetField("Publish Date") to my Document?
Instead of a single FacetField taking these two "dimensions" as
CategoryPaths... I know that's a major change but it's still confusing to my
[somewhat] new eyes ...
{quote}
It should be ok to add it more than once, with different FIParams, e.g.
different CLPs.
I guess that's where CDB simplifies things?
{quote}
Right ... I wouldn't say CDB simplifies things (for the "basic" usage).
{quote}
The check that you do in the ctor:
I think it can be simplified to just check FIP.getAllCLPs().size() != 1? If so,
throw the exception
Also note that you have a typo in the exception msg
{quote}
OK good! Hmm that's an Iterable ... and ... I don't really want to iterate
over it if I'm iterating over the CPs anyway ... I think? Or maybe we have it
return a List instead? Hmm but this call (in DefaultFIP) makes a new ArrayList
every time ... we should just use Collections.singletonList here ... I'll fix
that. So what to do?
bq. The rest of the code just freaks the hell out of me!
LOL!! Me too :) Trying to figure out how to just get a byte[] out (so I can
do the DocValuesFacetField) is not easy ...
bq. I think that you need the stream because that's the only way to add the
field with the drill-down term and fulltree posting.
OK.
bq. Would rather if that was simplified ... but I don't want to implement a
different TokenStream, not sure there's much value in it.
This TokenStream somehow adds one position w/ the payload (with token
"fulltree", if no partitioning I think?), and then provides more tokens so they
are indexed for drilldown?
bq. Still, maybe put a TODO (not nocommit) to check if it can be simplified?
OK will do.
{quote}
BTW, when I look at the test, adding the PayloadFacetField is super neat. I
wonder if we simplified CDB to have less API, then all this would still be
required. E.g.:
{noformat}
List<CPath> categories; // you need to prepare that w/ PayloadFacetField too!
Document doc = new Document();
cdb.addFields(doc, categories);
{noformat}
It's not a Field, but it works w/ multiple CLPs and all (basically the code
today). Only you don't need to set the categories. It can also mask in the
future the case where we put the fulltree under one field and the terms under
another, or everything as a DV ... What do you think?
{quote}
I think that's a step in the right direction ... but I think a FacetField is
even easier to consume?
bq. Sorry that I don't fix the patch myself, but I'm rather short on time now.
I can do it later. I'm also going to tackle LUCENE-4621 now.
No prob, I'll fix.
I'm also working on hacked up DocValuesFacetField. Wait til you see how I get
the byte[] to feed to DV :)
> Create a specialized path for facets counting
> ---------------------------------------------
>
> Key: LUCENE-4619
> URL: https://issues.apache.org/jira/browse/LUCENE-4619
> Project: Lucene - Core
> Issue Type: Improvement
> Components: modules/facet
> Reporter: Shai Erera
> Attachments: LUCENE-4619.patch
>
>
> Mike and I have been discussing that on several issues (LUCENE-4600,
> LUCENE-4602) and on GTalk ... it looks like the current API abstractions may
> be responsible for some of the performance loss that we see, compared to
> specialized code.
> During our discussion, we've decided to target a specific use case - facets
> counting and work on it, top-to-bottom by reusing as much code as possible.
> Specifically, we'd like to implement a FacetsCollector/Accumulator which can
> do only counting (i.e. respects only CountFacetRequest), no sampling,
> partitions and complements. The API allows us to do so very cleanly, and in
> the context of that issue, we'd like to do the following:
> * Implement a FacetsField which takes a TaxonomyWriter, FacetIndexingParams
> and CategoryPath (List, Iterable, whatever) and adds the needed information
> to both the taxonomy index as well as the search index.
> ** That API is similar in nature to CategoryDocumentBuilder, only easier to
> consume -- it's just another field that you add to the Document.
> ** We'll have two extensions for it: PayloadFacetsField and
> DocValuesFacetsField, so that we can benchmark the two approaches.
> Eventually, one of them we believe, will be eliminated, and we'll remain w/
> just one (hopefully the DV one).
> * Implement either a FacetsAccumulator/Collector which takes a bunch of
> CountFacetRequests and returns the top-counts.
> ** Aggregations are done in-collection, rather than post. Note that we have
> LUCENE-4600 open for exploring that. Either we finish this exploration here,
> or do it there. Just FYI that the issue exists.
> ** Reuses the CategoryListIterator, IntDecoder and Aggregator code. I'll open
> a separate issue to explore improving that API to be bulk, and then we can
> decide if this specialized Collector should use those abstractions, or be
> really optimized for the facet counting case.
> * At the moment, this path will assume that a document holds multiple
> dimensions, but only one value from each (i.e. no Author/Shai, Author/Mike
> for a document), and therefore use OrdPolicy.NO_PARENTS.
> ** Later, we'd like to explore how to have this specialized path handle the
> ALL_PARENTS case too, as it shouldn't be so hard to do.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@lucene.apache.org
For additional commands, e-mail: dev-h...@lucene.apache.org
