[ https://issues.apache.org/jira/browse/OAK-4636?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15409736#comment-15409736 ]
Vikas Saurabh edited comment on OAK-4636 at 8/9/16 2:38 AM: ------------------------------------------------------------ Fixed in [r1755366|https://svn.apache.org/r1755366]. The tests currently just check for tolerance for {{declaringNodeTypes}} and {{propertyNames}}. Although the code would first try {{getNames}} and then fallback to String\[] and logging a warning - the tests don't test for the warning. Backported to 1.4 at [r1755549|https://svn.apache.org/r1755549], to 1.2 at [r1755550|https://svn.apache.org/r1755550] and to 1.2 at [r1755551|https://svn.apache.org/r1755551]. was (Author: catholicon): Fixed in [r1755366|https://svn.apache.org/r1755366]. The tests currently just check for tolerance for {{declaringNodeTypes}} and {{propertyNames}}. Although the code would first try {{getNames}} and then fallback to String\[] and logging a warning - the tests don't test for the warning. Backported to 1.4 at [r1755549|https://svn.apache.org/r1755549], to 1.2 at [r1755550|https://svn.apache.org/r1755550]. Also, while this is just a minor improvement, marking it as candidate for 1.0. > PropertyIndexLookup#getIndexNode should be more tolerant towards property > types > ------------------------------------------------------------------------------- > > Key: OAK-4636 > URL: https://issues.apache.org/jira/browse/OAK-4636 > Project: Jackrabbit Oak > Issue Type: Improvement > Components: query > Reporter: Vikas Saurabh > Assignee: Vikas Saurabh > Priority: Minor > Fix For: 1.6, 1.0.33, 1.4.6, 1.2.18, 1.5.8 > > > Currently, {{PropertyIndexLookup#getIndexNode}} \[0] uses > {{NodeState#getNames}} for {{propertyNames}} \[1] and {{declaringNodeTypes}} > \[2]. That means that these properties need to be either of {{Name}} or > {{Name\[]}} types. While that's ok and probably useful as well - this can > potentially be used for validation and javadoc for getNames says that the > implementation is free to use an optimized path. > That being said, at least in one case (issue 768 in ensure oak index \[3]) > the values get set as {{String/String\[]}} instead which can very easily > render useful indices like {{nodetype}} useless for the running system. > I see 2 ways around it: > * PropertyIndexLookup can be more resilient and accept non-Name-type too. For > optimal case, we can probably try getNames() first and then fallback to > getProperties with potential to cast (and may be log a warning for mistake in > property type) > * Proactively validate that properties such as declNodeTypes etc must have > {{Name/Name\[]}} during commit > I'm ok with any of the approach... but current scenario is too silent in > failing... saving the change doesn't cause any issue... cost calculation > while picking index simply ignore such indices. > /cc [~tmueller], [~chetanm] > \[0]: > https://github.com/apache/jackrabbit-oak/blob/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/property/PropertyIndexLookup.java#L159 > \[1]: > https://github.com/apache/jackrabbit-oak/blob/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/property/PropertyIndexLookup.java#L171 > \[2]: > https://github.com/apache/jackrabbit-oak/blob/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/index/property/PropertyIndexLookup.java#L179 > \[3]: https://github.com/Adobe-Consulting-Services/acs-aem-commons/issues/768 -- This message was sent by Atlassian JIRA (v6.3.4#6332)