This is an automated email from the ASF dual-hosted git repository. thomasm pushed a commit to branch OAK-10261 in repository https://gitbox.apache.org/repos/asf/jackrabbit-oak.git
commit 34865a30c2784d8ec805c202acde6efc358bc261 Author: Thomas Mueller <[email protected]> AuthorDate: Wed May 24 15:00:44 2023 +0200 OAK-10261 Query with OR clause with COALESCE function incorrectly interpreted --- .../jackrabbit/oak/query/ast/CoalesceImpl.java | 4 +- .../apache/jackrabbit/oak/query/ast/OrImpl.java | 2 +- .../jackrabbit/oak/query/ast/OrImplTest.java | 12 ++- .../oak/query/ast/PropertyExistenceTest.java | 102 +++++++++++++++++++++ 4 files changed, 112 insertions(+), 8 deletions(-) diff --git a/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/CoalesceImpl.java b/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/CoalesceImpl.java index 68fd4487bf..9465c34338 100644 --- a/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/CoalesceImpl.java +++ b/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/CoalesceImpl.java @@ -62,8 +62,8 @@ public class CoalesceImpl extends DynamicOperandImpl { @Override public PropertyExistenceImpl getPropertyExistence() { - PropertyExistenceImpl pe = operand1.getPropertyExistence(); - return pe != null ? pe : operand2.getPropertyExistence(); + // we could support "<function> is not null", but right now we don't + return null; } @Override diff --git a/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/OrImpl.java b/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/OrImpl.java index 68d01b051c..03b5787a32 100644 --- a/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/OrImpl.java +++ b/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/OrImpl.java @@ -144,7 +144,7 @@ public class OrImpl extends ConstraintImpl { @Override public Set<PropertyExistenceImpl> getPropertyExistenceConditions() { // for the condition "x=1 or x=2", the existence condition - // "x is not null" be be derived + // "x is not null" can be derived Set<PropertyExistenceImpl> result = null; for (ConstraintImpl constraint : constraints) { if (result == null) { diff --git a/oak-core/src/test/java/org/apache/jackrabbit/oak/query/ast/OrImplTest.java b/oak-core/src/test/java/org/apache/jackrabbit/oak/query/ast/OrImplTest.java index 5a6a27ed1f..665fe5573a 100644 --- a/oak-core/src/test/java/org/apache/jackrabbit/oak/query/ast/OrImplTest.java +++ b/oak-core/src/test/java/org/apache/jackrabbit/oak/query/ast/OrImplTest.java @@ -17,15 +17,16 @@ package org.apache.jackrabbit.oak.query.ast; import static org.apache.jackrabbit.guava.common.collect.ImmutableSet.of; -import static org.hamcrest.CoreMatchers.is; -import static org.junit.Assert.assertThat; +import static org.hamcrest.MatcherAssert.assertThat; import static org.mockito.Mockito.mock; import java.util.Set; +import org.hamcrest.Matchers; import org.junit.Test; public class OrImplTest { + @Test public void simplifyForUnion() { ConstraintImpl op1, op2, op3, op4, or; @@ -35,14 +36,14 @@ public class OrImplTest { op2 = mock(ComparisonImpl.class); or = new OrImpl(op1, op2); expected = of(op1, op2); - assertThat(or.convertToUnion(), is(expected)); + assertThat(or.convertToUnion(), Matchers.is(expected)); op1 = mock(ComparisonImpl.class); op2 = mock(ComparisonImpl.class); op3 = mock(ComparisonImpl.class); or = new OrImpl(new OrImpl(op1, op2), op3); expected = of(op1, op2, op3); - assertThat(or.convertToUnion(), is(expected)); + assertThat(or.convertToUnion(), Matchers.is(expected)); op1 = mock(ComparisonImpl.class); op2 = mock(ComparisonImpl.class); @@ -50,6 +51,7 @@ public class OrImplTest { op4 = mock(ComparisonImpl.class); or = new OrImpl(new OrImpl(new OrImpl(op1, op4), op2), op3); expected = of(op1, op2, op3, op4); - assertThat(or.convertToUnion(), is(expected)); + assertThat(or.convertToUnion(), Matchers.is(expected)); } + } diff --git a/oak-core/src/test/java/org/apache/jackrabbit/oak/query/ast/PropertyExistenceTest.java b/oak-core/src/test/java/org/apache/jackrabbit/oak/query/ast/PropertyExistenceTest.java new file mode 100644 index 0000000000..55cd51a28a --- /dev/null +++ b/oak-core/src/test/java/org/apache/jackrabbit/oak/query/ast/PropertyExistenceTest.java @@ -0,0 +1,102 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.jackrabbit.oak.query.ast; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; + +import java.util.Arrays; + +import org.apache.jackrabbit.oak.plugins.memory.PropertyValues; +import org.junit.Test; + +public class PropertyExistenceTest { + + @Test + public void coalesce() { + CoalesceImpl c1 = new CoalesceImpl( + new PropertyValueImpl("a", "x"), + new PropertyValueImpl("b", "y")); + assertNull(c1.getPropertyExistence()); + } + + @Test + public void orCoalesce() { + CoalesceImpl c1 = new CoalesceImpl( + new PropertyValueImpl("a", "x"), + new PropertyValueImpl("b", "y")); + ComparisonImpl comp1 = new ComparisonImpl(c1, Operator.EQUAL, + new LiteralImpl(PropertyValues.newString("hello"))); + CoalesceImpl c2 = new CoalesceImpl( + new PropertyValueImpl("a", "x"), + new PropertyValueImpl("b", "y")); + ComparisonImpl comp2 = new ComparisonImpl(c2, Operator.EQUAL, + new LiteralImpl(PropertyValues.newString("world"))); + OrImpl or = new OrImpl(comp1, comp2); + assertTrue(or.getPropertyExistenceConditions().isEmpty()); + } + + @Test + public void orX() { + PropertyValueImpl p1 = new PropertyValueImpl("a", "x"); + ComparisonImpl comp1 = new ComparisonImpl(p1, Operator.EQUAL, + new LiteralImpl(PropertyValues.newString("hello"))); + PropertyValueImpl p2 = new PropertyValueImpl("a", "x"); + ComparisonImpl comp2 = new ComparisonImpl(p2, Operator.EQUAL, + new LiteralImpl(PropertyValues.newString("world"))); + OrImpl or = new OrImpl(comp1, comp2); + assertEquals("[[a].[x] is not null]", + or.getPropertyExistenceConditions().toString()); + } + + @Test + public void in() { + PropertyValueImpl p1 = new PropertyValueImpl("a", "x"); + InImpl in = new InImpl(p1, Arrays.asList( + new LiteralImpl(PropertyValues.newString("hello")), + new LiteralImpl(PropertyValues.newString("hello")))); + assertEquals("[[a].[x] is not null]", + in.getPropertyExistenceConditions().toString()); + } + + @Test + public void orXY() { + PropertyValueImpl p1 = new PropertyValueImpl("a", "x"); + ComparisonImpl comp1 = new ComparisonImpl(p1, Operator.EQUAL, + new LiteralImpl(PropertyValues.newString("hello"))); + PropertyValueImpl p2 = new PropertyValueImpl("b", "y"); + ComparisonImpl comp2 = new ComparisonImpl(p2, Operator.EQUAL, + new LiteralImpl(PropertyValues.newString("world"))); + OrImpl or = new OrImpl(comp1, comp2); + assertEquals("[]", or.getPropertyExistenceConditions().toString()); + } + + @Test + public void andXY() { + PropertyValueImpl p1 = new PropertyValueImpl("a", "x"); + ComparisonImpl comp1 = new ComparisonImpl(p1, Operator.EQUAL, + new LiteralImpl(PropertyValues.newString("hello"))); + PropertyValueImpl p2 = new PropertyValueImpl("b", "y"); + ComparisonImpl comp2 = new ComparisonImpl(p2, Operator.EQUAL, + new LiteralImpl(PropertyValues.newString("world"))); + AndImpl or = new AndImpl(comp1, comp2); + assertEquals("[[a].[x] is not null, [b].[y] is not null]", + or.getPropertyExistenceConditions().toString()); + } + +}
