diff --git a/oak-core/src/main/java/org/apache/jackrabbit/oak/query/Query.java b/oak-core/src/main/java/org/apache/jackrabbit/oak/query/Query.java index aa87715..ef3b503 100644 --- a/oak-core/src/main/java/org/apache/jackrabbit/oak/query/Query.java +++ b/oak-core/src/main/java/org/apache/jackrabbit/oak/query/Query.java @@ -16,7 +16,11 @@ package org.apache.jackrabbit.oak.query; import java.util.Iterator; import java.util.List; +import javax.annotation.Nonnull; +import javax.annotation.Nullable; + import aQute.bnd.annotation.ProviderType; + import org.apache.jackrabbit.oak.api.PropertyValue; import org.apache.jackrabbit.oak.api.Result; import org.apache.jackrabbit.oak.api.Tree; @@ -125,4 +129,69 @@ public interface Query { * @return if sorted by index */ boolean isSortedByIndex(); + + /** + * Perform optimisation on the object itself. To avoid any potential error due to state + * variables perfom the optimisation before the {@link #init()}. + * + * @return {@code this} if no optimisations are possible or a new instance of a {@link Query}. + * Cannot return null. + */ + @Nonnull + Query optimise(); + + /** + *

+ * returns a clone of the current object. Will throw an exception in case it's invoked in a non + * appropriate moment. For example the default {@link QueryImpl} cannot be cloned once the + * {@link #init()} has been executed. + *

+ * + *

+ * May return null if not implemented. + *

+ * @return a clone of self + * @throws IllegalStateException + */ + @Nullable + Query copyOf() throws IllegalStateException; + + /** + * @return {@code true} if the query has been already initialised. {@code false} otherwise. + */ + boolean isInit(); + + /** + * @return {@code true} if the query is a result of optimisations. {@code false} if it's the + * originally computed one. + */ + boolean isOptimised(); + + /** + * @return the original statement as it was used to construct the object. If not provided the + * {@link #toString()} will be used instead. + */ + String getStatement(); + + /** + * + * @return {@code true} if the current query is internal. {@code false} otherwise. + */ + boolean isInternal(); + + /** + *

+ * Some queries can bring with them a cost overhead that the query engine could consider when + * electing the best query between the original SQL2 and the possible available optimisations. + *

+ *

+ * For example for the case of if + * you have a case where {@code (a = 'v' OR CONTAINS(b, 'v1') OR CONTAINS(c, 'v2')) AND (...)} + * currently the query engine does not know how to leverage indexes and post conditions and the + * query is better suited with a UNION. + *

+ * + * @return a positive number or 0. Cannot be negative. + */ + double getCostOverhead(); } diff --git a/oak-core/src/main/java/org/apache/jackrabbit/oak/query/QueryEngineImpl.java b/oak-core/src/main/java/org/apache/jackrabbit/oak/query/QueryEngineImpl.java index 0469637..7b1314b 100644 --- a/oak-core/src/main/java/org/apache/jackrabbit/oak/query/QueryEngineImpl.java +++ b/oak-core/src/main/java/org/apache/jackrabbit/oak/query/QueryEngineImpl.java @@ -16,7 +16,9 @@ */ package org.apache.jackrabbit.oak.query; +import static com.google.common.base.Preconditions.checkNotNull; import static com.google.common.collect.ImmutableSet.of; +import static com.google.common.collect.Sets.newHashSet; import static org.apache.jackrabbit.JcrConstants.JCR_SYSTEM; import static org.apache.jackrabbit.oak.plugins.nodetype.NodeTypeConstants.JCR_NODE_TYPES; @@ -27,6 +29,8 @@ import java.util.Map.Entry; import java.util.Set; import java.util.concurrent.atomic.AtomicInteger; +import javax.annotation.Nonnull; + import org.apache.jackrabbit.oak.api.PropertyValue; import org.apache.jackrabbit.oak.api.QueryEngine; import org.apache.jackrabbit.oak.api.Result; @@ -44,6 +48,27 @@ import org.slf4j.MDC; */ public abstract class QueryEngineImpl implements QueryEngine { + /** + * used to instruct the {@link QueryEngineImpl} on how to act with respect of the SQL2 + * optimisation. + */ + public static enum ForceOptimised { + /** + * will force the original SQL2 query to be executed + */ + ORIGINAL, + + /** + * will force the computed optimised query to be executed. If available. + */ + OPTIMISED, + + /** + * will execute the cheapest. + */ + CHEAPEST + } + private static final AtomicInteger ID_COUNTER = new AtomicInteger(); private static final String MDC_QUERY_ID = "oak.query.id"; private static final String OAK_QUERY_ANALYZE = "oak.query.analyze"; @@ -70,6 +95,12 @@ public abstract class QueryEngineImpl implements QueryEngine { private boolean traversalEnabled = true; /** + * Whether the query engine should be forced to use the optimised version of the query if + * available. + */ + private ForceOptimised forceOptimised = ForceOptimised.CHEAPEST; + + /** * Get the execution context for a single query execution. * * @return the context @@ -94,11 +125,12 @@ public abstract class QueryEngineImpl implements QueryEngine { public List getBindVariableNames( String statement, String language, Map mappings) throws ParseException { - Query q = parseQuery(statement, language, getExecutionContext(), mappings); - return q.getBindVariableNames(); + Set qs = parseQuery(statement, language, getExecutionContext(), mappings); + + return qs.iterator().next().getBindVariableNames(); } - private static Query parseQuery( + private static Set parseQuery( String statement, String language, ExecutionContext context, Map mappings) throws ParseException { @@ -123,11 +155,16 @@ public abstract class QueryEngineImpl implements QueryEngine { parser.setAllowNumberLiterals(false); parser.setAllowTextLiterals(false); } + + Set queries = newHashSet(); + + Query q; + if (SQL2.equals(language) || JQOM.equals(language)) { - return parser.parse(statement); + q = parser.parse(statement, false); } else if (SQL.equals(language)) { parser.setSupportSQL1(true); - return parser.parse(statement); + q = parser.parse(statement, false); } else if (XPATH.equals(language)) { XPathToSQL2Converter converter = new XPathToSQL2Converter(); String sql2 = converter.convert(statement); @@ -135,7 +172,7 @@ public abstract class QueryEngineImpl implements QueryEngine { try { // OAK-874: No artificial XPath selector name in wildcards parser.setIncludeSelectorNameInWildcardColumns(false); - return parser.parse(sql2); + q = parser.parse(sql2, false); } catch (ParseException e) { ParseException e2 = new ParseException( statement + " converted to SQL-2 " + e.getMessage(), 0); @@ -145,6 +182,34 @@ public abstract class QueryEngineImpl implements QueryEngine { } else { throw new ParseException("Unsupported language: " + language, 0); } + + queries.add(q); + + if (settings.isSql2Optimisation()) { + if (q.isInternal()) { + LOG.trace("Skipping optimisation as internal query."); + } else { + LOG.trace("Attempting optimisation"); + Query q2 = q.optimise(); + if (q2 != q) { + LOG.debug("Optimised query available. {}", q2); + queries.add(q2); + } + } + } + + // initialising all the queries. + for (Query query : queries) { + try { + query.init(); + } catch (Exception e) { + ParseException e2 = new ParseException(query.getStatement() + ": " + e.getMessage(), 0); + e2.initCause(e); + throw e2; + } + } + + return queries; } @Override @@ -176,7 +241,9 @@ public abstract class QueryEngineImpl implements QueryEngine { } ExecutionContext context = getExecutionContext(); - Query q = parseQuery(statement, language, context, mappings); + Set queries = parseQuery(statement, language, context, mappings); + + for (Query q : queries) { q.setExecutionContext(context); q.setLimit(limit); q.setOffset(offset); @@ -186,11 +253,13 @@ public abstract class QueryEngineImpl implements QueryEngine { } } q.setTraversalEnabled(traversalEnabled); + } boolean mdc = false; try { - mdc = setupMDC(q); - q.prepare(); + MdcAndPrepared map = prepareAndGetCheapest(queries); + mdc = map.mdc; + Query q = map.query; return q.executeQuery(); } finally { if (mdc) { @@ -199,6 +268,133 @@ public abstract class QueryEngineImpl implements QueryEngine { } } + /** + * POJO class used to return the cheapest prepared query from the set and related MDC status + */ + private static class MdcAndPrepared { + private final boolean mdc; + private final Query query; + + public MdcAndPrepared(final boolean mdc, @Nonnull final Query q) { + this.mdc = mdc; + this.query = checkNotNull(q); + } + } + + /** + * will prepare all the available queries and by based on the {@link ForceOptimised} flag return + * the appropriate. + * + * @param queries the list of queries to be executed. cannot be null + * @return + */ + @Nonnull + private MdcAndPrepared prepareAndGetCheapest(@Nonnull final Set queries) { + MdcAndPrepared map = null; + Query cheapest = null; + + + if (checkNotNull(queries).size() == 1) { + // Optimisation. We only have the original query so we prepare and return it. + cheapest = queries.iterator().next(); + cheapest.prepare(); + LOG.debug("No optimisations found. Cheapest is the original query: {}", cheapest); + map = new MdcAndPrepared(setupMDC(cheapest), cheapest); + } else { + double bestCost = Double.MAX_VALUE; + double originalCost = Double.MAX_VALUE; + boolean firstLoop = true; + Query original = null; + + // always prepare all of the queries and compute the cheapest as it's the default behaviour. + // It should trigger more errors during unit and integration testing. Changing + // `forceOptimised` flag should be in case used only during testing. + for (Query q : checkNotNull(queries)) { + LOG.debug("Preparing: {}", q); + q.prepare(); + + double actualCost = q.getEstimatedCost(); + double costOverhead = q.getCostOverhead(); + double overallCost = Math.min(actualCost + costOverhead, Double.MAX_VALUE); + + LOG.debug("actualCost: {} - costOverhead: {} - overallCost: {}", actualCost, + costOverhead, overallCost); + + if (firstLoop) { + // first time we're always the best cost. Avoiding situations where the original + // query has an overall cost as Double.MAX_VALUE. + bestCost = overallCost; + cheapest = q; + firstLoop = false; + } else if (overallCost < bestCost) { + bestCost = overallCost; + cheapest = q; + } + if (!q.isOptimised()) { + original = q; + originalCost = overallCost; + } + } + + if (original != null && bestCost == originalCost && cheapest != original) { + // if the optimised cost is the same as the original SQL2 query we prefer the original. As + // we deal with references the `cheapest!=original` should work. + LOG.trace("Same cost for original and optimised. Forcing original"); + cheapest = original; + } + + switch (forceOptimised) { + case ORIGINAL: + LOG.debug("Forcing the original SQL2 query to be executed by flag"); + for (Query q : checkNotNull(queries)) { + if (!q.isOptimised()) { + map = new MdcAndPrepared(setupMDC(q), q); + } + } + break; + + case OPTIMISED: + LOG.debug("Forcing the optimised SQL2 query to be executed by flag"); + for (Query q : checkNotNull(queries)) { + if (q.isOptimised()) { + map = new MdcAndPrepared(setupMDC(q), q); + } + } + break; + + // CHEAPEST is the default behaviour + case CHEAPEST: + default: + if (cheapest == null) { + // this should not really happen. Defensive coding. + LOG.debug("Cheapest is null. Returning the original SQL2 query."); + for (Query q : checkNotNull(queries)) { + if (!q.isOptimised()) { + map = new MdcAndPrepared(setupMDC(q), q); + } + } + } else { + LOG.debug("Cheapest cost: {} - query: {}", bestCost, cheapest); + map = new MdcAndPrepared(setupMDC(cheapest), cheapest); + } + } + } + + + if (map == null) { + // we should only get here in case of testing forcing weird conditions + LOG.trace("`MdcAndPrepared` is null. Falling back to the original query"); + for (Query q : checkNotNull(queries)) { + if (!q.isOptimised()) { + map = new MdcAndPrepared(setupMDC(q), q); + break; + } + } + } + + return map; + } + protected void setTraversalEnabled(boolean traversalEnabled) { this.traversalEnabled = traversalEnabled; } @@ -222,4 +418,13 @@ public abstract class QueryEngineImpl implements QueryEngine { MDC.remove(OAK_QUERY_ANALYZE); } + /** + * Instruct the query engine on how to behave with regards to the SQL2 optimised query if + * available. + * + * @param forceOptimised cannot be null + */ + protected void setForceOptimised(@Nonnull ForceOptimised forceOptimised) { + this.forceOptimised = forceOptimised; + } } 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 07560c3..a040aa9 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 @@ -41,6 +41,8 @@ public class QueryEngineSettings implements QueryEngineSettingsMBean { private boolean fullTextComparisonWithoutIndex = DEFAULT_FULL_TEXT_COMPARISON_WITHOUT_INDEX; + private boolean sql2Optimisation = Boolean.getBoolean("oak.query.sql2optimisation"); + /** * Get the limit on how many nodes a query may read at most into memory, for * "order by" and "distinct" queries. If this limit is exceeded, the query @@ -93,4 +95,7 @@ public class QueryEngineSettings implements QueryEngineSettingsMBean { return fullTextComparisonWithoutIndex; } + public boolean isSql2Optimisation() { + return sql2Optimisation; + } } diff --git a/oak-core/src/main/java/org/apache/jackrabbit/oak/query/QueryImpl.java b/oak-core/src/main/java/org/apache/jackrabbit/oak/query/QueryImpl.java index 8be367d..646a3d4 100644 --- a/oak-core/src/main/java/org/apache/jackrabbit/oak/query/QueryImpl.java +++ b/oak-core/src/main/java/org/apache/jackrabbit/oak/query/QueryImpl.java @@ -13,6 +13,10 @@ */ package org.apache.jackrabbit.oak.query; +import static com.google.common.base.Preconditions.checkNotNull; +import static com.google.common.collect.Lists.newArrayList; +import static org.apache.jackrabbit.oak.query.ast.AstElementFactory.copyElementAndCheckReference; + import java.math.BigInteger; import java.util.ArrayList; import java.util.Arrays; @@ -24,11 +28,14 @@ import java.util.List; import java.util.Map; import java.util.Set; +import javax.annotation.Nonnull; + +import com.google.common.base.Strings; import com.google.common.collect.AbstractIterator; import com.google.common.collect.Lists; import com.google.common.collect.Maps; - import com.google.common.collect.Ordering; + import org.apache.jackrabbit.oak.api.PropertyValue; import org.apache.jackrabbit.oak.api.Tree; import org.apache.jackrabbit.oak.api.Type; @@ -79,6 +86,7 @@ import org.apache.jackrabbit.oak.query.plan.ExecutionPlan; import org.apache.jackrabbit.oak.query.plan.SelectorExecutionPlan; import org.apache.jackrabbit.oak.spi.query.Filter; import org.apache.jackrabbit.oak.spi.query.PropertyValues; +import org.apache.jackrabbit.oak.spi.query.QueryConstants; import org.apache.jackrabbit.oak.spi.query.QueryIndex; import org.apache.jackrabbit.oak.spi.query.QueryIndex.AdvancedQueryIndex; import org.apache.jackrabbit.oak.spi.query.QueryIndex.IndexPlan; @@ -131,7 +139,7 @@ public class QueryImpl implements Query { }; SourceImpl source; - final String statement; + private String statement; final HashMap bindVariableMap = new HashMap(); final HashMap selectorIndexes = new HashMap(); final ArrayList selectors = new ArrayList(); @@ -161,6 +169,16 @@ public class QueryImpl implements Query { private boolean prepared; private ExecutionContext context; + /** + * whether the object has been initialised or not + */ + private boolean init; + + /** + * whether the query is a result of optimisation or original one. + */ + private boolean optimised; + private boolean isSortedByIndex; private final NamePathMapper namePathMapper; @@ -175,12 +193,19 @@ public class QueryImpl implements Query { QueryImpl(String statement, SourceImpl source, ConstraintImpl constraint, ColumnImpl[] columns, NamePathMapper mapper, QueryEngineSettings settings) { + this(statement, source, constraint, columns, mapper, settings, false); + } + + QueryImpl(String statement, SourceImpl source, ConstraintImpl constraint, + ColumnImpl[] columns, NamePathMapper mapper, QueryEngineSettings settings, + final boolean optimised) { this.statement = statement; this.source = source; this.constraint = constraint; this.columns = columns; this.namePathMapper = mapper; this.settings = settings; + this.optimised = optimised; } @Override @@ -416,6 +441,8 @@ public class QueryImpl implements Query { } distinctColumns[i] = distinct; } + + init = true; } @Override @@ -1138,8 +1165,9 @@ public class QueryImpl implements Query { return Math.min(limit, source.getSize(precision, max)); } + @Override public String getStatement() { - return statement; + return Strings.isNullOrEmpty(statement) ? toString() : statement; } public QueryEngineSettings getSettings() { @@ -1170,4 +1198,171 @@ public class QueryImpl implements Query { return sum.min(max).max(min).longValue(); } + @Override + public Query optimise() { + // optimising for UNION + Query optimised = this; + + if (constraint != null) { + Set unionList = constraint.simplifyForUnion(); + if (unionList.size() > 1) { + // there are some cases where multiple ORs simplify into a single one. If we get a + // union list of just one we don't really have to UNION anything. + QueryImpl left = null; + Query right = null; + // we have something to do here. + for (ConstraintImpl c : unionList) { + if (right != null) { + right = newOptimisedUnionQuery(left, right); + } else { + // pulling left to the right + if (left != null) { + right = left; + } + } + + // cloning original query + left = (QueryImpl) this.copyOf(true); + + // cloning the constraints and assigning to new query + left.constraint = (ConstraintImpl) copyElementAndCheckReference(c); + // re-composing the statement for better debug messages + left.statement = recomposeStatement(left); + } + + optimised = newOptimisedUnionQuery(left, right); + } + } + + return optimised; + } + + private static String recomposeStatement(@Nonnull QueryImpl query) { + checkNotNull(query); + String original = query.getStatement(); + String origUpper = original.toUpperCase(); + StringBuilder recomputed = new StringBuilder(); + final String where = " WHERE "; + final String orderBy = " ORDER BY "; + int whereOffset = where.length(); + + if (query.getConstraint() == null) { + recomputed.append(original); + } else { + recomputed.append(original.substring(0, origUpper.indexOf(where) + whereOffset)); + recomputed.append(query.getConstraint()); + if (origUpper.indexOf(orderBy) > -1) { + recomputed.append(original.substring(origUpper.indexOf(orderBy))); + } + } + return recomputed.toString(); + } + + /** + * convenience method for creating a UnionQueryImpl with proper settings. + * + * @param left + * @param right + * @return + */ + private UnionQueryImpl newOptimisedUnionQuery(@Nonnull Query left, @Nonnull Query right) { + UnionQueryImpl u = new UnionQueryImpl( + false, + checkNotNull(left, "`left` cannot be null"), + checkNotNull(right, "`right` cannot be null"), + this.settings, + true); + u.setExplain(explain); + return u; + } + + @Override + public Query copyOf() throws IllegalStateException { + return copyOf(false); + } + + private Query copyOf(final boolean optimised) { + if (isInit()) { + throw new IllegalStateException("QueryImpl cannot be cloned once initialised."); + } + + List cols = newArrayList(); + for (ColumnImpl c : columns) { + cols.add((ColumnImpl) copyElementAndCheckReference(c)); + } + + QueryImpl copy = new QueryImpl( + this.statement, + (SourceImpl) copyElementAndCheckReference(this.source), + this.constraint, + cols.toArray(new ColumnImpl[0]), + this.namePathMapper, + this.settings, + optimised); + copy.explain = this.explain; + copy.distinct = this.distinct; + + return copy; + } + + @Override + public boolean isInit() { + return init; + } + + @Override + public boolean isOptimised() { + return optimised; + } + + @Override + public boolean isInternal() { + return isInternal; + } + + @Override + public double getCostOverhead() { + return oak2660CostOverhead(getConstraint()); + } + + /** + * compute a cost overhead for the OAK-2660 use case. The query engine better perform/compute + * the use case `(a = 'v' OR CONTAINS(b, 'v1') OR CONTAINS(c, 'v2') AND (...)` as a UNION query + * to leverage different indexes. In this case we return an 'Infinity' overhead for make the + * query engine choose a union query instead. + * + * @param constraint the constraint to analyse. Cannot be null. + * @return + */ + private double oak2660CostOverhead(@Nonnull ConstraintImpl constraint) { + if (checkNotNull(constraint) instanceof OrImpl) { + boolean fulltext = false, plain = false; + for (ConstraintImpl c : constraint.getConstraints()) { + if (c instanceof FullTextSearchImpl) { + fulltext = true; + } else { + plain = true; + } + + if (fulltext && plain) { + return Double.MAX_VALUE; + } + } + } else { + List cs = constraint.getConstraints(); + if (cs == null) { + return 0; + } else { + double cost = 0; + for (ConstraintImpl c : cs) { + cost += oak2660CostOverhead(c); + if (cost == Double.MAX_VALUE) { + return cost; + } + } + return cost; + } + } + return 0; + } } diff --git a/oak-core/src/main/java/org/apache/jackrabbit/oak/query/SQL2Parser.java b/oak-core/src/main/java/org/apache/jackrabbit/oak/query/SQL2Parser.java index 4b6fa43..75d75f8 100644 --- a/oak-core/src/main/java/org/apache/jackrabbit/oak/query/SQL2Parser.java +++ b/oak-core/src/main/java/org/apache/jackrabbit/oak/query/SQL2Parser.java @@ -125,10 +125,11 @@ public class SQL2Parser { * Parse the statement and return the query. * * @param query the query string + * @param initialise if performing the query init ({@code true}) or not ({@code false}) * @return the query * @throws ParseException if parsing fails */ - public Query parse(String query) throws ParseException { + public Query parse(final String query, final boolean initialise) throws ParseException { // TODO possibly support union,... as available at // http://docs.jboss.org/modeshape/latest/manuals/reference/html/jcr-query-and-search.html @@ -164,17 +165,32 @@ public class SQL2Parser { q.setOrderings(orderings); q.setExplain(explain); q.setMeasure(measure); + q.setInternal(isInternal(query)); + + if (initialise) { try { q.init(); } catch (Exception e) { - ParseException e2 = new ParseException(query + ": " + e.getMessage(), 0); + ParseException e2 = new ParseException(statement + ": " + e.getMessage(), 0); e2.initCause(e); throw e2; } - q.setInternal(isInternal(query)); + } + return q; } + /** + * as {@link #parse(String, boolean)} by providing {@code true} to the initialisation flag. + * + * @param query + * @return + * @throws ParseException + */ + public Query parse(final String query) throws ParseException { + return parse(query, true); + } + private QueryImpl parseSelect() throws ParseException { read("SELECT"); boolean distinct = readIf("DISTINCT"); diff --git a/oak-core/src/main/java/org/apache/jackrabbit/oak/query/UnionQueryImpl.java b/oak-core/src/main/java/org/apache/jackrabbit/oak/query/UnionQueryImpl.java index 797d5a5..6f39d49 100644 --- a/oak-core/src/main/java/org/apache/jackrabbit/oak/query/UnionQueryImpl.java +++ b/oak-core/src/main/java/org/apache/jackrabbit/oak/query/UnionQueryImpl.java @@ -56,11 +56,22 @@ public class UnionQueryImpl implements Query { private final QueryEngineSettings settings; private boolean isInternal; + /** + * whether the query is a result of optimisation or not + */ + private boolean optimised; + UnionQueryImpl(boolean unionAll, Query left, Query right, QueryEngineSettings settings) { + this(unionAll, left, right, settings, false); + } + + UnionQueryImpl(final boolean unionAll, final Query left, final Query right, + final QueryEngineSettings settings, final boolean optimised) { this.unionAll = unionAll; this.left = left; this.right = right; this.settings = settings; + this.optimised = optimised; } @Override @@ -355,4 +366,40 @@ public class UnionQueryImpl implements Query { public boolean isSortedByIndex() { return left.isSortedByIndex() && right.isSortedByIndex(); } + + @Override + public Query optimise() { + return this; + } + + @Override + public Query copyOf() throws IllegalStateException { + return null; + } + + @Override + public boolean isInit() { + return left.isInit() || right.isInit(); + } + + @Override + public boolean isOptimised() { + return optimised; + } + + @Override + public String getStatement() { + return toString(); + } + + @Override + public boolean isInternal() { + return left.isInternal() || right.isInternal(); + } + + @Override + public double getCostOverhead() { + // for now we don't really have any case where a union query should suffer from overheads. + return 0; + } } diff --git a/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/AndImpl.java b/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/AndImpl.java index 433eea0..8834978 100644 --- a/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/AndImpl.java +++ b/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/AndImpl.java @@ -22,7 +22,9 @@ import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.collect.Lists.newArrayList; import static com.google.common.collect.Sets.newHashSet; import static com.google.common.collect.Sets.newLinkedHashSet; +import static org.apache.jackrabbit.oak.query.ast.AstElementFactory.copyElementAndCheckReference; +import java.util.ArrayList; import java.util.Arrays; import java.util.LinkedHashSet; import java.util.List; @@ -32,6 +34,8 @@ import org.apache.jackrabbit.oak.query.fulltext.FullTextAnd; import org.apache.jackrabbit.oak.query.fulltext.FullTextExpression; import org.apache.jackrabbit.oak.query.index.FilterImpl; +import com.google.common.collect.Sets; + /** * An AND condition. */ @@ -48,6 +52,7 @@ public class AndImpl extends ConstraintImpl { this(Arrays.asList(constraint1, constraint2)); } + @Override public List getConstraints() { return constraints; } @@ -205,4 +210,43 @@ public class AndImpl extends ConstraintImpl { return constraints.hashCode(); } + @Override + public AstElement copyOf() { + List clone = new ArrayList(constraints.size()); + for (ConstraintImpl c : constraints) { + clone.add((ConstraintImpl) copyElementAndCheckReference(c)); + } + return new AndImpl(clone); + } + + @Override + public Set simplifyForUnion() { + Set union = Sets.newHashSet(); + Set result = Sets.newHashSet(); + Set nonUnion = Sets.newHashSet(); + + for (ConstraintImpl c : getConstraints()) { + Set ccc = c.simplifyForUnion(); + if (ccc.isEmpty()) { + nonUnion.add(c); + } else { + union.addAll(ccc); + } + } + if (!union.isEmpty() && nonUnion.size() == 1) { + // this is the simplest case where, for example, out of the two AND operands at least + // one is a non-union. For example WHERE (a OR b OR c) AND d + ConstraintImpl right = nonUnion.iterator().next(); + for (ConstraintImpl c : union) { + result.add(new AndImpl(c, right)); + } + } else { + // in this case prefer to be conservative and don't optimise. This could happen when for + // example: WHERE (a OR b) AND (c OR d). + // This should be translated into a AND c, a AND d, b AND c, b AND d. + } + + return result; + } + } diff --git a/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/AstElement.java b/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/AstElement.java index 109d7d5..f615550 100644 --- a/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/AstElement.java +++ b/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/AstElement.java @@ -18,6 +18,8 @@ */ package org.apache.jackrabbit.oak.query.ast; +import javax.annotation.Nonnull; + import org.apache.jackrabbit.oak.api.PropertyValue; import org.apache.jackrabbit.oak.commons.PathUtils; import org.apache.jackrabbit.oak.query.QueryImpl; @@ -27,7 +29,6 @@ import org.apache.jackrabbit.oak.spi.query.PropertyValues; * The base class for all abstract syntax tree nodes. */ abstract class AstElement { - protected QueryImpl query; abstract boolean accept(AstVisitor v); @@ -147,5 +148,13 @@ abstract class AstElement { return path; } + /** + * @return a clone of self. Default implementation in {@link AstElement} returns same reference + * to {@code this}. + */ + @Nonnull + public AstElement copyOf() { + return this; + } } diff --git a/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/AstElementFactory.java b/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/AstElementFactory.java index e2ec02f..ea2522e 100644 --- a/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/AstElementFactory.java +++ b/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/AstElementFactory.java @@ -13,15 +13,22 @@ */ package org.apache.jackrabbit.oak.query.ast; +import static com.google.common.base.Preconditions.checkNotNull; + import java.util.ArrayList; +import javax.annotation.Nonnull; + import org.apache.jackrabbit.oak.api.PropertyValue; import org.apache.jackrabbit.oak.spi.state.NodeState; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; /** * A factory for syntax tree elements. */ public class AstElementFactory { + private static final Logger LOG = LoggerFactory.getLogger(AstElementFactory.class); public AndImpl and(ConstraintImpl constraint1, ConstraintImpl constraint2) { return new AndImpl(constraint1, constraint2); @@ -164,4 +171,28 @@ public class AstElementFactory { public ConstraintImpl suggest(String selectorName, StaticOperandImpl expression) { return new SuggestImpl(selectorName, expression); } + + /** + *

+ * as the {@link AstElement#copyOf()} can return {@code this} is the cloning is not implemented + * by the subclass, this method add some spice around it by checking for this case and tracking + * a DEBUG message in the logs. + *

+ * + * @param e the element to be cloned. Cannot be null. + * @return same as {@link AstElement#copyOf()} + */ + @Nonnull + public static AstElement copyElementAndCheckReference(@Nonnull final AstElement e) { + AstElement clone = checkNotNull(e).copyOf(); + + if (clone == e && LOG.isDebugEnabled()) { + LOG.debug( + "Failed to clone the AstElement. Returning same reference; the client may fail. {} - {}", + e.getClass().getName(), e); + } + + return clone; + } + } diff --git a/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/ChildNodeImpl.java b/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/ChildNodeImpl.java index a053ada..385a408 100644 --- a/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/ChildNodeImpl.java +++ b/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/ChildNodeImpl.java @@ -95,4 +95,8 @@ public class ChildNodeImpl extends ConstraintImpl { } } + @Override + public AstElement copyOf() { + return new ChildNodeImpl(selectorName, parentPath); + } } \ No newline at end of file diff --git a/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/ChildNodeJoinConditionImpl.java b/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/ChildNodeJoinConditionImpl.java index 2994549..b91c484 100644 --- a/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/ChildNodeJoinConditionImpl.java +++ b/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/ChildNodeJoinConditionImpl.java @@ -103,4 +103,8 @@ public class ChildNodeJoinConditionImpl extends JoinConditionImpl { return available.contains(childSelector) && available.contains(parentSelector); } + @Override + public AstElement copyOf() { + return new ChildNodeJoinConditionImpl(childSelectorName, parentSelectorName); + } } \ No newline at end of file diff --git a/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/ColumnImpl.java b/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/ColumnImpl.java index ed806a8..e60736a 100644 --- a/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/ColumnImpl.java +++ b/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/ColumnImpl.java @@ -67,4 +67,8 @@ public class ColumnImpl extends AstElement { return selector; } + @Override + public AstElement copyOf() { + return new ColumnImpl(selectorName, propertyName, columnName); + } } diff --git a/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/ComparisonImpl.java b/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/ComparisonImpl.java index 51531a3..90ee031 100644 --- a/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/ComparisonImpl.java +++ b/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/ComparisonImpl.java @@ -30,11 +30,14 @@ import org.apache.jackrabbit.oak.plugins.memory.PropertyStates; import org.apache.jackrabbit.oak.query.fulltext.LikePattern; import org.apache.jackrabbit.oak.query.index.FilterImpl; import org.apache.jackrabbit.oak.spi.query.PropertyValues; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; /** * A comparison operation (including "like"). */ public class ComparisonImpl extends ConstraintImpl { + private static final Logger LOG = LoggerFactory.getLogger(ComparisonImpl.class); private final DynamicOperandImpl operand1; private final Operator operator; @@ -195,4 +198,8 @@ public class ComparisonImpl extends ConstraintImpl { } } + @Override + public AstElement copyOf() { + return new ComparisonImpl(operand1.createCopy(), operator, operand2); + } } diff --git a/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/ConstraintImpl.java b/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/ConstraintImpl.java index 2561730..fffcf88 100644 --- a/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/ConstraintImpl.java +++ b/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/ConstraintImpl.java @@ -16,8 +16,13 @@ */ package org.apache.jackrabbit.oak.query.ast; +import java.util.Collections; +import java.util.List; import java.util.Set; +import javax.annotation.Nonnull; +import javax.annotation.Nullable; + import org.apache.jackrabbit.oak.query.fulltext.FullTextExpression; import org.apache.jackrabbit.oak.query.index.FilterImpl; @@ -136,4 +141,34 @@ public abstract class ConstraintImpl extends AstElement { return toString().hashCode(); } + /** + * + * @return the list of {@link ConstraintImpl} that the current constraint could hold. Default + * implementation returns {@code null}. + */ + @Nullable + public List getConstraints() { + return null; + } + + /** + *

+ * Compute a Set of sub-constraints that could be used for composing UNION statements. For + * example in case of {@code OR (c1, c2)} it will return to the caller {@code [c1, c2]}. Those + * can be later on used for re-composing conditions. + *

+ *

+ * If no union optimisations are possible it must return an empty set. + *

+ *

+ * Default implementation in {@link ConstraintImpl#simplifyForUnion()} always return an empty + * set. + *

+ * + * @return + */ + @Nonnull + public Set simplifyForUnion() { + return Collections.emptySet(); + } } diff --git a/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/DescendantNodeImpl.java b/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/DescendantNodeImpl.java index 85aa73b..c2f7098 100644 --- a/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/DescendantNodeImpl.java +++ b/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/DescendantNodeImpl.java @@ -92,4 +92,8 @@ public class DescendantNodeImpl extends ConstraintImpl { } } + @Override + public AstElement copyOf() { + return new DescendantNodeImpl(selectorName, ancestorPath); + } } diff --git a/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/DescendantNodeJoinConditionImpl.java b/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/DescendantNodeJoinConditionImpl.java index d0f7e0e..e8a5f66 100644 --- a/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/DescendantNodeJoinConditionImpl.java +++ b/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/DescendantNodeJoinConditionImpl.java @@ -97,4 +97,9 @@ public class DescendantNodeJoinConditionImpl extends JoinConditionImpl { return available.contains(descendantSelector) && available.contains(ancestorSelector); } + @Override + public AstElement copyOf() { + return new DescendantNodeJoinConditionImpl(descendantSelectorName, ancestorSelectorName); + } + } diff --git a/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/EquiJoinConditionImpl.java b/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/EquiJoinConditionImpl.java index 1e26c73..6908019 100644 --- a/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/EquiJoinConditionImpl.java +++ b/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/EquiJoinConditionImpl.java @@ -172,4 +172,8 @@ public class EquiJoinConditionImpl extends JoinConditionImpl { return available.contains(selector1) && available.contains(selector2); } + @Override + public AstElement copyOf() { + return new EquiJoinConditionImpl(selector1Name, property1Name, selector2Name, property2Name); + } } diff --git a/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/FullTextSearchImpl.java b/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/FullTextSearchImpl.java index 20f8da7..c892c9a 100644 --- a/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/FullTextSearchImpl.java +++ b/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/FullTextSearchImpl.java @@ -303,4 +303,9 @@ public class FullTextSearchImpl extends ConstraintImpl { void restrictPropertyOnFilter(String propertyName, FilterImpl f) { f.restrictProperty(propertyName, Operator.NOT_EQUAL, null); } + + @Override + public AstElement copyOf() { + return new FullTextSearchImpl(selectorName, propertyName, fullTextSearchExpression); + } } diff --git a/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/InImpl.java b/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/InImpl.java index d306d92..cea92e4 100644 --- a/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/InImpl.java +++ b/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/InImpl.java @@ -179,4 +179,8 @@ public class InImpl extends ConstraintImpl { return operand1.hashCode(); } + @Override + public AstElement copyOf() { + return new InImpl(operand1.createCopy(), operand2); + } } diff --git a/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/JoinImpl.java b/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/JoinImpl.java index 528ed90..a6dca52 100644 --- a/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/JoinImpl.java +++ b/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/JoinImpl.java @@ -13,6 +13,8 @@ */ package org.apache.jackrabbit.oak.query.ast; +import static org.apache.jackrabbit.oak.query.ast.AstElementFactory.copyElementAndCheckReference; + import java.util.ArrayList; import java.util.List; @@ -27,7 +29,6 @@ import org.apache.jackrabbit.oak.spi.state.NodeState; * source, the join type, and the join condition. */ public class JoinImpl extends SourceImpl { - private final JoinConditionImpl joinCondition; private JoinType joinType; private SourceImpl left; @@ -285,4 +286,13 @@ public class JoinImpl extends SourceImpl { return -1; } + @Override + public AstElement copyOf() { + return new JoinImpl( + (SourceImpl) copyElementAndCheckReference(left), + (SourceImpl) copyElementAndCheckReference(right), + joinType, + (JoinConditionImpl) copyElementAndCheckReference(joinCondition) + ); + } } diff --git a/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/NativeFunctionImpl.java b/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/NativeFunctionImpl.java index 083d7ec..303f139 100644 --- a/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/NativeFunctionImpl.java +++ b/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/NativeFunctionImpl.java @@ -107,4 +107,8 @@ public class NativeFunctionImpl extends ConstraintImpl { return nativeSearchExpression; } + @Override + public AstElement copyOf() { + return new NativeFunctionImpl(selectorName, language, nativeSearchExpression); + } } diff --git a/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/NotImpl.java b/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/NotImpl.java index 40623d8..79b5fc5 100644 --- a/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/NotImpl.java +++ b/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/NotImpl.java @@ -18,6 +18,9 @@ */ package org.apache.jackrabbit.oak.query.ast; +import static com.google.common.collect.Lists.newArrayList; +import static org.apache.jackrabbit.oak.query.ast.AstElementFactory.copyElementAndCheckReference; + import java.util.Collections; import java.util.Set; @@ -110,4 +113,8 @@ public class NotImpl extends ConstraintImpl { // TODO convert NOT conditions } + @Override + public AstElement copyOf() { + return new NotImpl((ConstraintImpl) copyElementAndCheckReference(constraint)); + } } 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 821a01b..1f27fc3 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 @@ -23,6 +23,7 @@ import static com.google.common.collect.Lists.newArrayList; import static com.google.common.collect.Maps.newLinkedHashMap; import static com.google.common.collect.Sets.newHashSet; import static com.google.common.collect.Sets.newLinkedHashSet; +import static org.apache.jackrabbit.oak.query.ast.AstElementFactory.copyElementAndCheckReference; import static org.apache.jackrabbit.oak.query.ast.Operator.EQUAL; import java.util.Arrays; @@ -37,6 +38,8 @@ import org.apache.jackrabbit.oak.query.fulltext.FullTextExpression; import org.apache.jackrabbit.oak.query.fulltext.FullTextOr; import org.apache.jackrabbit.oak.query.index.FilterImpl; +import com.google.common.collect.Sets; + /** * An "or" condition. */ @@ -53,6 +56,7 @@ public class OrImpl extends ConstraintImpl { this(Arrays.asList(constraint1, constraint2)); } + @Override public List getConstraints() { return constraints; } @@ -345,4 +349,26 @@ public class OrImpl extends ConstraintImpl { return constraints.hashCode(); } + @Override + public AstElement copyOf() { + List clone = newArrayList(); + for (ConstraintImpl c : constraints) { + clone.add((ConstraintImpl) copyElementAndCheckReference(c)); + } + return new OrImpl(clone); + } + + @Override + public Set simplifyForUnion() { + Set cc = Sets.newHashSet(); + for (ConstraintImpl c : getConstraints()) { + Set ccc = c.simplifyForUnion(); + if (ccc.isEmpty()) { + cc.add(c); + } else { + cc.addAll(ccc); + } + } + return cc; + } } diff --git a/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/PropertyExistenceImpl.java b/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/PropertyExistenceImpl.java index 8dbd367..ca17395 100644 --- a/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/PropertyExistenceImpl.java +++ b/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/PropertyExistenceImpl.java @@ -114,4 +114,8 @@ public class PropertyExistenceImpl extends ConstraintImpl { return a == null || b == null ? a == b : a.equals(b); } + @Override + public AstElement copyOf() { + return new PropertyExistenceImpl(selectorName, propertyName); + } } diff --git a/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/PropertyInexistenceImpl.java b/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/PropertyInexistenceImpl.java index 259c386..6ad156b 100644 --- a/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/PropertyInexistenceImpl.java +++ b/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/PropertyInexistenceImpl.java @@ -165,4 +165,8 @@ public class PropertyInexistenceImpl extends ConstraintImpl { return a == null || b == null ? a == b : a.equals(b); } + @Override + public AstElement copyOf() { + return new PropertyInexistenceImpl(selectorName, propertyName); + } } \ No newline at end of file diff --git a/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/PropertyValueImpl.java b/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/PropertyValueImpl.java index 0c30068..1f365d3 100644 --- a/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/PropertyValueImpl.java +++ b/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/PropertyValueImpl.java @@ -155,5 +155,4 @@ public class PropertyValueImpl extends DynamicOperandImpl { public PropertyValueImpl createCopy() { return new PropertyValueImpl(selectorName, propertyName); } - } diff --git a/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/SameNodeImpl.java b/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/SameNodeImpl.java index 57995e3..c84686a 100644 --- a/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/SameNodeImpl.java +++ b/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/SameNodeImpl.java @@ -84,4 +84,8 @@ public class SameNodeImpl extends ConstraintImpl { } } + @Override + public AstElement copyOf() { + return new SameNodeImpl(selectorName, path); + } } diff --git a/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/SelectorImpl.java b/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/SelectorImpl.java index a7ecce1..58ba983 100644 --- a/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/SelectorImpl.java +++ b/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/SelectorImpl.java @@ -787,4 +787,8 @@ public class SelectorImpl extends SourceImpl { return cursor.getSize(precision, max); } + @Override + public SourceImpl copyOf() { + return new SelectorImpl(nodeType, selectorName); + } } diff --git a/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/SimilarImpl.java b/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/SimilarImpl.java index ada24fd..70af239 100644 --- a/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/SimilarImpl.java +++ b/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/SimilarImpl.java @@ -127,4 +127,9 @@ public class SimilarImpl extends ConstraintImpl { return pathExpression; } + @Override + public AstElement copyOf() { + return new SimilarImpl(selectorName, propertyName, pathExpression); + } + } \ No newline at end of file diff --git a/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/SourceImpl.java b/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/SourceImpl.java index dd6f82e..4c8e009 100644 --- a/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/SourceImpl.java +++ b/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/SourceImpl.java @@ -170,5 +170,4 @@ public abstract class SourceImpl extends AstElement { * @return the size, or -1 if unknown */ public abstract long getSize(SizePrecision precision, long max); - } diff --git a/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/SpellcheckImpl.java b/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/SpellcheckImpl.java index c120c76..4a3e8cc 100644 --- a/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/SpellcheckImpl.java +++ b/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/SpellcheckImpl.java @@ -114,4 +114,9 @@ public class SpellcheckImpl extends ConstraintImpl { return expression; } + @Override + public AstElement copyOf() { + return new SpellcheckImpl(selectorName, expression); + } + } \ No newline at end of file diff --git a/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/SuggestImpl.java b/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/SuggestImpl.java index 033dd9e..f3211f3 100644 --- a/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/SuggestImpl.java +++ b/oak-core/src/main/java/org/apache/jackrabbit/oak/query/ast/SuggestImpl.java @@ -114,4 +114,8 @@ public class SuggestImpl extends ConstraintImpl { return expression; } + @Override + public AstElement copyOf() { + return new SuggestImpl(selectorName, expression); + } } \ No newline at end of file diff --git a/oak-core/src/main/java/org/apache/jackrabbit/oak/query/package-info.java b/oak-core/src/main/java/org/apache/jackrabbit/oak/query/package-info.java index 5354d47..27b2474 100644 --- a/oak-core/src/main/java/org/apache/jackrabbit/oak/query/package-info.java +++ b/oak-core/src/main/java/org/apache/jackrabbit/oak/query/package-info.java @@ -14,7 +14,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -@Version("2.3") +@Version("2.4") @Export(optional = "provide:=true") package org.apache.jackrabbit.oak.query; diff --git a/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/index/property/MultiPropertyOrTestOptimisation.java b/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/index/property/MultiPropertyOrTestOptimisation.java new file mode 100644 index 0000000..c90fba2 --- /dev/null +++ b/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/index/property/MultiPropertyOrTestOptimisation.java @@ -0,0 +1,35 @@ +/* + * 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.plugins.index.property; + +import static org.apache.jackrabbit.oak.query.QueryEngineImpl.ForceOptimised.OPTIMISED; + +import org.junit.Before; + +/** + * should be executing the {@link MultiPropertyOrTest} by forcing the optimisation in place. + */ +public class MultiPropertyOrTestOptimisation extends MultiPropertyOrTest { + + @Override + @Before + public void before() throws Exception { + super.before(); + setForceOptimised(OPTIMISED); + setTraversalEnabled(false); + } +} diff --git a/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/index/property/PropertyIndexQueryTest.java b/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/index/property/PropertyIndexQueryTest.java index 7bd6a43..1c03d00 100644 --- a/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/index/property/PropertyIndexQueryTest.java +++ b/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/index/property/PropertyIndexQueryTest.java @@ -52,11 +52,21 @@ public class PropertyIndexQueryTest extends AbstractQueryTest { @Override protected ContentRepository createRepository() { + return getOakRepositoryInstance().createContentRepository(); + } + + /** + * return an instance of {@link Oak} repository ready to be built with + * {@link Oak#createContentRepository()}. + * + * @return + */ + @Nonnull + Oak getOakRepositoryInstance() { return new Oak().with(new InitialContent()) .with(new OpenSecurityProvider()) .with(new PropertyIndexProvider()) - .with(new PropertyIndexEditorProvider()) - .createContentRepository(); + .with(new PropertyIndexEditorProvider()); } @Test diff --git a/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/index/property/PropertyIndexQueryTestSQL2Optimisations.java b/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/index/property/PropertyIndexQueryTestSQL2Optimisations.java new file mode 100644 index 0000000..b4152a0 --- /dev/null +++ b/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/index/property/PropertyIndexQueryTestSQL2Optimisations.java @@ -0,0 +1,38 @@ +/* + * 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.plugins.index.property; + +import org.apache.jackrabbit.oak.Oak; +import org.apache.jackrabbit.oak.query.QueryEngineSettings; + +/** + * checks the same as {@link PropertyIndexQueryTest} enabling the feature for optimising SQL2 + * statements. + */ +public class PropertyIndexQueryTestSQL2Optimisations extends PropertyIndexQueryTest { + + @Override + Oak getOakRepositoryInstance() { + return super.getOakRepositoryInstance() + .with(new QueryEngineSettings() { + @Override + public boolean isSql2Optimisation() { + return true; + } + }); + } +} diff --git a/oak-core/src/test/java/org/apache/jackrabbit/oak/query/AbstractQueryTest.java b/oak-core/src/test/java/org/apache/jackrabbit/oak/query/AbstractQueryTest.java index fe5f1d1..aeb1f11 100644 --- a/oak-core/src/test/java/org/apache/jackrabbit/oak/query/AbstractQueryTest.java +++ b/oak-core/src/test/java/org/apache/jackrabbit/oak/query/AbstractQueryTest.java @@ -32,6 +32,7 @@ import java.util.HashSet; import java.util.List; import java.util.Map; +import javax.annotation.Nonnull; import javax.jcr.PropertyType; import com.google.common.collect.Lists; @@ -55,9 +56,11 @@ import org.apache.jackrabbit.oak.json.TypeCodes; import org.apache.jackrabbit.oak.plugins.memory.BooleanPropertyState; import org.apache.jackrabbit.oak.plugins.memory.StringPropertyState; import org.apache.jackrabbit.oak.plugins.value.Conversions; +import org.apache.jackrabbit.oak.query.QueryEngineImpl.ForceOptimised; import org.apache.jackrabbit.oak.query.xpath.XPathToSQL2Converter; import org.junit.Before; +import static com.google.common.base.Preconditions.checkNotNull; import static org.apache.jackrabbit.oak.api.QueryEngine.NO_BINDINGS; import static org.apache.jackrabbit.oak.api.QueryEngine.NO_MAPPINGS; import static org.apache.jackrabbit.oak.plugins.index.IndexConstants.INDEX_DEFINITIONS_NAME; @@ -308,19 +311,28 @@ public abstract class AbstractQueryTest { protected List assertQuery(String sql, String language, List expected, boolean skipSort) { List paths = executeQuery(sql, language, true, skipSort); - for (String p : expected) { - assertTrue("Expected path " + p + " not found, got " + paths, paths.contains(p)); - } - assertEquals("Result set size is different", expected.size(), - paths.size()); + assertResult(expected, paths); return paths; } + protected static void assertResult(@Nonnull List expected, @Nonnull List actual) { + for (String p : checkNotNull(expected)) { + assertTrue("Expected path " + p + " not found, got " + actual, checkNotNull(actual) + .contains(p)); + } + assertEquals("Result set size is different", expected.size(), + actual.size()); + } + protected void setTraversalEnabled(boolean traversalEnabled) { ((QueryEngineImpl) qe).setTraversalEnabled(traversalEnabled); } + protected void setForceOptimised(@Nonnull ForceOptimised forceOptimised) { + ((QueryEngineImpl) qe).setForceOptimised(checkNotNull(forceOptimised)); + } + protected static String readRow(ResultRow row, boolean pathOnly) { if (pathOnly) { return row.getValue(QueryImpl.JCR_PATH).getValue(Type.STRING); diff --git a/oak-core/src/test/java/org/apache/jackrabbit/oak/query/QueryCostOverheadTest.java b/oak-core/src/test/java/org/apache/jackrabbit/oak/query/QueryCostOverheadTest.java new file mode 100644 index 0000000..3f0e6ef --- /dev/null +++ b/oak-core/src/test/java/org/apache/jackrabbit/oak/query/QueryCostOverheadTest.java @@ -0,0 +1,101 @@ +/* + * 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; + +import static com.google.common.collect.ImmutableList.of; +import static junit.framework.Assert.assertEquals; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +import org.apache.jackrabbit.oak.query.ast.AndImpl; +import org.apache.jackrabbit.oak.query.ast.ComparisonImpl; +import org.apache.jackrabbit.oak.query.ast.ConstraintImpl; +import org.apache.jackrabbit.oak.query.ast.DescendantNodeImpl; +import org.apache.jackrabbit.oak.query.ast.FullTextSearchImpl; +import org.apache.jackrabbit.oak.query.ast.OrImpl; +import org.junit.Test; + +public class QueryCostOverheadTest { + @Test + public void getCostOverhead() { + final double allowedDelta = 10; + QueryImpl query; + UnionQueryImpl union; + ConstraintImpl c, c1, c2, c3, c4, c5; + + union = new UnionQueryImpl(false, null, null, null); + assertEquals("we always expect 0 from a `UnionQueryImpl`", 0, union.getCostOverhead(), + allowedDelta); + + c = mock(OrImpl.class); + c1 = mock(ComparisonImpl.class); + c2 = mock(FullTextSearchImpl.class); + when(c.getConstraints()).thenReturn(of(c1, c2)); + query = new QueryImpl(null, null, c, null, null, null); + assertEquals(Double.MAX_VALUE, query.getCostOverhead(), allowedDelta); + + c = mock(OrImpl.class); + c1 = mock(ComparisonImpl.class); + c2 = mock(FullTextSearchImpl.class); + c3 = mock(FullTextSearchImpl.class); + when(c.getConstraints()).thenReturn(of(c1, c2, c3)); + query = new QueryImpl(null, null, c, null, null, null); + assertEquals(Double.MAX_VALUE, query.getCostOverhead(), allowedDelta); + + c1 = mock(OrImpl.class); + c2 = mock(FullTextSearchImpl.class); + c3 = mock(FullTextSearchImpl.class); + c4 = mock(ComparisonImpl.class); + when(c1.getConstraints()).thenReturn(of(c2, c3, c4)); + c = mock(AndImpl.class); + c5 = mock(DescendantNodeImpl.class); + when(c.getConstraints()).thenReturn(of(c1, c5)); + query = new QueryImpl(null, null, c, null, null, null); + assertEquals(Double.MAX_VALUE, query.getCostOverhead(), allowedDelta); + + c = mock(FullTextSearchImpl.class); + query = new QueryImpl(null, null, c, null, null, null); + assertEquals(0, query.getCostOverhead(), allowedDelta); + + c = mock(OrImpl.class); + c1 = mock(FullTextSearchImpl.class); + c2 = mock(FullTextSearchImpl.class); + c3 = mock(FullTextSearchImpl.class); + when(c.getConstraints()).thenReturn(of(c1, c2, c3)); + query = new QueryImpl(null, null, c, null, null, null); + assertEquals(0, query.getCostOverhead(), allowedDelta); + + c = mock(AndImpl.class); + c1 = mock(ComparisonImpl.class); + c2 = mock(FullTextSearchImpl.class); + c3 = mock(FullTextSearchImpl.class); + when(c.getConstraints()).thenReturn(of(c1, c2, c3)); + query = new QueryImpl(null, null, c, null, null, null); + assertEquals(0, query.getCostOverhead(), allowedDelta); + + c = mock(AndImpl.class); + c1 = mock(ComparisonImpl.class); + c2 = mock(ComparisonImpl.class); + when(c.getConstraints()).thenReturn(of(c1, c2, c3)); + query = new QueryImpl(null, null, c, null, null, null); + assertEquals(0, query.getCostOverhead(), allowedDelta); + + c2 = mock(ComparisonImpl.class); + query = new QueryImpl(null, null, c, null, null, null); + assertEquals(0, query.getCostOverhead(), allowedDelta); + } +} diff --git a/oak-core/src/test/java/org/apache/jackrabbit/oak/query/SQL2OptimiseQueryTest.java b/oak-core/src/test/java/org/apache/jackrabbit/oak/query/SQL2OptimiseQueryTest.java new file mode 100644 index 0000000..a340770 --- /dev/null +++ b/oak-core/src/test/java/org/apache/jackrabbit/oak/query/SQL2OptimiseQueryTest.java @@ -0,0 +1,248 @@ +/* + * 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; + +import static com.google.common.base.Preconditions.checkNotNull; +import static com.google.common.collect.ImmutableList.of; +import static javax.jcr.query.Query.JCR_SQL2; +import static org.apache.jackrabbit.JcrConstants.JCR_PRIMARYTYPE; +import static org.apache.jackrabbit.JcrConstants.JCR_SYSTEM; +import static org.apache.jackrabbit.oak.api.Type.NAME; +import static org.apache.jackrabbit.oak.plugins.nodetype.NodeTypeConstants.JCR_NODE_TYPES; +import static org.apache.jackrabbit.oak.plugins.nodetype.NodeTypeConstants.NT_OAK_UNSTRUCTURED; +import static org.apache.jackrabbit.oak.query.QueryEngineImpl.ForceOptimised.CHEAPEST; +import static org.apache.jackrabbit.oak.query.QueryEngineImpl.ForceOptimised.OPTIMISED; +import static org.apache.jackrabbit.oak.query.QueryEngineImpl.ForceOptimised.ORIGINAL; +import static org.hamcrest.CoreMatchers.is; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNotSame; +import static org.junit.Assert.assertThat; +import static org.junit.Assert.assertTrue; + +import java.text.ParseException; +import java.util.List; + +import javax.annotation.Nonnull; +import javax.jcr.RepositoryException; + +import org.apache.jackrabbit.oak.Oak; +import org.apache.jackrabbit.oak.api.CommitFailedException; +import org.apache.jackrabbit.oak.api.ContentRepository; +import org.apache.jackrabbit.oak.api.QueryEngine; +import org.apache.jackrabbit.oak.api.Tree; +import org.apache.jackrabbit.oak.namepath.LocalNameMapper; +import org.apache.jackrabbit.oak.namepath.NamePathMapper; +import org.apache.jackrabbit.oak.namepath.NamePathMapperImpl; +import org.apache.jackrabbit.oak.plugins.memory.MemoryNodeStore; +import org.apache.jackrabbit.oak.plugins.nodetype.write.InitialContent; +import org.apache.jackrabbit.oak.spi.security.OpenSecurityProvider; +import org.apache.jackrabbit.oak.spi.state.NodeState; +import org.apache.jackrabbit.oak.spi.state.NodeStore; +import org.junit.Test; + +/** + * aim to cover the various aspects of Query.optimise() + */ +public class SQL2OptimiseQueryTest extends AbstractQueryTest { + private NodeStore store; + private QueryEngineSettings qeSettings = new QueryEngineSettings() { + @Override + public boolean isSql2Optimisation() { + return true; + } + }; + + /** + * checks the {@code Query#optimise()} calls for the conversion from OR to UNION from a query + * POV; ensuring that it returns always the same, expected resultset. + * + * @throws RepositoryException + * @throws CommitFailedException + */ + @Test + public void orToUnions() throws RepositoryException, CommitFailedException { + Tree test, t; + List original, optimised, cheapest, expected; + String statement; + + test = root.getTree("/").addChild("test"); + test.setProperty(JCR_PRIMARYTYPE, NT_OAK_UNSTRUCTURED, NAME); + t = addChildWithProperty(test, "a", "p", "a"); + t.setProperty("p1", "a1"); + t = addChildWithProperty(test, "b", "p", "b"); + t.setProperty("p1", "b1"); + t.setProperty("p2", "a"); + t = addChildWithProperty(test, "c", "p", "c"); + t.setProperty("p3", "a"); + addChildWithProperty(test, "d", "p", "d"); + addChildWithProperty(test, "e", "p", "e"); + test = root.getTree("/").addChild("test2"); + addChildWithProperty(test, "a", "p", "a"); + root.commit(); + + statement = String.format("SELECT * FROM [%s] WHERE p = 'a' OR p = 'b'", + NT_OAK_UNSTRUCTURED); + expected = of("/test/a", "/test/b", "/test2/a"); + setForceOptimised(ORIGINAL); + original = executeQuery(statement, JCR_SQL2, true); + setForceOptimised(OPTIMISED); + optimised = executeQuery(statement, JCR_SQL2, true); + setForceOptimised(CHEAPEST); + cheapest = executeQuery(statement, JCR_SQL2, true); + assertOrToUnionResults(expected, original, optimised, cheapest); + + statement = String.format( + "SELECT * FROM [%s] WHERE p = 'a' OR p = 'b' OR p = 'c' OR p = 'd' OR p = 'e' ", + NT_OAK_UNSTRUCTURED); + expected = of("/test/a", "/test/b", "/test/c", "/test/d", "/test/e", "/test2/a"); + setForceOptimised(ORIGINAL); + original = executeQuery(statement, JCR_SQL2, true); + setForceOptimised(OPTIMISED); + optimised = executeQuery(statement, JCR_SQL2, true); + setForceOptimised(CHEAPEST); + cheapest = executeQuery(statement, JCR_SQL2, true); + assertOrToUnionResults(expected, original, optimised, cheapest); + + statement = String.format( + "SELECT * FROM [%s] WHERE (p = 'a' OR p = 'b') AND (p1 = 'a1' OR p1 = 'b1')", + NT_OAK_UNSTRUCTURED); + expected = of("/test/a", "/test/b"); + setForceOptimised(ORIGINAL); + original = executeQuery(statement, JCR_SQL2, true); + setForceOptimised(OPTIMISED); + optimised = executeQuery(statement, JCR_SQL2, true); + setForceOptimised(CHEAPEST); + cheapest = executeQuery(statement, JCR_SQL2, true); + assertOrToUnionResults(expected, original, optimised, cheapest); + + statement = String.format( + "SELECT * FROM [%s] WHERE (p = 'a' AND p1 = 'a1') OR (p = 'b' AND p1 = 'b1')", + NT_OAK_UNSTRUCTURED); + expected = of("/test/a", "/test/b"); + setForceOptimised(ORIGINAL); + original = executeQuery(statement, JCR_SQL2, true); + setForceOptimised(OPTIMISED); + optimised = executeQuery(statement, JCR_SQL2, true); + setForceOptimised(CHEAPEST); + cheapest = executeQuery(statement, JCR_SQL2, true); + assertOrToUnionResults(expected, original, optimised, cheapest); + + statement = "SELECT * FROM [oak:Unstructured] AS c " + + "WHERE ( c.[p] = 'a' " + + "OR c.[p2] = 'a' " + + "OR c.[p3] = 'a') " + + "AND ISDESCENDANTNODE(c, '/test') " + + "ORDER BY added DESC"; + expected = of("/test/a", "/test/b", "/test/c"); + setForceOptimised(ORIGINAL); + original = executeQuery(statement, JCR_SQL2, true); + setForceOptimised(OPTIMISED); + optimised = executeQuery(statement, JCR_SQL2, true); + setForceOptimised(CHEAPEST); + cheapest = executeQuery(statement, JCR_SQL2, true); + assertOrToUnionResults(expected, original, optimised, cheapest); + } + + private static void assertOrToUnionResults(@Nonnull List expected, + @Nonnull List original, + @Nonnull List optimised, + @Nonnull List cheapest) { + // checks that all the three list are the expected content + assertThat(checkNotNull(original), is(checkNotNull(expected))); + assertThat(checkNotNull(optimised), is(expected)); + assertThat(checkNotNull(cheapest), is(expected)); + + // check that all the three lists contains the same. Paranoid but still a fast check + assertThat(original, is(optimised)); + assertThat(optimised, is(cheapest)); + assertThat(cheapest, is(original)); + } + + private static Tree addChildWithProperty(@Nonnull Tree father, @Nonnull String name, + @Nonnull String propName, @Nonnull String propValue) { + Tree t = checkNotNull(father).addChild(checkNotNull(name)); + t.setProperty(JCR_PRIMARYTYPE, NT_OAK_UNSTRUCTURED, NAME); + t.setProperty(checkNotNull(propName), checkNotNull(propValue)); + return t; + } + + /** + * ensure that an optimisation is available for the provided queries. + * + * @throws ParseException + */ + @Test + public void optimise() throws ParseException { + SQL2Parser parser = new SQL2Parser(getMappings(), getTypes(), qeSettings); + String statement; + Query original, optimised; + + statement = + "SELECT * FROM [nt:unstructured] AS c " + + "WHERE " + + "(c.[p1]='a' OR c.[p2]='b') "; + original = parser.parse(statement, false); + assertNotNull(original); + optimised = original.optimise(); + assertNotNull(optimised); + assertNotSame(original, optimised); + assertTrue(optimised instanceof UnionQueryImpl); + + statement = + "SELECT * FROM [nt:unstructured] AS c " + + "WHERE " + + "(c.[p1]='a' OR c.[p2]='b') " + + "AND " + + "ISDESCENDANTNODE(c, '/test') "; + original = parser.parse(statement, false); + assertNotNull(original); + optimised = original.optimise(); + assertNotNull(optimised); + assertNotSame(original, optimised); + + statement = + "SELECT * FROM [nt:unstructured] AS c " + + "WHERE " + + "(c.[p1]='a' OR c.[p2]='b' OR c.[p3]='c') " + + "AND " + + "ISDESCENDANTNODE(c, '/test') "; + original = parser.parse(statement, false); + assertNotNull(original); + optimised = original.optimise(); + assertNotNull(optimised); + assertNotSame(original, optimised); + } + + private NamePathMapper getMappings() { + return new NamePathMapperImpl( + new LocalNameMapper(root, QueryEngine.NO_MAPPINGS)); + } + + private NodeState getTypes() { + return store.getRoot().getChildNode(JCR_SYSTEM).getChildNode(JCR_NODE_TYPES); + } + + @Override + protected ContentRepository createRepository() { + store = new MemoryNodeStore(); + return new Oak(store) + .with(new OpenSecurityProvider()) + .with(new InitialContent()) + .with(qeSettings) + .createContentRepository(); + } +} diff --git a/oak-core/src/test/java/org/apache/jackrabbit/oak/query/ast/AndImplTest.java b/oak-core/src/test/java/org/apache/jackrabbit/oak/query/ast/AndImplTest.java new file mode 100644 index 0000000..504d204 --- /dev/null +++ b/oak-core/src/test/java/org/apache/jackrabbit/oak/query/ast/AndImplTest.java @@ -0,0 +1,81 @@ +/* + * 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 com.google.common.base.Preconditions.checkNotNull; +import static com.google.common.collect.ImmutableSet.of; +import static java.util.Collections.emptySet; +import static org.hamcrest.CoreMatchers.is; +import static org.junit.Assert.assertThat; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +import java.util.Set; + +import javax.annotation.Nonnull; + +import org.junit.Test; + +public class AndImplTest { + @Test + public void simplifyForUnion() { + ConstraintImpl and, op1, op2, op3, op4; + Set expected; + + op1 = mock(ComparisonImpl.class); + op2 = mock(ComparisonImpl.class); + and = new AndImpl(op1, op2); + expected = emptySet(); + assertThat(and.simplifyForUnion(), is(expected)); + + op1 = mockConstraint("op1", ComparisonImpl.class); + op2 = mockConstraint("op2", ComparisonImpl.class); + op3 = mockConstraint("op3", ComparisonImpl.class); + and = new AndImpl(new OrImpl(op1, op2), op3); + expected = of( + (ConstraintImpl) new AndImpl(op1, op3) + , (ConstraintImpl) new AndImpl(op2, op3) + ); + assertThat(and.simplifyForUnion(), is(expected)); + + op1 = mockConstraint("op1", ComparisonImpl.class); + op2 = mockConstraint("op2", ComparisonImpl.class); + op3 = mockConstraint("op3", ComparisonImpl.class); + op4 = mockConstraint("op4", ComparisonImpl.class); + and = new AndImpl(new OrImpl(new OrImpl(op1, op4), op2), op3); + expected = of( + (ConstraintImpl) new AndImpl(op1, op3) + , (ConstraintImpl) new AndImpl(op2, op3) + , (ConstraintImpl) new AndImpl(op4, op3) + ); + assertThat(and.simplifyForUnion(), is(expected)); +} + + /** + * convenience method for having better assertion messages + * + * @param toString the {@link String#toString()} message to be shown. Cannot be null; + * @param clazz the class you want Mockito to generate for you. + * @return a Mockito instance of the provided ConstraintImpl + */ + private static ConstraintImpl mockConstraint(@Nonnull String toString, + @Nonnull Class clazz) { + ConstraintImpl c = mock(checkNotNull(clazz)); + when(c.toString()).thenReturn(checkNotNull(toString)); + return c; + } +} 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 new file mode 100644 index 0000000..0416d6e --- /dev/null +++ b/oak-core/src/test/java/org/apache/jackrabbit/oak/query/ast/OrImplTest.java @@ -0,0 +1,55 @@ +/* + * 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 com.google.common.collect.ImmutableSet.of; +import static org.hamcrest.CoreMatchers.is; +import static org.junit.Assert.assertThat; +import static org.mockito.Mockito.mock; + +import java.util.Set; + +import org.junit.Test; + +public class OrImplTest { + @Test + public void simplifyForUnion() { + ConstraintImpl op1, op2, op3, op4, or; + Set expected; + + op1 = mock(ComparisonImpl.class); + op2 = mock(ComparisonImpl.class); + or = new OrImpl(op1, op2); + expected = of(op1, op2); + assertThat(or.simplifyForUnion(), 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.simplifyForUnion(), is(expected)); + + op1 = mock(ComparisonImpl.class); + op2 = mock(ComparisonImpl.class); + op3 = mock(ComparisonImpl.class); + op4 = mock(ComparisonImpl.class); + or = new OrImpl(new OrImpl(new OrImpl(op1, op4), op2), op3); + expected = of(op1, op2, op3, op4); + assertThat(or.simplifyForUnion(), is(expected)); + } +} diff --git a/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/LuceneIndexQueryTest.java b/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/LuceneIndexQueryTest.java index 8b0a924..0545787 100644 --- a/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/LuceneIndexQueryTest.java +++ b/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/LuceneIndexQueryTest.java @@ -17,12 +17,9 @@ package org.apache.jackrabbit.oak.plugins.index.lucene; import com.google.common.collect.ImmutableList; -import com.google.common.collect.Lists; import java.util.ArrayList; -import java.util.Calendar; import java.util.Iterator; -import java.util.List; import org.apache.jackrabbit.oak.Oak; import org.apache.jackrabbit.oak.api.ContentRepository; @@ -42,7 +39,6 @@ import static java.util.Arrays.asList; import static junit.framework.Assert.assertEquals; import static org.apache.jackrabbit.JcrConstants.JCR_PRIMARYTYPE; import static org.apache.jackrabbit.JcrConstants.NT_UNSTRUCTURED; -import static org.apache.jackrabbit.oak.api.Type.NAME; import static org.apache.jackrabbit.oak.api.Type.STRING; import static org.apache.jackrabbit.oak.api.Type.STRINGS; import static org.apache.jackrabbit.oak.plugins.index.lucene.TestUtil.useV2; @@ -67,19 +63,24 @@ public class LuceneIndexQueryTest extends AbstractQueryTest { props.getParent().setProperty(LuceneIndexConstants.INDEX_NODE_NAME, true); TestUtil.enablePropertyIndex(props, "c1/p", false); TestUtil.enableForFullText(props, LuceneIndexConstants.REGEX_ALL_PROPS, true); + TestUtil.enablePropertyIndex(props, "a/name", false); + TestUtil.enablePropertyIndex(props, "b/name", false); root.commit(); } @Override protected ContentRepository createRepository() { + return getOakRepo().createContentRepository(); + } + + Oak getOakRepo() { LowCostLuceneIndexProvider provider = new LowCostLuceneIndexProvider(); return new Oak().with(new InitialContent()) .with(new OpenSecurityProvider()) .with((QueryIndexProvider) provider) .with((Observer) provider) - .with(new LuceneIndexEditorProvider()) - .createContentRepository(); + .with(new LuceneIndexEditorProvider()); } @Test @@ -375,109 +376,6 @@ public class LuceneIndexQueryTest extends AbstractQueryTest { } @Test - @Ignore - public void oak2660() throws Exception { - final String name = "name"; - final String surname = "surname"; - final String description = "description"; - final String added = "added"; - final String yes = "yes"; - - Tree t; - - // re-define the lucene index - t = root.getTree("/oak:index/" + TEST_INDEX_NAME); - assertTrue(t.exists()); - t.remove(); - root.commit(); - assertFalse(root.getTree("/oak:index/" + TEST_INDEX_NAME).exists()); - - t = root.getTree("/"); - Tree indexDefn = createTestIndexNode(t, LuceneIndexConstants.TYPE_LUCENE); - useV2(indexDefn); - indexDefn.setProperty(LuceneIndexConstants.TEST_MODE, true); - - Tree props = TestUtil.newRulePropTree(indexDefn, NT_UNSTRUCTURED); - TestUtil.enablePropertyIndex(props, name, false); - TestUtil.enableForFullText(props, surname, false); - TestUtil.enableForFullText(props, description, false); - TestUtil.enableForOrdered(props, added); - - root.commit(); - - // creating the dataset - List expected = Lists.newArrayList(); - Tree content = root.getTree("/").addChild("content"); - t = content.addChild("test1"); - t.setProperty(JCR_PRIMARYTYPE, NT_UNSTRUCTURED, NAME); - t.setProperty(name, yes); - t.setProperty(surname, yes); - t.setProperty(description, yes); - t.setProperty(added, Calendar.getInstance()); - expected.add(t.getPath()); - - t = content.addChild("test2"); - t.setProperty(JCR_PRIMARYTYPE, NT_UNSTRUCTURED, NAME); - t.setProperty(name, yes); - t.setProperty(surname, yes); - t.setProperty(description, "no"); - t.setProperty(added, Calendar.getInstance()); - expected.add(t.getPath()); - - t = content.addChild("test3"); - t.setProperty(JCR_PRIMARYTYPE, NT_UNSTRUCTURED, NAME); - t.setProperty(name, yes); - t.setProperty(surname, "no"); - t.setProperty(description, "no"); - t.setProperty(added, Calendar.getInstance()); - expected.add(t.getPath()); - - t = content.addChild("test4"); - t.setProperty(JCR_PRIMARYTYPE, NT_UNSTRUCTURED, NAME); - t.setProperty(name, "no"); - t.setProperty(surname, yes); - t.setProperty(description, "no"); - t.setProperty(added, Calendar.getInstance()); - expected.add(t.getPath()); - - t = content.addChild("test5"); - t.setProperty(JCR_PRIMARYTYPE, NT_UNSTRUCTURED, NAME); - t.setProperty(name, "no"); - t.setProperty(surname, "no"); - t.setProperty(description, yes); - t.setProperty(added, Calendar.getInstance()); - expected.add(t.getPath()); - - t = content.addChild("test6"); - t.setProperty(JCR_PRIMARYTYPE, NT_UNSTRUCTURED, NAME); - t.setProperty(name, "no"); - t.setProperty(surname, "no"); - t.setProperty(description, "no"); - t.setProperty(added, Calendar.getInstance()); - - root.commit(); - - // asserting the initial state - for (String s : expected) { - assertTrue("wrong initial state", root.getTree(s).exists()); - } - - final String statement = - "SELECT * " + - "FROM [" + NT_UNSTRUCTURED + "] AS c " + - "WHERE " + - "( " + - "c.[" + name + "] = '" + yes + "' " + - "OR CONTAINS(c.[" + surname + "], '" + yes + "') " + - "OR CONTAINS(c.[" + description + "], '" + yes + "') " + - ") " + - "AND ISDESCENDANTNODE(c, '" + content.getPath() + "') " + - "ORDER BY " + added + " DESC "; - - assertQuery(statement, SQL2, expected); - } - - @Test public void testMultiValuedPropUpdate() throws Exception { Tree test = root.getTree("/").addChild("test"); String child = "child"; diff --git a/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/LuceneIndexQueryTestSQL2Optimisation.java b/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/LuceneIndexQueryTestSQL2Optimisation.java new file mode 100644 index 0000000..eaca3d8 --- /dev/null +++ b/oak-lucene/src/test/java/org/apache/jackrabbit/oak/plugins/index/lucene/LuceneIndexQueryTestSQL2Optimisation.java @@ -0,0 +1,149 @@ +/* + * 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.plugins.index.lucene; + +import static org.apache.jackrabbit.JcrConstants.JCR_PRIMARYTYPE; +import static org.apache.jackrabbit.JcrConstants.NT_UNSTRUCTURED; +import static org.apache.jackrabbit.oak.api.Type.NAME; +import static org.apache.jackrabbit.oak.plugins.index.lucene.TestUtil.useV2; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; + +import java.util.Calendar; +import java.util.List; + +import org.apache.jackrabbit.oak.Oak; +import org.apache.jackrabbit.oak.api.Tree; +import org.apache.jackrabbit.oak.query.QueryEngineSettings; +import org.junit.Test; + +import com.google.common.collect.Lists; + +public class LuceneIndexQueryTestSQL2Optimisation extends LuceneIndexQueryTest { + + @Override + Oak getOakRepo() { + return super.getOakRepo().with(new QueryEngineSettings() { + @Override + public boolean isSql2Optimisation() { + return true; + } + }); + } + + @Test + public void oak2660() throws Exception { + final String name = "name"; + final String surname = "surname"; + final String description = "description"; + final String added = "added"; + final String yes = "yes"; + + Tree t; + + // re-define the lucene index + t = root.getTree("/oak:index/" + TEST_INDEX_NAME); + assertTrue(t.exists()); + t.remove(); + root.commit(); + assertFalse(root.getTree("/oak:index/" + TEST_INDEX_NAME).exists()); + + t = root.getTree("/"); + Tree indexDefn = createTestIndexNode(t, LuceneIndexConstants.TYPE_LUCENE); + useV2(indexDefn); + indexDefn.setProperty(LuceneIndexConstants.TEST_MODE, true); + + Tree props = TestUtil.newRulePropTree(indexDefn, NT_UNSTRUCTURED); + TestUtil.enablePropertyIndex(props, name, false); + TestUtil.enableForFullText(props, surname, false); + TestUtil.enableForFullText(props, description, false); + TestUtil.enableForOrdered(props, added); + + root.commit(); + + // creating the dataset + List expected = Lists.newArrayList(); + Tree content = root.getTree("/").addChild("content"); + t = content.addChild("test1"); + t.setProperty(JCR_PRIMARYTYPE, NT_UNSTRUCTURED, NAME); + t.setProperty(name, yes); + t.setProperty(surname, yes); + t.setProperty(description, yes); + t.setProperty(added, Calendar.getInstance()); + expected.add(t.getPath()); + + t = content.addChild("test2"); + t.setProperty(JCR_PRIMARYTYPE, NT_UNSTRUCTURED, NAME); + t.setProperty(name, yes); + t.setProperty(surname, yes); + t.setProperty(description, "no"); + t.setProperty(added, Calendar.getInstance()); + expected.add(t.getPath()); + + t = content.addChild("test3"); + t.setProperty(JCR_PRIMARYTYPE, NT_UNSTRUCTURED, NAME); + t.setProperty(name, yes); + t.setProperty(surname, "no"); + t.setProperty(description, "no"); + t.setProperty(added, Calendar.getInstance()); + expected.add(t.getPath()); + + t = content.addChild("test4"); + t.setProperty(JCR_PRIMARYTYPE, NT_UNSTRUCTURED, NAME); + t.setProperty(name, "no"); + t.setProperty(surname, yes); + t.setProperty(description, "no"); + t.setProperty(added, Calendar.getInstance()); + expected.add(t.getPath()); + + t = content.addChild("test5"); + t.setProperty(JCR_PRIMARYTYPE, NT_UNSTRUCTURED, NAME); + t.setProperty(name, "no"); + t.setProperty(surname, "no"); + t.setProperty(description, yes); + t.setProperty(added, Calendar.getInstance()); + expected.add(t.getPath()); + + t = content.addChild("test6"); + t.setProperty(JCR_PRIMARYTYPE, NT_UNSTRUCTURED, NAME); + t.setProperty(name, "no"); + t.setProperty(surname, "no"); + t.setProperty(description, "no"); + t.setProperty(added, Calendar.getInstance()); + + root.commit(); + + // asserting the initial state + for (String s : expected) { + assertTrue("wrong initial state", root.getTree(s).exists()); + } + + final String statement = + "SELECT * " + + "FROM [" + NT_UNSTRUCTURED + "] AS c " + + "WHERE " + + "( " + + "c.[" + name + "] = '" + yes + "' " + + "OR CONTAINS(c.[" + surname + "], '" + yes + "') " + + "OR CONTAINS(c.[" + description + "], '" + yes + "') " + + ") " + + "AND ISDESCENDANTNODE(c, '" + content.getPath() + "') " + + "ORDER BY " + added + " DESC "; + + assertQuery(statement, SQL2, expected); + } +} diff --git a/oak-solr-core/pom.xml b/oak-solr-core/pom.xml index c5f0f5b..b3c06fd 100644 --- a/oak-solr-core/pom.xml +++ b/oak-solr-core/pom.xml @@ -60,6 +60,10 @@ org.apache.jackrabbit.core.query.ExcerptTest#testPunctuationStartsFragmentEndsWithDots org.apache.jackrabbit.core.query.ExcerptTest#testPreferPhrase org.apache.jackrabbit.oak.jcr.query.SpellcheckTest#testSpellcheckMultipleWords + + org.apache.jackrabbit.core.query.JoinTest#testJoinWithOR3 + org.apache.jackrabbit.core.query.JoinTest#testJoinWithOR4 + org.apache.jackrabbit.core.query.JoinTest#testJoinWithOR5