From 5096226234a58af2f546d16809a7361da872acea Mon Sep 17 00:00:00 2001 From: Adam Retter Date: Mon, 9 Oct 2023 14:30:42 +0200 Subject: [PATCH 1/8] [bugfix] Avoid NPE in exist:time pragma --- .../main/java/org/exist/xquery/pragmas/TimePragma.java | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/exist-core/src/main/java/org/exist/xquery/pragmas/TimePragma.java b/exist-core/src/main/java/org/exist/xquery/pragmas/TimePragma.java index d4e50043a21..6e8a44b6606 100644 --- a/exist-core/src/main/java/org/exist/xquery/pragmas/TimePragma.java +++ b/exist-core/src/main/java/org/exist/xquery/pragmas/TimePragma.java @@ -100,10 +100,12 @@ public void after(final XQueryContext context, final Expression expression) thro @Override public void resetState(final boolean postOptimization) { - if (timing != null && options.measurementMode == MeasurementMode.MULTIPLE) { - logMultipleMeasurement(); + if (timing != null) { + if (options.measurementMode == MeasurementMode.MULTIPLE) { + logMultipleMeasurement(); + } + this.timing.reset(); } - this.timing.reset(); } /** From d2424ea27e9908974fa9b991b88d10edb9d76250 Mon Sep 17 00:00:00 2001 From: Adam Retter Date: Mon, 9 Oct 2023 14:29:49 +0200 Subject: [PATCH 2/8] [ignore] Code cleanup --- .../exist/xquery/BasicExpressionVisitor.java | 117 +++++++++--------- .../org/exist/xquery/ExtensionExpression.java | 3 + 2 files changed, 62 insertions(+), 58 deletions(-) diff --git a/exist-core/src/main/java/org/exist/xquery/BasicExpressionVisitor.java b/exist-core/src/main/java/org/exist/xquery/BasicExpressionVisitor.java index 2260aa2596f..54cacc3bed1 100644 --- a/exist-core/src/main/java/org/exist/xquery/BasicExpressionVisitor.java +++ b/exist-core/src/main/java/org/exist/xquery/BasicExpressionVisitor.java @@ -28,19 +28,18 @@ * Basic implementation of the {@link ExpressionVisitor} interface. * This implementation will traverse a PathExpr object if it wraps * around a single other expression. All other methods are empty. - * - * @author wolf * + * @author wolf */ public class BasicExpressionVisitor implements ExpressionVisitor { @Override - public void visit(Expression expression) { + public void visit(final Expression expression) { processWrappers(expression); } @Override - public void visitCastExpr(CastExpression expression) { + public void visitCastExpr(final CastExpression expression) { //Nothing to do } @@ -50,7 +49,7 @@ public void visitCastExpr(CastExpression expression) { * expression object. */ @Override - public void visitPathExpr(PathExpr expression) { + public void visitPathExpr(final PathExpr expression) { if (expression.getLength() == 1) { final Expression next = expression.getExpression(0); next.accept(this); @@ -58,61 +57,61 @@ public void visitPathExpr(PathExpr expression) { } @Override - public void visitFunctionCall(FunctionCall call) { + public void visitFunctionCall(final FunctionCall call) { // Nothing to do } @Override - public void visitGeneralComparison(GeneralComparison comparison) { + public void visitGeneralComparison(final GeneralComparison comparison) { //Nothing to do } @Override - public void visitUnionExpr(Union union) { + public void visitUnionExpr(final Union union) { //Nothing to do } @Override - public void visitIntersectionExpr(Intersect intersect) { + public void visitIntersectionExpr(final Intersect intersect) { //Nothing to do } @Override - public void visitAndExpr(OpAnd and) { + public void visitAndExpr(final OpAnd and) { //Nothing to do } @Override - public void visitOrExpr(OpOr or) { + public void visitOrExpr(final OpOr or) { //Nothing to do } @Override - public void visitLocationStep(LocationStep locationStep) { + public void visitLocationStep(final LocationStep locationStep) { //Nothing to do } @Override - public void visitFilteredExpr(FilteredExpression filtered) { + public void visitFilteredExpr(final FilteredExpression filtered) { //Nothing to do } @Override - public void visitPredicate(Predicate predicate) { + public void visitPredicate(final Predicate predicate) { //Nothing to do } @Override - public void visitVariableReference(VariableReference ref) { + public void visitVariableReference(final VariableReference ref) { //Nothing to do } @Override - public void visitVariableDeclaration(VariableDeclaration decl) { - // Nothing to do + public void visitVariableDeclaration(final VariableDeclaration decl) { + // Nothing to do } - - protected void processWrappers(Expression expr) { + + protected void processWrappers(final Expression expr) { if (expr instanceof Atomize || expr instanceof DynamicCardinalityCheck || expr instanceof DynamicNameCheck || @@ -122,118 +121,120 @@ protected void processWrappers(Expression expr) { } } - public static LocationStep findFirstStep(Expression expr) { - if (expr instanceof LocationStep) - {return (LocationStep) expr;} + public static LocationStep findFirstStep(final Expression expr) { + if (expr instanceof LocationStep) { + return (LocationStep) expr; + } final FirstStepVisitor visitor = new FirstStepVisitor(); expr.accept(visitor); return visitor.firstStep; } - public static List findLocationSteps(Expression expr) { + public static List findLocationSteps(final Expression expr) { final List steps = new ArrayList<>(5); if (expr instanceof LocationStep) { - steps.add((LocationStep)expr); + steps.add((LocationStep) expr); return steps; } expr.accept( - new BasicExpressionVisitor() { - @Override - public void visitPathExpr(PathExpr expression) { - for (int i = 0; i < expression.getLength(); i++) { - final Expression next = expression.getExpression(i); - next.accept(this); - if (steps.size() - 1 != i) { - steps.add(null); + new BasicExpressionVisitor() { + @Override + public void visitPathExpr(final PathExpr expression) { + for (int i = 0; i < expression.getLength(); i++) { + final Expression next = expression.getExpression(i); + next.accept(this); + if (steps.size() - 1 != i) { + steps.add(null); + } } } + + @Override + public void visitLocationStep(final LocationStep locationStep) { + steps.add(locationStep); + } } - @Override - public void visitLocationStep(LocationStep locationStep) { - steps.add(locationStep); - } - } ); return steps; } - public static VariableReference findVariableRef(Expression expr) { + public static VariableReference findVariableRef(final Expression expr) { final VariableRefVisitor visitor = new VariableRefVisitor(); expr.accept(visitor); return visitor.ref; } @Override - public void visitForExpression(ForExpr forExpr) { + public void visitForExpression(final ForExpr forExpr) { //Nothing to do } @Override - public void visitLetExpression(LetExpr letExpr) { + public void visitLetExpression(final LetExpr letExpr) { //Nothing to do } @Override - public void visitOrderByClause(OrderByClause orderBy) { + public void visitOrderByClause(final OrderByClause orderBy) { // Nothing to do } @Override - public void visitGroupByClause(GroupByClause groupBy) { + public void visitGroupByClause(final GroupByClause groupBy) { // Nothing to do } @Override - public void visitWhereClause(WhereClause where) { + public void visitWhereClause(final WhereClause where) { // Nothing to do } @Override - public void visitBuiltinFunction(Function function) { + public void visitBuiltinFunction(final Function function) { //Nothing to do } @Override - public void visitUserFunction(UserDefinedFunction function) { + public void visitUserFunction(final UserDefinedFunction function) { //Nothing to do } @Override - public void visitConditional(ConditionalExpression conditional) { + public void visitConditional(final ConditionalExpression conditional) { //Nothing to do } @Override - public void visitTryCatch(TryCatchExpression conditional) { + public void visitTryCatch(final TryCatchExpression conditional) { //Nothing to do } @Override - public void visitDocumentConstructor(DocumentConstructor constructor) { - // Nothing to do + public void visitDocumentConstructor(final DocumentConstructor constructor) { + // Nothing to do } - - public void visitElementConstructor(ElementConstructor constructor) { + + public void visitElementConstructor(final ElementConstructor constructor) { //Nothing to do } @Override - public void visitTextConstructor(DynamicTextConstructor constructor) { + public void visitTextConstructor(final DynamicTextConstructor constructor) { //Nothing to do } @Override - public void visitAttribConstructor(AttributeConstructor constructor) { + public void visitAttribConstructor(final AttributeConstructor constructor) { //Nothing to do } @Override - public void visitAttribConstructor(DynamicAttributeConstructor constructor) { + public void visitAttribConstructor(final DynamicAttributeConstructor constructor) { //Nothing to do } @Override - public void visitSimpleMapOperator(OpSimpleMap simpleMap) { + public void visitSimpleMapOperator(final OpSimpleMap simpleMap) { // Nothing to do } @@ -246,7 +247,7 @@ public LocationStep getFirstStep() { } @Override - public void visitLocationStep(LocationStep locationStep) { + public void visitLocationStep(final LocationStep locationStep) { firstStep = locationStep; } } @@ -256,12 +257,12 @@ public static class VariableRefVisitor extends BasicExpressionVisitor { private VariableReference ref = null; @Override - public void visitVariableReference(VariableReference ref) { + public void visitVariableReference(final VariableReference ref) { this.ref = ref; } @Override - public void visitPathExpr(PathExpr expression) { + public void visitPathExpr(final PathExpr expression) { for (int i = 0; i < expression.getLength(); i++) { final Expression next = expression.getExpression(i); next.accept(this); diff --git a/exist-core/src/main/java/org/exist/xquery/ExtensionExpression.java b/exist-core/src/main/java/org/exist/xquery/ExtensionExpression.java index e18dcaf600e..62163a77507 100644 --- a/exist-core/src/main/java/org/exist/xquery/ExtensionExpression.java +++ b/exist-core/src/main/java/org/exist/xquery/ExtensionExpression.java @@ -91,10 +91,12 @@ private void callBefore(final Sequence contextSequence) throws XPathException { } } + @Override public int returnsType() { return innerExpression.returnsType(); } + @Override public void analyze(final AnalyzeContextInfo contextInfo) throws XPathException { final AnalyzeContextInfo newContext = new AnalyzeContextInfo(contextInfo); for (final Pragma pragma : pragmas) { @@ -103,6 +105,7 @@ public void analyze(final AnalyzeContextInfo contextInfo) throws XPathException innerExpression.analyze(newContext); } + @Override public void dump(final ExpressionDumper dumper) { for (final Pragma pragma : pragmas) { pragma.dump(dumper); From 16e9ef2e7cee34b63c8531fe8794f23cd364d09e Mon Sep 17 00:00:00 2001 From: Adam Retter Date: Mon, 9 Oct 2023 14:40:20 +0200 Subject: [PATCH 3/8] [optimize] Most Extension Expressions have only one pragma --- .../org/exist/xquery/ExtensionExpression.java | 59 +++++++++++++------ 1 file changed, 40 insertions(+), 19 deletions(-) diff --git a/exist-core/src/main/java/org/exist/xquery/ExtensionExpression.java b/exist-core/src/main/java/org/exist/xquery/ExtensionExpression.java index 62163a77507..b92b05ad67a 100644 --- a/exist-core/src/main/java/org/exist/xquery/ExtensionExpression.java +++ b/exist-core/src/main/java/org/exist/xquery/ExtensionExpression.java @@ -26,20 +26,20 @@ import org.exist.xquery.value.Item; import org.exist.xquery.value.Sequence; -import java.util.ArrayList; -import java.util.List; +import javax.annotation.Nullable; +import java.util.Arrays; /** * Implements an XQuery extension expression. An extension expression starts with - * a list of pragmas, followed by an expression enclosed in curly braces. For evaluation - * details check {{@link #eval(Sequence, Item)}. + * a one or more pragmas, followed by an expression enclosed in curly braces. For evaluation + * details check {@link #eval(Sequence, Item)}. * * @author wolf */ public class ExtensionExpression extends AbstractExpression { + @Nullable private Pragma[] pragmas = null; private Expression innerExpression; - private final List pragmas = new ArrayList<>(3); public ExtensionExpression(final XQueryContext context) { super(context); @@ -50,7 +50,12 @@ public void setExpression(final Expression inner) { } public void addPragma(final Pragma pragma) { - pragmas.add(pragma); + if (pragmas == null) { + pragmas = new Pragma[1]; + } else { + pragmas = Arrays.copyOf(pragmas, pragmas.length + 1); + } + pragmas[pragmas.length - 1] = pragma; } /** @@ -64,12 +69,14 @@ public void addPragma(final Pragma pragma) { @Override public Sequence eval(final Sequence contextSequence, final Item contextItem) throws XPathException { callBefore(contextSequence); - Sequence result = null; - for (final Pragma pragma : pragmas) { - final Sequence temp = pragma.eval(contextSequence, contextItem); - if (temp != null) { - result = temp; - break; + @Nullable Sequence result = null; + if (pragmas != null) { + for (final Pragma pragma : pragmas) { + final Sequence temp = pragma.eval(contextSequence, contextItem); + if (temp != null) { + result = temp; + break; + } } } if (result == null) { @@ -80,12 +87,20 @@ public Sequence eval(final Sequence contextSequence, final Item contextItem) thr } private void callAfter() throws XPathException { + if (pragmas == null) { + return; + } + for (final Pragma pragma : pragmas) { pragma.after(context, innerExpression); } } private void callBefore(final Sequence contextSequence) throws XPathException { + if (pragmas == null) { + return; + } + for (final Pragma pragma : pragmas) { pragma.before(context, innerExpression, contextSequence); } @@ -99,17 +114,21 @@ public int returnsType() { @Override public void analyze(final AnalyzeContextInfo contextInfo) throws XPathException { final AnalyzeContextInfo newContext = new AnalyzeContextInfo(contextInfo); - for (final Pragma pragma : pragmas) { - pragma.analyze(newContext); + if (pragmas != null) { + for (final Pragma pragma : pragmas) { + pragma.analyze(newContext); + } } innerExpression.analyze(newContext); } @Override public void dump(final ExpressionDumper dumper) { - for (final Pragma pragma : pragmas) { - pragma.dump(dumper); - dumper.nl(); + if (pragmas != null) { + for (final Pragma pragma : pragmas) { + pragma.dump(dumper); + dumper.nl(); + } } dumper.display('{'); dumper.startIndent(); @@ -148,8 +167,10 @@ public int getPrimaryAxis() { public void resetState(final boolean postOptimization) { super.resetState(postOptimization); innerExpression.resetState(postOptimization); - for (final Pragma pragma : pragmas) { - pragma.resetState(postOptimization); + if (pragmas != null) { + for (final Pragma pragma : pragmas) { + pragma.resetState(postOptimization); + } } } From 1bcf90bb259361995c190b9f58cfb35fe0c64f5f Mon Sep 17 00:00:00 2001 From: Adam Retter Date: Mon, 9 Oct 2023 14:41:52 +0200 Subject: [PATCH 4/8] [feature] Add missing #toString() method to ExtensionExpression to aid in debugging --- .../org/exist/xquery/ExtensionExpression.java | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/exist-core/src/main/java/org/exist/xquery/ExtensionExpression.java b/exist-core/src/main/java/org/exist/xquery/ExtensionExpression.java index b92b05ad67a..135646e874b 100644 --- a/exist-core/src/main/java/org/exist/xquery/ExtensionExpression.java +++ b/exist-core/src/main/java/org/exist/xquery/ExtensionExpression.java @@ -178,4 +178,21 @@ public void resetState(final boolean postOptimization) { public void accept(final ExpressionVisitor visitor) { visitor.visit(innerExpression); } + + @Override + public String toString() { + final StringBuilder result = new StringBuilder(); + + if (pragmas != null) { + for (final Pragma pragma : pragmas) { + result.append(pragma.toString()); + } + } + + result.append("{ "); + result.append(innerExpression.toString()); + result.append(" }"); + + return result.toString(); + } } From 42a32e258b7fc27391c1e76ae94efd1ac7cfa7e0 Mon Sep 17 00:00:00 2001 From: Adam Retter Date: Mon, 9 Oct 2023 16:17:58 +0200 Subject: [PATCH 5/8] [bugfix] Make sure to optimise Path Expressions that are visited by the BasicExpressionVisitor. One example, is that expressions inside of Extension Expressions were not previously optimised. --- .../src/main/java/org/exist/xquery/BasicExpressionVisitor.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/exist-core/src/main/java/org/exist/xquery/BasicExpressionVisitor.java b/exist-core/src/main/java/org/exist/xquery/BasicExpressionVisitor.java index 54cacc3bed1..2975f22c470 100644 --- a/exist-core/src/main/java/org/exist/xquery/BasicExpressionVisitor.java +++ b/exist-core/src/main/java/org/exist/xquery/BasicExpressionVisitor.java @@ -116,7 +116,8 @@ protected void processWrappers(final Expression expr) { expr instanceof DynamicCardinalityCheck || expr instanceof DynamicNameCheck || expr instanceof DynamicTypeCheck || - expr instanceof UntypedValueCheck) { + expr instanceof UntypedValueCheck || + expr instanceof PathExpr) { expr.accept(this); } } From 9e3e65ce165938a1337dfd6ab12ef779ce262728 Mon Sep 17 00:00:00 2001 From: Adam Retter Date: Mon, 9 Oct 2023 18:34:13 +0200 Subject: [PATCH 6/8] [optimize] Most predicates are not optimizable or only have a single optimizable expression --- .../main/java/org/exist/xquery/Optimizer.java | 41 +++++++++++-------- 1 file changed, 23 insertions(+), 18 deletions(-) diff --git a/exist-core/src/main/java/org/exist/xquery/Optimizer.java b/exist-core/src/main/java/org/exist/xquery/Optimizer.java index e8e163a6982..924344a7116 100644 --- a/exist-core/src/main/java/org/exist/xquery/Optimizer.java +++ b/exist-core/src/main/java/org/exist/xquery/Optimizer.java @@ -101,8 +101,8 @@ public void visitLocationStep(final LocationStep locationStep) { final FindOptimizable find = new FindOptimizable(); for (final Predicate pred : preds) { pred.accept(find); - @Nullable final List list = find.getOptimizables(); - if (list != null && canOptimize(list)) { + @Nullable final Optimizable[] list = find.getOptimizables(); + if (canOptimize(list)) { optimize = true; break; } @@ -217,8 +217,8 @@ private boolean hasOptimizable(final List preds) { final FindOptimizable find = new FindOptimizable(); for (final Predicate pred : preds) { pred.accept(find); - @Nullable final List list = find.getOptimizables(); - if (list != null && canOptimize(list)) { + @Nullable final Optimizable[] list = find.getOptimizables(); + if (canOptimize(list)) { return true; } find.reset(); @@ -345,7 +345,11 @@ public void visitVariableReference(final VariableReference ref) { } } - private boolean canOptimize(final List list) { + private boolean canOptimize(@Nullable final Optimizable[] list) { + if (list == null || list.length == 0) { + return false; + } + for (final Optimizable optimizable : list) { final int axis = optimizable.getOptimizeAxis(); if (!(axis == Constants.CHILD_AXIS || axis == Constants.DESCENDANT_AXIS || @@ -383,9 +387,9 @@ private Expression simplifyPath(final Expression expression) { * Try to find an expression object implementing interface Optimizable. */ public static class FindOptimizable extends BasicExpressionVisitor { - private List optimizables = null; + private @Nullable Optimizable[] optimizables = null; - public @Nullable List getOptimizables() { + public @Nullable Optimizable[] getOptimizables() { return optimizables; } @@ -399,10 +403,7 @@ public void visitPathExpr(final PathExpr expression) { @Override public void visitGeneralComparison(final GeneralComparison comparison) { - if (optimizables == null) { - optimizables = new ArrayList<>(4); - } - optimizables.add(comparison); + addOptimizable(comparison); } @Override @@ -413,22 +414,26 @@ public void visitPredicate(final Predicate predicate) { @Override public void visitBuiltinFunction(final Function function) { if (function instanceof final Optimizable optimizable) { - if (optimizables == null) { - optimizables = new ArrayList<>(4); - } - optimizables.add(optimizable); + addOptimizable(optimizable); } } + private void addOptimizable(final Optimizable optimizable) { + if (optimizables == null) { + optimizables = new Optimizable[1]; + } else { + optimizables = Arrays.copyOf(optimizables, optimizables.length + 1); + } + optimizables[optimizables.length - 1] = optimizable; + } + /** * Reset this visitor for reuse. * * Clears the known {@link #optimizables}. */ public void reset() { - if (optimizables != null) { - optimizables.clear(); - } + optimizables = null; } } From 2a91e8d427b8f61272db459c574306f81dd31784 Mon Sep 17 00:00:00 2001 From: Adam Retter Date: Mon, 9 Oct 2023 20:13:44 +0200 Subject: [PATCH 7/8] [optimize] We only need a single instance of FindOptimizable --- .../main/java/org/exist/xquery/Optimizer.java | 27 +++++++++++-------- 1 file changed, 16 insertions(+), 11 deletions(-) diff --git a/exist-core/src/main/java/org/exist/xquery/Optimizer.java b/exist-core/src/main/java/org/exist/xquery/Optimizer.java index 924344a7116..d231837d197 100644 --- a/exist-core/src/main/java/org/exist/xquery/Optimizer.java +++ b/exist-core/src/main/java/org/exist/xquery/Optimizer.java @@ -57,6 +57,7 @@ public class Optimizer extends DefaultExpressionVisitor { private final XQueryContext context; private final List rewriters; + private final FindOptimizable findOptimizable = new FindOptimizable(); private int predicates = 0; @@ -98,15 +99,16 @@ public void visitLocationStep(final LocationStep locationStep) { // walk through the predicates attached to the current location step. // try to find a predicate containing an expression which is an instance // of Optimizable. - final FindOptimizable find = new FindOptimizable(); for (final Predicate pred : preds) { - pred.accept(find); - @Nullable final Optimizable[] list = find.getOptimizables(); + pred.accept(findOptimizable); + @Nullable final Optimizable[] list = findOptimizable.getOptimizables(); if (canOptimize(list)) { optimize = true; + } + findOptimizable.reset(); + if (optimize) { break; } - find.reset(); } } @@ -214,16 +216,19 @@ private boolean hasOptimizable(final List preds) { // walk through the predicates attached to the current location step. // try to find a predicate containing an expression which is an instance // of Optimizable. - final FindOptimizable find = new FindOptimizable(); + boolean optimizable = false; for (final Predicate pred : preds) { - pred.accept(find); - @Nullable final Optimizable[] list = find.getOptimizables(); + pred.accept(findOptimizable); + @Nullable final Optimizable[] list = findOptimizable.getOptimizables(); if (canOptimize(list)) { - return true; + optimizable = true; + } + findOptimizable.reset(); + if (optimizable) { + break; } - find.reset(); } - return false; + return optimizable; } @Override @@ -433,7 +438,7 @@ private void addOptimizable(final Optimizable optimizable) { * Clears the known {@link #optimizables}. */ public void reset() { - optimizables = null; + this.optimizables = null; } } From d455497baa9d15a071b14e6fd34c741bb59b51c8 Mon Sep 17 00:00:00 2001 From: Adam Retter Date: Mon, 9 Oct 2023 21:51:11 +0200 Subject: [PATCH 8/8] [bugfix] Don't display the contents if it is null --- .../src/main/java/org/exist/xquery/AbstractPragma.java | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/exist-core/src/main/java/org/exist/xquery/AbstractPragma.java b/exist-core/src/main/java/org/exist/xquery/AbstractPragma.java index bd96758ba10..08057ad3aaa 100644 --- a/exist-core/src/main/java/org/exist/xquery/AbstractPragma.java +++ b/exist-core/src/main/java/org/exist/xquery/AbstractPragma.java @@ -93,6 +93,13 @@ public void resetState(final boolean postOptimization) { @Override public String toString() { - return "(# " + name + ' ' + contents + "#)"; + final StringBuilder builder = new StringBuilder(); + builder.append("(# "); + builder.append(name); + if (contents != null && !contents.isEmpty()) { + builder.append(' ').append(contents); + } + builder.append("#)"); + return builder.toString(); } }