This is an automated email from the ASF dual-hosted git repository. thomasm pushed a commit to branch OAK-12007 in repository https://gitbox.apache.org/repos/asf/jackrabbit-oak.git
commit 36848c17d0d02178cfff3413cd675da09afbe7dd Author: Thomas Mueller <[email protected]> AuthorDate: Thu Nov 13 11:52:08 2025 +0100 OAK-12007 XPath with or conditions should not always be converted to union --- .../main/java/org/apache/jackrabbit/oak/Oak.java | 8 + .../jackrabbit/oak/query/QueryEngineSettings.java | 12 ++ .../jackrabbit/oak/query/xpath/Statement.java | 13 +- .../oak/query/xpath/XPathToSQL2Converter.java | 3 +- .../oak/query/xpath/XPathToSQL2Test.java | 179 +++++++++++++++++++++ 5 files changed, 209 insertions(+), 6 deletions(-) diff --git a/oak-core/src/main/java/org/apache/jackrabbit/oak/Oak.java b/oak-core/src/main/java/org/apache/jackrabbit/oak/Oak.java index 3f39239168..67431ee879 100644 --- a/oak-core/src/main/java/org/apache/jackrabbit/oak/Oak.java +++ b/oak-core/src/main/java/org/apache/jackrabbit/oak/Oak.java @@ -593,6 +593,10 @@ public class Oak { LOG.info("Registered sort union query by score feature: " + QueryEngineSettings.FT_SORT_UNION_QUERY_BY_SCORE); closer.register(sortUnionQueryByScoreFeature); queryEngineSettings.setSortUnionQueryByScoreFeature(sortUnionQueryByScoreFeature); + Feature optimizeXPathUnion = newFeature(QueryEngineSettings.FT_OPTIMIZE_XPATH_UNION, whiteboard); + LOG.info("Registered optimize XPath union feature: " + QueryEngineSettings.FT_OPTIMIZE_XPATH_UNION); + closer.register(optimizeXPathUnion); + queryEngineSettings.setOptimizeXPathUnion(optimizeXPathUnion); } return this; @@ -1008,6 +1012,10 @@ public class Oak { settings.setSortUnionQueryByScoreFeature(feature); } + public void setOptimizeXPathUnion(@Nullable Feature feature) { + settings.setOptimizeXPathUnion(feature); + } + @Override public void setQueryValidatorPattern(String key, String pattern, String comment, boolean failQuery) { settings.getQueryValidator().setPattern(key, pattern, comment, failQuery); diff --git a/oak-core/src/main/java/org/apache/jackrabbit/oak/query/QueryEngineSettings.java b/oak-core/src/main/java/org/apache/jackrabbit/oak/query/QueryEngineSettings.java index 9a143f91ec..f34edce0b0 100644 --- a/oak-core/src/main/java/org/apache/jackrabbit/oak/query/QueryEngineSettings.java +++ b/oak-core/src/main/java/org/apache/jackrabbit/oak/query/QueryEngineSettings.java @@ -69,6 +69,8 @@ public class QueryEngineSettings implements QueryEngineSettingsMBean, QueryLimit public static final String FT_SORT_UNION_QUERY_BY_SCORE = "FT_OAK-11949"; + public static final String FT_OPTIMIZE_XPATH_UNION = "FT_OAK-12007"; + public static final int DEFAULT_PREFETCH_COUNT = Integer.getInteger(OAK_QUERY_PREFETCH_COUNT, -1); public static final String OAK_QUERY_FAIL_TRAVERSAL = "oak.queryFailTraversal"; @@ -127,6 +129,7 @@ public class QueryEngineSettings implements QueryEngineSettingsMBean, QueryLimit private Feature improvedIsNullCostFeature; private Feature optimizeInRestrictionsForFunctions; private Feature sortUnionQueryByScoreFeature; + private Feature optimizeXPathUnion; private String autoOptionsMappingJson = "{}"; private QueryOptions.AutomaticQueryOptionsMapping autoOptionsMapping = new QueryOptions.AutomaticQueryOptionsMapping(autoOptionsMappingJson); @@ -260,6 +263,15 @@ public class QueryEngineSettings implements QueryEngineSettingsMBean, QueryLimit return sortUnionQueryByScoreFeature != null && sortUnionQueryByScoreFeature.isEnabled(); } + public void setOptimizeXPathUnion(@Nullable Feature feature) { + this.optimizeXPathUnion = feature; + } + + public boolean isOptimizeXPathUnionEnabled() { + // disable if the feature toggle is not used + return optimizeXPathUnion != null && optimizeXPathUnion.isEnabled(); + } + public String getStrictPathRestriction() { return strictPathRestriction.name(); } diff --git a/oak-core/src/main/java/org/apache/jackrabbit/oak/query/xpath/Statement.java b/oak-core/src/main/java/org/apache/jackrabbit/oak/query/xpath/Statement.java index ad333c0ecc..8a97a120f7 100644 --- a/oak-core/src/main/java/org/apache/jackrabbit/oak/query/xpath/Statement.java +++ b/oak-core/src/main/java/org/apache/jackrabbit/oak/query/xpath/Statement.java @@ -62,13 +62,16 @@ public class Statement { QueryOptions queryOptions; - public Statement optimize() { + public Statement optimize(boolean convertOrToUnion) { ignoreOrderByScoreDesc(orderList); if (where == null) { return this; } where = where.optimize(); optimizeSelectorNodeTypes(); + if (!convertOrToUnion) { + return this; + } ArrayList<Expression> unionList = new ArrayList<Expression>(); try { addToUnionList(where, unionList); @@ -90,7 +93,7 @@ public class Statement { if (union == null) { union = s; } else { - union = new UnionStatement(union.optimize(), s.optimize()); + union = new UnionStatement(union.optimize(convertOrToUnion), s.optimize(convertOrToUnion)); } } union.orderList = orderList; @@ -303,12 +306,12 @@ public class Statement { } @Override - public Statement optimize() { + public Statement optimize(boolean convertOrToUnion) { if (!KEEP_UNION_ORDER) { ignoreOrderByScoreDesc(orderList); } - Statement s1b = s1.optimize(); - Statement s2b = s2.optimize(); + Statement s1b = s1.optimize(convertOrToUnion); + Statement s2b = s2.optimize(convertOrToUnion); if (s1 == s1b && s2 == s2b) { // no change return this; diff --git a/oak-core/src/main/java/org/apache/jackrabbit/oak/query/xpath/XPathToSQL2Converter.java b/oak-core/src/main/java/org/apache/jackrabbit/oak/query/xpath/XPathToSQL2Converter.java index 74ab7e3569..514d477262 100644 --- a/oak-core/src/main/java/org/apache/jackrabbit/oak/query/xpath/XPathToSQL2Converter.java +++ b/oak-core/src/main/java/org/apache/jackrabbit/oak/query/xpath/XPathToSQL2Converter.java @@ -97,7 +97,8 @@ public class XPathToSQL2Converter { */ public String convert(String query) throws ParseException { Statement statement = convertToStatement(query); - statement = statement.optimize(); + boolean convertOrToUnion = settings == null || !settings.isOptimizeXPathUnionEnabled(); + statement = statement.optimize(convertOrToUnion); return statement.toString(); } diff --git a/oak-core/src/test/java/org/apache/jackrabbit/oak/query/xpath/XPathToSQL2Test.java b/oak-core/src/test/java/org/apache/jackrabbit/oak/query/xpath/XPathToSQL2Test.java new file mode 100644 index 0000000000..7acf58e06e --- /dev/null +++ b/oak-core/src/test/java/org/apache/jackrabbit/oak/query/xpath/XPathToSQL2Test.java @@ -0,0 +1,179 @@ +/* + * 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.xpath; + +import static org.junit.Assert.assertEquals; + +import java.text.ParseException; + +import org.apache.jackrabbit.oak.query.QueryEngineSettings; +import org.apache.jackrabbit.oak.spi.toggle.Feature; +import org.junit.Test; +import org.mockito.Mockito; + +/** + * Tests for XPathToSQL2Converter focusing on the OR-to-UNION conversion behavior + * controlled by the optimizeXPathUnion feature toggle. + */ +public class XPathToSQL2Test { + + @Test + public void testOrToUnionConversionFeatureDisabled() throws ParseException { + // Default behavior (feature disabled): OR conditions should be converted to UNION + verify("//*[@a = 1 or @b = 2]", + false, + "select ... where [a] = 1 union select ... where [b] = 2 "); + } + + @Test + public void testOrToUnionConversionFeatureEnabled() throws ParseException { + // When feature is enabled: OR conditions should NOT be converted to UNION + verify("//*[@a = 1 or @b = 2]", + true, + "select ... where [a] = 1 or [b] = 2 "); + } + + @Test + public void testMultipleOrConditionsFeatureDisabled() throws ParseException { + // Test multiple OR conditions with feature disabled (default) + verify("//*[@a = 1 or @b = 2 or @c = 3]", + false, + "select ... where [a] = 1 union select ... where [b] = 2 union select ... where [c] = 3 "); + } + + @Test + public void testMultipleOrConditionsFeatureEnabled() throws ParseException { + // Test multiple OR conditions with feature enabled + verify("//*[@a = 1 or @b = 2 or @c = 3]", + true, + "select ... where [a] = 1 or [b] = 2 or [c] = 3 "); + } + + @Test + public void testNestedOrWithAndFeatureDisabled() throws ParseException { + // Test nested OR with AND conditions, feature disabled + verify("//*[@x = 1 and (@a = 1 or @b = 2)]", + false, + "select ... where [x] = 1 and [a] = 1 union select ... where [x] = 1 and [b] = 2 "); + } + + @Test + public void testNestedOrWithAndFeatureEnabled() throws ParseException { + // Test nested OR with AND conditions, feature enabled + verify("//*[@x = 1 and (@a = 1 or @b = 2)]", + true, + "select ... where [x] = 1 and ([a] = 1 or [b] = 2) "); + } + + @Test + public void testSimpleOrQueryFeatureDisabled() throws ParseException { + // Simple OR query with feature disabled + verify("//*[@name = 'foo' or @name = 'bar']", + false, + "select ... where [name] in('foo', 'bar') "); + } + + @Test + public void testSimpleOrQueryFeatureEnabled() throws ParseException { + // Simple OR query with feature enabled + verify("//*[@name = 'foo' or @name = 'bar']", + true, + "select ... where [name] in('foo', 'bar') "); + } + + @Test + public void testComplexNestedOrFeatureDisabled() throws ParseException { + // Complex nested query: (a AND (b OR c)) OR d + verify("//*[(@a = 1 and (@b = 2 or @c = 3)) or @d = 4]", + false, + "select ... where [a] = 1 and [b] = 2 union select ... where [a] = 1 and [c] = 3 union select ... where [d] = 4 "); + } + + @Test + public void testComplexNestedOrFeatureEnabled() throws ParseException { + // Complex nested query: (a AND (b OR c)) OR d + verify("//*[(@a = 1 and (@b = 2 or @c = 3)) or @d = 4]", + true, + "select ... where [a] = 1 and ([b] = 2 or [c] = 3) or [d] = 4 "); + } + + @Test + public void testOrWithDifferentPathsFeatureDisabled() throws ParseException { + // Test OR conditions with different paths + verify("//*[@title = 'Test' or @desc = 'Test']", + false, + "select ... where [title] = 'Test' union select ... where [desc] = 'Test' "); + } + + @Test + public void testOrWithDifferentPathsFeatureEnabled() throws ParseException { + // Test OR conditions with different paths + verify("//*[@title = 'Test' or @desc = 'Test']", + true, + "select ... where [title] = 'Test' or [desc] = 'Test' "); + } + + @Test + public void testOrWithFunctionCallFeatureDisabled() throws ParseException { + // Test OR with function calls + verify("//*[jcr:contains(., 'test') or @type = 'page']", + false, + "select ... where contains(*, 'test') union select ... where [type] = 'page' "); + } + + @Test + public void testOrWithFunctionCallFeatureEnabled() throws ParseException { + verify("//*[jcr:contains(., 'test') or @type = 'page']", + true, + "select ... where contains(*, 'test') or [type] = 'page' "); + } + + /** + * Helper method to create a Feature mock with the specified enabled state. + */ + private static Feature createFeature(boolean enabled) { + Feature feature = Mockito.mock(Feature.class); + Mockito.when(feature.isEnabled()).thenReturn(enabled); + return feature; + } + + private void verify(String xpath, boolean optimizeXPathUnion, String expectedSql2) throws ParseException { + Feature feature = createFeature(optimizeXPathUnion); + QueryEngineSettings settings = new QueryEngineSettings(); + settings.setOptimizeXPathUnion(feature); + XPathToSQL2Converter converter = new XPathToSQL2Converter(settings); + String sql2 = converter.convert(xpath); + sql2 = formatSQL(sql2); + assertEquals(expectedSql2, sql2); + } + + static String formatSQL(String sql) { + sql = sql.replace('\n', ' '); + /* + sql = sql.replaceAll(" from ", "\nfrom "); + sql = sql.replaceAll(" where ", "\nwhere "); + sql = sql.replaceAll(" and ", "\nand "); + sql = sql.replaceAll(" union ", "\nunion "); + sql = sql.replaceAll(" order by ", "\norder by "); + sql = sql.replaceAll(" option\\(", "\noption\\("); + */ + sql = sql.replaceAll("\\[jcr:path\\], \\[jcr:score\\], \\* from \\[nt:base\\] as a", "..."); + sql = sql.replaceAll("\\/\\*.*\\*/", ""); + return sql; + } +} +
