[FIX] XQuery: parse only once if error occurs. Closes #1200

BaseXdb · Sep 25, 2015 · fde9724 · fde9724
1 parent aff8dea
commit fde9724
Show file tree

Hide file tree

Showing 7 changed files with 85 additions and 70 deletions.
diff --git a/basex-core/src/main/java/org/basex/core/Command.java b/basex-core/src/main/java/org/basex/core/Command.java
@@ -90,7 +90,7 @@ public final void execute(final Context ctx, final OutputStream os) throws BaseX
     if(!ctx.perm(perm, data != null && !data.inMemory() ? data.meta.name : null))
       throw new BaseXException(PERM_REQUIRED_X, perm);
 
-    // set updating flag
+    // checks if the command performs updates
     updating = updating(ctx);
 
     try {

diff --git a/basex-core/src/main/java/org/basex/core/Proc.java b/basex-core/src/main/java/org/basex/core/Proc.java
@@ -21,7 +21,8 @@ public abstract class Proc {
   public boolean updating;
 
   /** Indicates if a process is currently registered. */
-  boolean registered;
+  protected boolean registered;
+
   /** Stopped flag. */
   private boolean stopped;
   /** Timer. */

diff --git a/basex-core/src/main/java/org/basex/core/cmd/AQuery.java b/basex-core/src/main/java/org/basex/core/cmd/AQuery.java
@@ -151,7 +151,7 @@ private void parse(final Performance p) throws QueryException {
    * @param query query string
    * @return result of check
    */
-  final boolean updating(final Context ctx, final String query) {
+  final boolean updates(final Context ctx, final String query) {
     try {
       final Performance p = new Performance();
       qp(query, ctx);
@@ -285,11 +285,11 @@ private void plan(final boolean comp) {
 
   @Override
   public boolean updating(final Context ctx) {
-    return args[0] != null && updating(ctx, args[0]);
+    return updates(ctx, args[0]);
   }
 
   @Override
-  public boolean updated(final Context ctx) {
+  public final boolean updated(final Context ctx) {
     return qp != null && qp.updates() != 0;
   }
 

diff --git a/basex-core/src/main/java/org/basex/core/cmd/Find.java b/basex-core/src/main/java/org/basex/core/cmd/Find.java
@@ -53,7 +53,7 @@ protected boolean run() {
 
   @Override
   public boolean updating(final Context ctx) {
-    return updating(ctx, find(args[0], ctx, root));
+    return updates(ctx, find(args[0], ctx, root));
   }
 
   @Override

diff --git a/basex-core/src/main/java/org/basex/core/cmd/InfoStorage.java b/basex-core/src/main/java/org/basex/core/cmd/InfoStorage.java
@@ -4,6 +4,7 @@
 
 import java.io.*;
 
+import org.basex.core.*;
 import org.basex.core.locks.*;
 import org.basex.core.parse.*;
 import org.basex.core.parse.Commands.Cmd;
@@ -34,15 +35,12 @@ public InfoStorage(final String... arg) {
 
   @Override
   protected boolean run() throws IOException {
-    // get arguments
-    final String start = args[0];
-    final String end = args[1];
-
+    final String first = args[0], second = args[1];
     DBNodes nodes = null;
-    if(!start.isEmpty() && toInt(start) == Integer.MIN_VALUE) {
+    if(isQuery()) {
       try {
         // evaluate input as query
-        final Value value = qp(args[0], context).value();
+        final Value value = qp(first, context).value();
         if(value instanceof DBNodes) nodes = (DBNodes) value;
       } catch(final QueryException ex) {
         error(Util.message(ex));
@@ -56,22 +54,27 @@ protected boolean run() throws IOException {
     if(nodes != null) {
       for(final int pre : nodes.pres()) dp.add(pre);
     } else {
-      int ps = 0, pe = 1000;
-      if(!start.isEmpty()) {
-        if(!end.isEmpty()) {
-          ps = toInt(start);
-          pe = toInt(end) + 1;
+      int start = 0, end = 1000;
+      if(!first.isEmpty()) {
+        if(!second.isEmpty()) {
+          start = toInt(first);
+          end = toInt(second) + 1;
         } else {
-          ps = toInt(start);
-          pe = ps + 1;
+          start = toInt(first);
+          end = start + 1;
         }
       }
-      dp.add(ps, pe);
+      dp.add(start, end);
     }
     out.print(dp.finish());
     return true;
   }
 
+  @Override
+  public boolean updating(final Context ctx) {
+    return isQuery() && updates(ctx, args[0]);
+  }
+
   @Override
   public void databases(final LockResult lr) {
     lr.read.add(DBLocking.CONTEXT);
@@ -80,10 +83,16 @@ public void databases(final LockResult lr) {
   @Override
   public void build(final CmdBuilder cb) {
     cb.init(Cmd.INFO + " " + CmdInfo.STORAGE);
-    if(!args[0].isEmpty() && toInt(args[0]) == Integer.MIN_VALUE) {
-      cb.xquery(0);
-    } else {
-      cb.arg(0).arg(1);
-    }
+    if(isQuery()) cb.xquery(0);
+    else cb.arg(0).arg(1);
+  }
+
+  /**
+   * Checks if the first argument is a query.
+   * @return result of check
+   */
+  private boolean isQuery() {
+    final String first = args[0];
+    return !first.isEmpty() && toInt(first) == Integer.MIN_VALUE;
   }
 }
diff --git a/basex-core/src/main/java/org/basex/query/QueryContext.java b/basex-core/src/main/java/org/basex/query/QueryContext.java
@@ -272,52 +272,54 @@ void check(final MainModule main, final StaticContext sc) throws QueryException
    */
   public void compile() throws QueryException {
     if(compiled) return;
-
-    // set database options
-    final StringList opts = tempOpts;
-    final int os = opts.size();
-    for(int o = 0; o < os; o += 2) {
-      try {
-        context.options.assign(opts.get(o).toUpperCase(Locale.ENGLISH), opts.get(o + 1));
-      } catch(final BaseXException ex) {
-        throw BASX_VALUE_X_X.get(null, opts.get(o), opts.get(o + 1));
+    try {
+      // set database options
+      final StringList opts = tempOpts;
+      final int os = opts.size();
+      for(int o = 0; o < os; o += 2) {
+        try {
+          context.options.assign(opts.get(o).toUpperCase(Locale.ENGLISH), opts.get(o + 1));
+        } catch(final BaseXException ex) {
+          throw BASX_VALUE_X_X.get(null, opts.get(o), opts.get(o + 1));
+        }
       }
-    }
-    // set tail call option after assignment database option
-    maxCalls = context.options.get(MainOptions.TAILCALLS);
-
-    // bind external variables
-    vars.bindExternal(this, bindings);
-
-    if(ctxItem != null) {
-      // evaluate initial expression
-      try {
-        ctxItem.compile(this);
-        value = ctxItem.cache(this).value();
-      } catch(final QueryException ex) {
-        // only {@link ParseExpr} instances may lead to a missing context
-        throw ex.error() == NOCTX_X ? CIRCCTX.get(ctxItem.info) : ex;
+      // set tail call option after assignment database option
+      maxCalls = context.options.get(MainOptions.TAILCALLS);
+
+      // bind external variables
+      vars.bindExternal(this, bindings);
+
+      if(ctxItem != null) {
+        // evaluate initial expression
+        try {
+          ctxItem.compile(this);
+          value = ctxItem.cache(this).value();
+        } catch(final QueryException ex) {
+          // only {@link ParseExpr} instances may lead to a missing context
+          throw ex.error() == NOCTX_X ? CIRCCTX.get(ctxItem.info) : ex;
+        }
+      } else {
+        // cache the initial context nodes
+        final DBNodes nodes = context.current();
+        if(nodes != null) {
+          if(!context.perm(Perm.READ, nodes.data().meta.name))
+            throw BASX_PERM_X.get(null, Perm.READ);
+          value = resources.compile(nodes);
+        }
       }
-    } else {
-      // cache the initial context nodes
-      final DBNodes nodes = context.current();
-      if(nodes != null) {
-        if(!context.perm(Perm.READ, nodes.data().meta.name))
-          throw BASX_PERM_X.get(null, Perm.READ);
-        value = resources.compile(nodes);
+
+      // if specified, convert context value to specified type
+      // [LW] should not be necessary
+      if(value != null && root.sc.contextType != null) {
+        value = root.sc.contextType.promote(this, root.sc, null, value, true);
       }
-    }
 
-    // if specified, convert context value to specified type
-    // [LW] should not be necessary
-    if(value != null && root.sc.contextType != null) {
-      value = root.sc.contextType.promote(this, root.sc, null, value, true);
+      // dynamic compilation
+      analyze();
+      info.runtime = true;
+    } finally {
+      compiled = true;
     }
-
-    // dynamic compilation
-    analyze();
-    info.runtime = true;
-    compiled = true;
   }
 
   /**
@@ -583,7 +585,7 @@ public FElem plan() {
    * Indicates that the query contains any updating expressions.
    */
   public void updating() {
-    updating = true;
+    updating = Boolean.TRUE;
   }
 
   @Override

diff --git a/basex-core/src/main/java/org/basex/query/QueryProcessor.java b/basex-core/src/main/java/org/basex/query/QueryProcessor.java
@@ -53,9 +53,12 @@ public QueryProcessor(final String query, final Context ctx) {
    */
   public void parse() throws QueryException {
     if(parsed) return;
-    qc.parseMain(query, null, sc);
-    updating = qc.updating;
-    parsed = true;
+    try {
+      qc.parseMain(query, null, sc);
+    } finally {
+      parsed = true;
+      updating = qc.updating;
+    }
   }
 
   /**