[bugfix] Check before optimizing innerExpr. No need to optimize if it…

… is already optimized.

Closes #3624

extensions/indexes/range/src/main/java/org/exist/xquery/modules/range/Lookup.java

@@ -40,7 +40,6 @@
 import javax.annotation.Nullable;
 import java.io.IOException;
 import java.util.ArrayList;
-import java.util.Arrays;
 import java.util.Iterator;
 import java.util.List;
 
@@ -153,7 +152,7 @@ public class Lookup extends Function implements Optimizable {
  private boolean optimizeChild = false;
  private boolean usesCollation = false;
  @Nullable private Expression fallback = null;
- @Nullable private final NodePath contextPath;
+ @Nullable private NodePath contextPath;
 
  /**
  * Constructor called via reflection from {@link Function#createFunction(XQueryContext, XQueryAST, Module, FunctionDef)}.
@@ -178,6 +177,24 @@ private Lookup(final XQueryContext context, final FunctionSignature signature, @
  this.contextPath = contextPath;
  }
 
+ /**
+ * Get the node path of the optimization.
+ *
+ * @return the node path of the optimization, or null if unknown.
+ */
+ public @Nullable NodePath getContextPath() {
+ return contextPath;
+ }
+
+ /**
+ * Set the node path of the optimization.
+ *
+ * @param contextPath the node path of the optimization, or null if unknown.
+ */
+ public void setContextPath(@Nullable final NodePath contextPath) {
+ this.contextPath = contextPath;
+ }
+
  public void setFallback(@Nullable Expression expression, final int optimizeAxis) {
  if (expression instanceof final InternalFunctionCall fcall) {
  expression = fcall.getFunction();

extensions/indexes/range/src/main/java/org/exist/xquery/modules/range/RangeQueryRewriter.java

@@ -28,6 +28,7 @@
 
 import javax.annotation.Nullable;
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.List;
 
 /**
@@ -60,16 +61,16 @@ public Pragma rewriteLocationStep(final LocationStep locationStep) throws XPathE
  // will become true if optimizable expression is found
  boolean canOptimize = false;
  // get path of path expression before the predicates
- NodePath contextPath = toNodePath(getPrecedingSteps(locationStep));
+ final NodePath contextPath = toNodePath(getPrecedingSteps(locationStep));
  // process the remaining predicates
- for (Predicate pred : preds) {
+ for (final Predicate pred : preds) {
  if (pred.getLength() != 1) {
  // can only optimize predicates with one expression
  break;
  }
 
- Expression innerExpr = pred.getExpression(0);
- List<LocationStep> steps = getStepsToOptimize(innerExpr);
+ final Expression innerExpr = pred.getExpression(0);
+ final List<LocationStep> steps = getStepsToOptimize(innerExpr);
  if (steps == null || steps.isEmpty()) {
  // no optimizable steps found
  continue;
@@ -90,11 +91,11 @@ public Pragma rewriteLocationStep(final LocationStep locationStep) throws XPathE
  }
 
  // compute left hand path
- NodePath innerPath = toNodePath(steps);
+ final NodePath innerPath = toNodePath(steps);
  if (innerPath == null) {
  continue;
  }
- NodePath path;
+ final NodePath path;
  if (contextPath == null) {
  path = innerPath;
  } else {
@@ -103,14 +104,24 @@ public Pragma rewriteLocationStep(final LocationStep locationStep) throws XPathE
  }
 
  if (path.length() > 0) {
- // replace with call to lookup function
- // collect arguments
- Lookup func = rewrite(innerExpr, path);
- // preserve original comparison: may need it for in-memory lookups
- func.setFallback(innerExpr, axis);
- func.setLocation(innerExpr.getLine(), innerExpr.getColumn());
+ if (innerExpr instanceof final InternalFunctionCall internalFunctionCall
+ && internalFunctionCall.getFunction() instanceof final Lookup lookup) {
+
+ // innerExpr was already optimized, just update the contextPath if it is missing
+ if (lookup.getContextPath() == null) {
+ lookup.setContextPath(path);;
+ }
+
+ } else {
+ // replace with call to lookup function
+ final Lookup func = rewrite(innerExpr, path);
+ // preserve original comparison: may need it for in-memory lookups
+ func.setFallback(innerExpr, axis);
+ func.setLocation(innerExpr.getLine(), innerExpr.getColumn());
+
+ pred.replace(innerExpr, new InternalFunctionCall(func));
+ }
 
- pred.replace(innerExpr, new InternalFunctionCall(func));
  canOptimize = true;
  }
  }
@@ -126,28 +137,23 @@ public Pragma rewriteLocationStep(final LocationStep locationStep) throws XPathE
  return null;
  }
 
- protected static Lookup rewrite(Expression expression, NodePath path) throws XPathException {
- ArrayList<Expression> eqArgs = new ArrayList<>(2);
- if (expression instanceof GeneralComparison) {
- GeneralComparison comparison = (GeneralComparison) expression;
- eqArgs.add(comparison.getLeft());
- eqArgs.add(comparison.getRight());
- Lookup func = Lookup.create(comparison.getContext(), getOperator(expression), path);
- func.setArguments(eqArgs);
- return func;
+ protected static @Nullable Lookup rewrite(final Expression expression, final NodePath path) throws XPathException {
+ if (expression instanceof final GeneralComparison comparison) {
+ final List<Expression> eqArgs = Arrays.asList(comparison.getLeft(), comparison.getRight());
+ final Lookup lookup = Lookup.create(comparison.getContext(), getOperator(expression), path);
+ if (lookup != null) {
+ lookup.setArguments(eqArgs);
+ }
+ return lookup;
  } else if (expression instanceof InternalFunctionCall) {
- InternalFunctionCall fcall = (InternalFunctionCall) expression;
- Function function = fcall.getFunction();
- if (function instanceof Lookup) {
- if (function.isCalledAs("matches")) {
- eqArgs.add(function.getArgument(0));
- eqArgs.add(function.getArgument(1));
- Lookup func = Lookup.create(function.getContext(), RangeIndex.Operator.MATCH, path);
- func.setArguments(eqArgs);
- return func;
- }
+ final InternalFunctionCall fcall = (InternalFunctionCall) expression;
+ final Function function = fcall.getFunction();
+ if (function instanceof final Lookup lookup && lookup.getContextPath() == null) {
+ lookup.setContextPath(path);
+ return lookup;
  }
  }
+
  return null;
  }
 
@@ -159,6 +165,7 @@ protected static List<LocationStep> getStepsToOptimize(Expression expr) {
  InternalFunctionCall fcall = (InternalFunctionCall) expr;
  Function function = fcall.getFunction();
  if (function instanceof Lookup) {
+ // TODO(AR) is this check for range:matches needed here?
  if (function.isCalledAs("matches")) {
  return BasicExpressionVisitor.findLocationSteps(function.getArgument(0));
  } else {
@@ -171,16 +178,20 @@ protected static List<LocationStep> getStepsToOptimize(Expression expr) {
  }
 
  public static RangeIndex.Operator getOperator(Expression expr) {
- if (expr instanceof InternalFunctionCall) {
- InternalFunctionCall fcall = (InternalFunctionCall) expr;
- Function function = fcall.getFunction();
- if (function instanceof Lookup) {
- expr = ((Lookup)function).getFallback();
+ if (expr instanceof final InternalFunctionCall fcall) {
+ final Function function = fcall.getFunction();
+ if (function instanceof final Lookup lookup) {
+ final Expression fallback = lookup.getFallback();
+ if (fallback != null) {
+ expr = fallback;
+ } else {
+ expr = lookup;
+ }
  }
  }
+
  RangeIndex.Operator operator = RangeIndex.Operator.EQ;
- if (expr instanceof GeneralComparison) {
- GeneralComparison comparison = (GeneralComparison) expr;
+ if (expr instanceof final GeneralComparison comparison) {
  final Comparison relation = comparison.getRelation();
  switch(relation) {
  case LT:
@@ -216,15 +227,14 @@ public static RangeIndex.Operator getOperator(Expression expr) {
  break;
 
  }
- } else if (expr instanceof InternalFunctionCall) {
- InternalFunctionCall fcall = (InternalFunctionCall) expr;
- Function function = fcall.getFunction();
- if (function instanceof Lookup && function.isCalledAs("matches")) {
- operator = RangeIndex.Operator.MATCH;
- }
- } else if (expr instanceof Lookup && ((Function)expr).isCalledAs("matches")) {
+ } else if (expr instanceof final InternalFunctionCall fcall) {
+ expr = fcall.getFunction();
+ }
+
+ if (expr instanceof final Lookup lookup && lookup.isCalledAs("matches")) {
  operator = RangeIndex.Operator.MATCH;
  }
+
  return operator;
  }