[ 
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)

Reply via email to