svn commit: r1126358 - in /db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/compile: BinaryOperatorNode.java OperatorNode.java

[ka...@apache.org]

Author: kahatlen
Date: Mon May 23 07:43:50 2011
New Revision: 1126358

URL: http://svn.apache.org/viewvc?rev=1126358&view=rev
Log:
DERBY-3870: Concurrent Inserts of rows with XML data results in an exception

Remove lazy initialization of the field that holds the cached
SqlXmlUtil instance. This simplifies the generated byte code. It also
removes the need for an explicit syntax check of the XML query during
the bind phase, as the earlier initialization ensures that syntax
errors will be detected at compile time.

Modified:
    db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/compile/BinaryOperatorNode.java
    db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/compile/OperatorNode.java

Modified: db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/compile/BinaryOperatorNode.java
URL: http://svn.apache.org/viewvc/db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/compile/BinaryOperatorNode.java?rev=1126358&r1=1126357&r2=1126358&view=diff
==============================================================================
--- db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/compile/BinaryOperatorNode.java (original)
+++ db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/compile/BinaryOperatorNode.java Mon May 23 07:43:50 2011
@@ -31,7 +31,6 @@ import org.apache.derby.iapi.services.co
 import java.lang.reflect.Modifier;
 import org.apache.derby.iapi.types.TypeId;
 import org.apache.derby.iapi.types.DataTypeDescriptor;
-import org.apache.derby.iapi.types.SqlXmlUtil;
 
 import org.apache.derby.iapi.reference.ClassName;
 import org.apache.derby.iapi.reference.JDBC40Translation;
@@ -343,12 +342,6 @@ public class BinaryOperatorNode extends 
         }
         else {
             xmlQuery = ((CharConstantNode)leftOperand).getString();
-
-            // Compile the query expression. The compiled query will not be
-            // used, as each activation will need to compile its own version.
-            // But we still do this here to get a compile-time error in case
-            // the query expression has syntax errors.
-            new SqlXmlUtil().compileXQExpr(xmlQuery, operator);
         }
 
         // Right operand must be an XML data value.  NOTE: This

Modified: db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/compile/OperatorNode.java
URL: http://svn.apache.org/viewvc/db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/compile/OperatorNode.java?rev=1126358&r1=1126357&r2=1126358&view=diff
==============================================================================
--- db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/compile/OperatorNode.java (original)
+++ db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/compile/OperatorNode.java Mon May 23 07:43:50 2011
@@ -36,9 +36,8 @@ abstract class OperatorNode extends Valu
     /**
      * <p>
      * Generate code that pushes an SqlXmlUtil instance onto the stack. The
-     * instance will be created and cached in the activation the first time
-     * the code is executed, so that we don't need to create a new instance
-     * for every row.
+     * instance will be created and cached in the activation's constructor, so
+     * that we don't need to create a new instance for every row.
      * </p>
      *
      * <p>
@@ -59,37 +58,31 @@ abstract class OperatorNode extends Valu
 
         // Create a field in which the instance can be cached.
         LocalField sqlXmlUtil = acb.newFieldDeclaration(
-                Modifier.PRIVATE, SqlXmlUtil.class.getName());
+                Modifier.PRIVATE | Modifier.FINAL, SqlXmlUtil.class.getName());
 
-        // Read the cached value.
-        mb.getField(sqlXmlUtil);
-
-        // Check if the cached value is null. If it is, create a new instance.
-        // Otherwise, we're happy with the stack as it is (the cached instance
-        // will be on top of it), and nothing more is needed.
-        mb.dup();
-        mb.conditionalIfNull();
-
-        // The cached value is null. Pop it from the stack so that we can put
-        // a fresh instance there in its place.
-        mb.pop();
-
-        // Create a new instance and cache it in the field. Its value will be
-        // on the top of the stack after this sequence.
-        mb.pushNewStart(SqlXmlUtil.class.getName());
-        mb.pushNewComplete(0);
-        mb.putField(sqlXmlUtil);
-
-        // If a query is specified, compile it.
-        if (xmlQuery != null) {
-            mb.dup();
-            mb.push(xmlQuery);
-            mb.push(xmlOpName);
-            mb.callMethod(
+        // Add code that creates the SqlXmlUtil instance in the constructor.
+        MethodBuilder constructor = acb.getConstructor();
+        constructor.pushNewStart(SqlXmlUtil.class.getName());
+        constructor.pushNewComplete(0);
+        constructor.putField(sqlXmlUtil);
+
+        // Compile the query, if one is specified.
+        if (xmlQuery == null) {
+            // No query. The SqlXmlUtil instance is still on the stack. Pop it
+            // to restore the initial state of the stack.
+            constructor.pop();
+        } else {
+            // Compile the query. This will consume the SqlXmlUtil instance
+            // and leave the stack in its initial state.
+            constructor.push(xmlQuery);
+            constructor.push(xmlOpName);
+            constructor.callMethod(
                     VMOpcode.INVOKEVIRTUAL, SqlXmlUtil.class.getName(),
                     "compileXQExpr", "void", 2);
         }
 
-        mb.completeConditional();
+        // Read the cached value and push it onto the stack in the method
+        // generated for the operator.
+        mb.getField(sqlXmlUtil);
     }
 }