nit0906 commented on a change in pull request #465:
URL: https://github.com/apache/jackrabbit-oak/pull/465#discussion_r787076069



##########
File path: 
oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/PropertyValueImpl.java
##########
@@ -125,12 +125,7 @@ public void restrict(FilterImpl f, Operator operator, 
PropertyValue v) {
                     f.restrictPath(v.getValue(Type.STRING), 
PathRestriction.EXACT);
                 }
             } else {
-                if (operator == Operator.NOT_EQUAL && v != null) {
-                    // "x <> 1" also means "x is not null"
-                    f.restrictProperty(pn, Operator.NOT_EQUAL, null, 
propertyType);

Review comment:
       @thomasmueller  - thanks. I understand now.
   
   >And to me it seems there are not enough test cases in the property index, 
maybe.
   
   Yes, all the tests were passing and I was unaware of this usage in 
ContentMirrorStoreStrategy. 
   
   I have made some changes in the latest commit - 
   
https://github.com/apache/jackrabbit-oak/pull/465/commits/f9d45d8f8f422cdf9f27f1aca794ae73e9b5e0bc.
 Now adding 2 propertyRestrictions - so that the old behaviour with 
PropertyIndex is not broken.
   
   Can you please re review ?




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscr...@jackrabbit.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


Reply via email to