jackrabbit-oak-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Chetan Mehrotra (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (OAK-2999) Index updation fails on updating multivalued property
Date Fri, 03 Jul 2015 06:21:04 GMT

    [ https://issues.apache.org/jira/browse/OAK-2999?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14612882#comment-14612882
] 

Chetan Mehrotra commented on OAK-2999:
--------------------------------------

{noformat}
-        boolean dirty = false;
+        boolean dirty = propertiesChanged;
+
         for (PropertyState property : state.getProperties()) {
             String pname = property.getName();
 
@@ -327,6 +328,8 @@
             }
 
             dirty |= indexProperty(path, fields, state, property, pname, pd);
+            // If the only property was MVP and indexed earlier and has now been emptied
+            dirty |= !before.hasProperty(pname) || (property.count() == 0);
{noformat}

If we set dirty to {{propertiesChanged}} then the other change might not be required as if
any indexed property is changed then {{propertiesChanged}} would be true and dirty would remain
true. In fact we can then do away with dirty flag at all.

However there is one issue with how {{propertiesChanged}} is currently set
{code}
    private void markPropertyChanged(String name) {
        if (isIndexable()
                && !propertiesChanged
                && indexingRule.isIndexed(name)) {
            propertiesChanged = true;
        }
    }
{code}

{{indexingRule.isIndexed(name)}} just checks for presence of property definition. But that
has one bug where a property might have property definition but with {{index}} set to false
i.e. its a negative entry to prevent some properties from getting indexed. This is typically
used when you need to exclude certain properties from fulltext index. There are other cases
also where propertyDefinition might exist for given name but the type does not match. Due
to this {{propertiesChanged}} so far was like hint with actual check done in various indexing
logic

{noformat}
+ nt:base
      + properties
        - jcr:primaryType = "nt:unstructured"
        +loginTime
         - name = "loginTime"
         - index = false
        + allProps
          - name = ".*"
          - isRegexp = true
          - nodeScopeIndex = true
{noformat}

So we would need to be carefull there. Some minor comments

* Change {{state}} in {{makeDocument}} to {{after}}
* There are some unnecessary changes in the patch mostly around formatting. Can you avoid
them

> Index updation fails on updating multivalued property
> -----------------------------------------------------
>
>                 Key: OAK-2999
>                 URL: https://issues.apache.org/jira/browse/OAK-2999
>             Project: Jackrabbit Oak
>          Issue Type: Bug
>          Components: lucene
>    Affects Versions: 1.2, 1.0.15
>            Reporter: Rishabh Maurya
>            Assignee: Amit Jain
>             Fix For: 1.2.3, 1.3.3, 1.0.17
>
>         Attachments: OAK-2999.patch
>
>
> On emptying a multivalued property, fulltext index updation fails and one can search
on old values. Following test demonstrates the issue.
> Added below test in [LuceneIndexQueryTest.java|https://github.com/apache/jackrabbit-oak/blob/trunk/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/LuceneIndexQueryTest.java]
which should pass - 
> {code}
>     @Test
>     public void testMultiValuedPropUpdate() throws Exception {
>         Tree test = root.getTree("/").addChild("test");
>         String child = "child";
>         String mulValuedProp = "prop";
>         test.addChild(child).setProperty(mulValuedProp, of("foo","bar"), Type.STRINGS);
>         root.commit();
>         assertQuery(
>                 "/jcr:root//*[jcr:contains(@" + mulValuedProp + ", 'foo')]",
>                 "xpath", ImmutableList.of("/test/" + child));
>         test.getChild(child).setProperty(mulValuedProp, new ArrayList<String>(),
Type.STRINGS);
>         root.commit();
>         assertQuery(
>                 "/jcr:root//*[jcr:contains(@" + mulValuedProp + ", 'foo')]",
>                 "xpath", new ArrayList<String>());
>         test.getChild(child).setProperty(mulValuedProp, of("bar"), Type.STRINGS);
>         root.commit();
>         assertQuery(
>                 "/jcr:root//*[jcr:contains(@" + mulValuedProp + ", 'foo')]",
>                 "xpath", new ArrayList<String>());
>     }
> {code}



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Mime
View raw message