[ https://issues.apache.org/jira/browse/OAK-2511?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15059743#comment-15059743 ]
Tommaso Teofili commented on OAK-2511: -------------------------------------- bq. Property definition name - Instead of facets it might be better to use faceted they both sound ok for me, if you prefer _faceted_ I'm fine. bq. Facets config - Currently there is one secureFacets. Would we later expose more config to ow facets are managed? If yes then have a node facets and then move this config under that that may be possible, so I agree, better to have a dedicated structure from day 1. bq. facetsConfig is static. It does not look like to be stateless right, I'm aware we have to persist that and get the values from there, maybe under the dedicated config node you proposed. bq. Move facet field name logic to FieldNames class +1 bq. Better to move this logic to PropertyDefinition#validate and have it as stats set. Easier to update later! I'm not sure how it'd work there. It looks like if we move it there we couldn't have facets on all props (via regex) except the forbidden ones, but we could only enable facets for Lucene property indexes on explicitly named properties, unless we make sure the regex excludes those forbidden ones... basically I'm not sure I get how you'd do it there. bq. Current impl looks like creats faceted field for all type. It would be better to restrict it to String type I think for the long run we'll also want range facets (e.g. facets on user defined ranges price:[10 TO 20], price:[20 TO 50], etc.) so I was thinking to facets also on such non String types. However for the current scope (facet on fields / properties) it sounds good to limit to String. bq. Current approach would not work for relative properties. If you create facet fields from within indexProperty then it would be called from both makeDocument for immediate properties and indexAggregates for relative properties ok, I think we can move it to {{#indexProperty}}. bq. loadDocs method is now becoming too big and doing lots of things. We should move faceting logic to separate method that's a general issue I think (we have too long / complex) classes, if you have time for a refactoring that'd be good. bq. Move Facet field logic to IndexPlan as part of IndexPlanner#getPlanBuilder call where it iterates over the property definitions ok bq. Use PERF_LOGGER instead of System.currentTimeMillis() +1 bq. Facet field name logic to a method +1 bq. Any details on what purpose does {{FilteredSortedSetDocValuesFacetCounts}}. Look like we can mark it static. Also having some test around its logic would be useful! it's an extensions of the Lucene class {{SortedSetDocValuesFacetCounts}} which filter facets according to calling user ACLs. +1 for a unit test. > Support for faceted search in Lucene index > ------------------------------------------ > > Key: OAK-2511 > URL: https://issues.apache.org/jira/browse/OAK-2511 > Project: Jackrabbit Oak > Issue Type: Sub-task > Components: lucene > Reporter: Tommaso Teofili > Assignee: Tommaso Teofili > Fix For: 1.3.13 > > > A Lucene index should be able to provide faceted search results, based on the > support provided in the query engine. -- This message was sent by Atlassian JIRA (v6.3.4#6332)