thomasmueller commented on a change in pull request #444:
URL: https://github.com/apache/jackrabbit-oak/pull/444#discussion_r772377566
##########
File path:
oak-search/src/test/java/org/apache/jackrabbit/oak/plugins/index/PropertyIndexCommonTest.java
##########
@@ -171,6 +180,56 @@ public void propertyExistenceQuery() throws Exception {
Arrays.asList("/test/a", "/test/b")));
}
+ @Test
+ public void propertyExistenceQueryWithNullCheck() throws Exception {
+ NodeTypeRegistry.register(root,
IOUtils.toInputStream(TestUtil.TEST_NODE_TYPE), "test nodeType");
+
+ Tree idx = indexOptions.setIndex(root, "test1",
+
indexOptions.createIndex(indexOptions.createIndexDefinitionBuilder(),
TestUtil.NT_TEST, false, "propa", "propb"));
+ Tree props = TestUtil.newRulePropTree(idx, TestUtil.NT_TEST);
+ Tree prop = props.addChild(TestUtil.unique("prop"));
+ prop.setProperty(PROP_NAME, "propa");
+ prop.setProperty(PROP_PROPERTY_INDEX, true);
+ prop.setProperty(PROP_NOT_NULL_CHECK_ENABLED, true);
+ root.commit();
+
+ Tree test = root.getTree("/").addChild("test");
+ createNodeWithType(test, "a", "oak:TestNode").setProperty("propa",
"a");
+ createNodeWithType(test, "b", "oak:TestNode").setProperty("propa",
"c");
+ createNodeWithType(test, "c", "oak:TestNode").setProperty("propb",
"e");
+ root.commit();
+
+ String query = "select [jcr:path] from [oak:TestNode] where [propa] is
not null";
+ String explanation = explain(query);
+ assertThat(explanation, containsString(indexOptions.getIndexType() +
":test1(/oak:index/test1) "));
+ assertEventually(() -> assertQuery(query, asList("/test/a",
"/test/b")));
+ }
+
+ @Test
+ public void propertyNonExistenceQuery() throws Exception {
+ NodeTypeRegistry.register(root,
IOUtils.toInputStream(TestUtil.TEST_NODE_TYPE), "test nodeType");
+
+ Tree idx = indexOptions.setIndex(root, "test2",
+
indexOptions.createIndex(indexOptions.createIndexDefinitionBuilder(),
TestUtil.NT_TEST, false, "propa", "propb"));
+ Tree props = TestUtil.newRulePropTree(idx, TestUtil.NT_TEST);
+ Tree prop = props.addChild(TestUtil.unique("prop"));
+ prop.setProperty(PROP_NAME, "propa");
+ prop.setProperty(PROP_PROPERTY_INDEX, true);
+ prop.setProperty(PROP_NULL_CHECK_ENABLED, true);
+ root.commit();
+
+ Tree test = root.getTree("/").addChild("test");
+ createNodeWithType(test, "a", "oak:TestNode").setProperty("propa",
"a");
+ createNodeWithType(test, "b", "oak:TestNode").setProperty("propa",
"c");
+ createNodeWithType(test, "c", "oak:TestNode").setProperty("propb",
"e");
+ root.commit();
+
+ String query = "select [jcr:path] from [oak:TestNode] where [propa] is
null";
+ String explanation = explain(query);
+ assertThat(explanation, containsString(indexOptions.getIndexType() +
":test2(/oak:index/test2) "));
Review comment:
I think it is insufficient. The old test verified the elastic query,
however the new test doesn't. The test wouldn't fail even if the query string
isn't correct, as the Oak query engine would filter out the data.
##########
File path:
oak-search-elastic/src/main/java/org/apache/jackrabbit/oak/plugins/index/elastic/util/TermQueryBuilderFactory.java
##########
@@ -128,20 +115,17 @@ public static TermQueryBuilder newNullPropQuery(String
propName) {
} else if (pr.first != null && pr.last != null) {
return newRangeQuery(propertyName, first, last,
pr.firstIncluding, pr.lastIncluding);
- } else if (pr.first != null && pr.last == null) {
+ } else if (pr.first != null) {
Review comment:
This change seems to be unrelated to the oak issue. I see that
technically, it's not needed.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
