lucene-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Shai Erera (JIRA)" <j...@apache.org>
Subject [jira] [Updated] (LUCENE-5387) Improve FacetConfig.build
Date Wed, 29 Jan 2014 15:46:10 GMT

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

Shai Erera updated LUCENE-5387:
-------------------------------

    Attachment: LUCENE-5387.patch

Patch fixes FacetConfig.build to take and return Document. I had to modify it to take Document
as well because if we take IndexDocument, we cannot add the Indexable/StorableFields to Document,
as it only takes Field. I don't think it is critical that we make FacetConfig that generic
- up until now users had to use FacetFields.addFields and pass Document, so we didn't make
anything worse.

I didn't add CHANGES as this whole change wasn't released yet. However I wonder if we should
mention somewhere that when users port their code to the new API, they don't forget to add
the Document that was returned by FacetConfig.build(), and not the one that they passed in.
I added warnings in the javadocs, but still...

> Improve FacetConfig.build
> -------------------------
>
>                 Key: LUCENE-5387
>                 URL: https://issues.apache.org/jira/browse/LUCENE-5387
>             Project: Lucene - Core
>          Issue Type: Improvement
>          Components: modules/facet
>            Reporter: Shai Erera
>            Assignee: Shai Erera
>         Attachments: LUCENE-5387.patch
>
>
> FacetConfig.build() takes an IndexDocument and returns a new instance of IndexDocument.
This forces you to write code like this:
> {code}
> Document doc = new Document();
> doc.add(new StringField("id", "someID", Store.NO));
> doc.add(new FacetField("author", "john"));
> IndexDocument facetDoc = facetConfig.build(doc);
> indexWriter.addDocument(facetDoc);
> {code}
> Technically, you don't need to declare 'facetDoc', you could just {{indexWriter.addDocument(facetConfig.build(doc))}},
but it's weird:
> * After you call facetConfig.build(), you cannot add any more fields to the document
(since you get an IndexDoc), so you must call it last.
> * Nothing suggests you *should* call facetConfig.build() at all - I can already see users
trapped by the new API, thinking that adding a FacetField is enough. We should at least document
on FacetField that you should call FacetConfig.build().
> * Nothing suggests that you shouldn't ignore the returned IndexDoc from FC.build() -
we should at least document that.
> I think that if FacetConfig.build() took an IndexDocument but returned a Document, that
will at least allow you to call it in whatever stage of the pipeline that you want (after
adding all FacetFields though)...
> I'll post a patch later.



--
This message was sent by Atlassian JIRA
(v6.1.5#6160)

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


Mime
View raw message