You are viewing a plain text version of this content. The canonical link for it is here.
Posted to derby-commits@db.apache.org by ka...@apache.org on 2011/05/26 14:19:00 UTC

svn commit: r1127883 - in /db/derby/code/trunk: java/engine/org/apache/derby/iapi/types/ java/engine/org/apache/derby/impl/sql/compile/ java/engine/org/apache/derby/impl/sql/execute/ tools/jar/

Author: kahatlen
Date: Thu May 26 12:18:59 2011
New Revision: 1127883

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

Remove the unneeded indirection via SqlXmlExecutor. This reduces the
amount of code and removes one object allocation per row when using an
XML operator.

Removed:
    db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/execute/SqlXmlExecutor.java
Modified:
    db/derby/code/trunk/java/engine/org/apache/derby/iapi/types/SqlXmlUtil.java
    db/derby/code/trunk/java/engine/org/apache/derby/iapi/types/XML.java
    db/derby/code/trunk/java/engine/org/apache/derby/iapi/types/XMLDataValue.java
    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/UnaryOperatorNode.java
    db/derby/code/trunk/tools/jar/extraDBMSclasses.properties

Modified: db/derby/code/trunk/java/engine/org/apache/derby/iapi/types/SqlXmlUtil.java
URL: http://svn.apache.org/viewvc/db/derby/code/trunk/java/engine/org/apache/derby/iapi/types/SqlXmlUtil.java?rev=1127883&r1=1127882&r2=1127883&view=diff
==============================================================================
--- db/derby/code/trunk/java/engine/org/apache/derby/iapi/types/SqlXmlUtil.java (original)
+++ db/derby/code/trunk/java/engine/org/apache/derby/iapi/types/SqlXmlUtil.java Thu May 26 12:18:59 2011
@@ -80,10 +80,8 @@ import javax.xml.transform.stream.Stream
  *       query expression a single time per statement, instead of
  *       having to do it for every row against which the query
  *       is evaluated.  An instance of this class is created at
- *       compile time and then passed (using "saved objects")
- *       to the appropriate operator implementation method in
- *       XML.java; see SqlXmlExecutor.java for more about the
- *       role this class plays in "saved object" processing.
+ *       compile time and then passed to the appropriate operator
+ *       implementation method in XML.java.
  *
  *    2. By keeping all XML-specific references in this one class, 
  *       we have a single "point of entry" to the XML objects--namely,

Modified: db/derby/code/trunk/java/engine/org/apache/derby/iapi/types/XML.java
URL: http://svn.apache.org/viewvc/db/derby/code/trunk/java/engine/org/apache/derby/iapi/types/XML.java?rev=1127883&r1=1127882&r2=1127883&view=diff
==============================================================================
--- db/derby/code/trunk/java/engine/org/apache/derby/iapi/types/XML.java (original)
+++ db/derby/code/trunk/java/engine/org/apache/derby/iapi/types/XML.java Thu May 26 12:18:59 2011
@@ -598,7 +598,7 @@ public class XML
      * store the _serialized_ version locally and then return
      * this XMLDataValue.
      *
-     * @param text The string value to check.
+     * @param stringValue The string value to check.
      * @param preserveWS Whether or not to preserve
      *  ignorable whitespace.
      * @param sqlxUtil Contains SQL/XML objects and util
@@ -609,9 +609,18 @@ public class XML
      *  value is returned; otherwise, an exception is thrown. 
      * @exception StandardException Thrown on error.
      */
-    public XMLDataValue XMLParse(String text, boolean preserveWS,
-        SqlXmlUtil sqlxUtil) throws StandardException
+    public XMLDataValue XMLParse(
+            StringDataValue stringValue,
+            boolean preserveWS,
+            SqlXmlUtil sqlxUtil)
+        throws StandardException
     {
+        if (stringValue.isNull()) {
+            setToNull();
+            return this;
+        }
+
+        String text = stringValue.getString();
         try {
 
             if (preserveWS) {
@@ -834,10 +843,10 @@ public class XML
      * the received XMLDataValue "result" param (assuming "result" is
      * non-null; else create a new XMLDataValue).
      *
-     * @param result The result of a previous call to this method; null
-     *  if not called yet.
      * @param sqlxUtil Contains SQL/XML objects and util methods that
      *  facilitate execution of XML-related operations
+     * @param result The result of a previous call to this method; null
+     *  if not called yet.
      * @return An XMLDataValue whose content corresponds to the serialized
      *  version of the results from evaluation of the query expression.
      *  Note: this XMLDataValue may not be storable into Derby XML
@@ -845,8 +854,8 @@ public class XML
      * @exception Exception thrown on error (and turned into a
      *  StandardException by the caller).
      */
-    public XMLDataValue XMLQuery(XMLDataValue result,
-        SqlXmlUtil sqlxUtil) throws StandardException
+    public XMLDataValue XMLQuery(SqlXmlUtil sqlxUtil, XMLDataValue result)
+            throws StandardException
     {
         if (this.isNull()) {
         // if the context is null, we return null,

Modified: db/derby/code/trunk/java/engine/org/apache/derby/iapi/types/XMLDataValue.java
URL: http://svn.apache.org/viewvc/db/derby/code/trunk/java/engine/org/apache/derby/iapi/types/XMLDataValue.java?rev=1127883&r1=1127882&r2=1127883&view=diff
==============================================================================
--- db/derby/code/trunk/java/engine/org/apache/derby/iapi/types/XMLDataValue.java (original)
+++ db/derby/code/trunk/java/engine/org/apache/derby/iapi/types/XMLDataValue.java Thu May 26 12:18:59 2011
@@ -30,7 +30,7 @@ public interface XMLDataValue extends Da
      * store the _serialized_ version locally and then return
      * this XMLDataValue.
      *
-     * @param text The string value to check.
+     * @param stringValue The string value to check.
      * @param preserveWS Whether or not to preserve
      *  ignorable whitespace.
      * @param sqlxUtil Contains SQL/XML objects and util
@@ -41,8 +41,11 @@ public interface XMLDataValue extends Da
      *  value returned; otherwise, an exception is thrown. 
      * @exception StandardException Thrown on error.
      */
-	public XMLDataValue XMLParse(String text, boolean preserveWS,
-		SqlXmlUtil sqlxUtil) throws StandardException;
+    public XMLDataValue XMLParse(
+            StringDataValue stringValue,
+            boolean preserveWS,
+            SqlXmlUtil sqlxUtil)
+        throws StandardException;
 
     /**
      * The SQL/XML XMLSerialize operator.
@@ -90,10 +93,10 @@ public interface XMLDataValue extends Da
      * the received XMLDataValue "result" param (assuming "result" is
      * non-null; else create a new XMLDataValue).
      *
-     * @param result The result of a previous call to this method; null
-     *  if not called yet.
      * @param sqlxUtil Contains SQL/XML objects and util methods that
      *  facilitate execution of XML-related operations
+     * @param result The result of a previous call to this method; null
+     *  if not called yet.
      * @return An XMLDataValue whose content corresponds to the serialized
      *  version of the results from evaluation of the query expression.
      *  Note: this XMLDataValue may not be storable into Derby XML
@@ -101,7 +104,7 @@ public interface XMLDataValue extends Da
      * @exception Exception thrown on error (and turned into a
      *  StandardException by the caller).
      */
-    public XMLDataValue XMLQuery(XMLDataValue result, SqlXmlUtil sqlxUtil)
+    public XMLDataValue XMLQuery(SqlXmlUtil sqlxUtil, XMLDataValue result)
 		throws StandardException;
 
     /* ****

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=1127883&r1=1127882&r2=1127883&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 Thu May 26 12:18:59 2011
@@ -483,30 +483,15 @@ public class BinaryOperatorNode extends 
 ** but how?
 */
 
+        // The number of arguments to pass to the method that implements the
+        // operator, depends on the type of the operator.
+        int numArgs;
+
 		// If we're dealing with XMLEXISTS or XMLQUERY, there is some
 		// additional work to be done.
 		boolean xmlGen =
 			(operatorType == XMLQUERY_OP) || (operatorType == XMLEXISTS_OP);
 
-		if (xmlGen) {
-		// We create an execution-time object so that we can retrieve
-		// saved objects (esp. our compiled query expression) from
-		// the activation.  We do this for two reasons: 1) this level
-		// of indirection allows us to separate the XML data type
-		// from the required XML implementation classes (esp. JAXP
-		// and Xalan classes)--for more on how this works, see the
-		// comments in SqlXmlUtil.java; and 2) we can take
-		// the XML query expression, which we've already compiled,
-		// and pass it to the execution-time object for each row,
-		// which means that we only have to compile the query
-		// expression once per SQL statement (instead of once per
-		// row); see SqlXmlExecutor.java for more.
-			mb.pushNewStart(
-				"org.apache.derby.impl.sql.execute.SqlXmlExecutor");
-            pushSqlXmlUtil(acb, mb, xmlQuery, operator);
-            mb.pushNewComplete(1);
-		}
-
 		/*
 		** The receiver is the operand with the higher type precedence.
 		** Like always makes the left the receiver.
@@ -544,6 +529,9 @@ public class BinaryOperatorNode extends 
 			rightOperand.generateExpression(acb, mb);
 			mb.cast(rightInterfaceType); // second arg with cast
 			// stack: left, left, right
+
+            // We've pushed two arguments
+            numArgs = 2;
 		}
 		else
 		{
@@ -567,28 +555,33 @@ public class BinaryOperatorNode extends 
 			** UNLESS we're generating an XML operator such as XMLEXISTS.
 			** In that case we want to generate
 			** 
-			**  SqlXmlExecutor.method(left, right)"
-			**
-			** and we've already pushed the SqlXmlExecutor object to
-			** the stack.
+			**  <right expression>.method(sqlXmlUtil)
 			*/
 
 			rightOperand.generateExpression(acb, mb);			
 			mb.cast(receiverType); // cast the method instance
 			// stack: right
 			
-			if (!xmlGen) {
+            if (xmlGen) {
+                // Push one argument (the SqlXmlUtil instance)
+                numArgs = 1;
+                pushSqlXmlUtil(acb, mb, xmlQuery, operator);
+                // stack: right,sqlXmlUtil
+            } else {
+                // Push two arguments (left, right)
+                numArgs = 2;
+
 				mb.dup();
 				mb.cast(rightInterfaceType);
 				// stack: right,right
-			}
-			
-			leftOperand.generateExpression(acb, mb);
-			mb.cast(leftInterfaceType); // second arg with cast
-			// stack: right,right,left
 			
-			mb.swap();
-			// stack: right,left,right			
+                leftOperand.generateExpression(acb, mb);
+                mb.cast(leftInterfaceType); // second arg with cast
+                // stack: right,right,left
+
+                mb.swap();
+                // stack: right,left,right
+            }
 		}
 
 		/* Figure out the result type name */
@@ -596,15 +589,13 @@ public class BinaryOperatorNode extends 
 			? getTypeCompiler().interfaceName()
 			: resultInterfaceType;
 
-		// Boolean return types don't need a result field
-		boolean needField = !getTypeId().isBooleanTypeId();
-
-		if (needField) {
-
-			/* Allocate an object for re-use to hold the result of the operator */
-			LocalField resultField =
-				acb.newFieldDeclaration(Modifier.PRIVATE, resultTypeName);
+        // Boolean return types don't need a result field. For other types,
+        // allocate an object for re-use to hold the result of the operator.
+        LocalField resultField = getTypeId().isBooleanTypeId() ?
+            null : acb.newFieldDeclaration(Modifier.PRIVATE, resultTypeName);
 
+        // Push the result field onto the stack, if there is a result field.
+		if (resultField != null) {
 			/*
 			** Call the method for this operator.
 			*/
@@ -613,6 +604,9 @@ public class BinaryOperatorNode extends 
 			//before generating code "field = method(p1, p2, field);"
 			initializeResultField(acb, mb, resultField);
 
+            // Adjust number of arguments for the result field
+            numArgs++;
+
 			/* pass statically calculated scale to decimal divide method to make
 			 * result set scale consistent, beetle 3901
 			 */
@@ -623,17 +617,15 @@ public class BinaryOperatorNode extends 
 				operator.equals("/"))
 			{
 				mb.push(getTypeServices().getScale());		// 4th arg
-				mb.callMethod(VMOpcode.INVOKEINTERFACE, receiverType, methodName, resultTypeName, 4);
-			}
-			else if (xmlGen) {
-			// This is for an XMLQUERY operation, so invoke the method
-			// on our execution-time object.
-				mb.callMethod(VMOpcode.INVOKEVIRTUAL, null,
-					methodName, resultTypeName, 3);
+                numArgs++;
 			}
-			else
-				mb.callMethod(VMOpcode.INVOKEINTERFACE, receiverType, methodName, resultTypeName, 3);
+        }
+
+        mb.callMethod(VMOpcode.INVOKEINTERFACE, receiverType,
+                      methodName, resultTypeName, numArgs);
 
+        // Store the result of the method call, if there is a result field.
+        if (resultField != null) {
 			//the need for following if was realized while fixing bug 5704 where decimal*decimal was resulting an overflow value but we were not detecting it
 			if (getTypeId().variableLength())//since result type is numeric variable length, generate setWidth code.
 			{
@@ -656,17 +648,6 @@ public class BinaryOperatorNode extends 
 			*/
 
 			mb.putField(resultField);
-		} else {
-			if (xmlGen) {
-			// This is for an XMLEXISTS operation, so invoke the method
-			// on our execution-time object.
-				mb.callMethod(VMOpcode.INVOKEVIRTUAL, null,
-					methodName, resultTypeName, 2);
-			}
-			else {
-				mb.callMethod(VMOpcode.INVOKEINTERFACE, receiverType,
-					methodName, resultTypeName, 2);
-			}
 		}
 	}
 

Modified: db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/compile/UnaryOperatorNode.java
URL: http://svn.apache.org/viewvc/db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/compile/UnaryOperatorNode.java?rev=1127883&r1=1127882&r2=1127883&view=diff
==============================================================================
--- db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/compile/UnaryOperatorNode.java (original)
+++ db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/compile/UnaryOperatorNode.java Thu May 26 12:18:59 2011
@@ -607,26 +607,6 @@ public class UnaryOperatorNode extends O
 											MethodBuilder mb)
 									throws StandardException
 	{
-		// For XML operator we do some extra work.
-		boolean xmlGen = (operatorType == XMLPARSE_OP) ||
-			(operatorType == XMLSERIALIZE_OP);
-
-		if (xmlGen) {
-		// We create an execution-time object from which we call
-		// the necessary methods.  We do this for two reasons: 1) this
-		// level of indirection allows us to separate the XML data type
-		// from the required XML implementation classes (esp. JAXP and
-		// Xalan classes)--for more on how this works, see the comments
-		// in SqlXmlUtil.java; and 2) this allows us to create the
-		// required XML objects a single time (which we did at bind time
-		// when we created a new SqlXmlUtil) and then reuse those objects
-		// for each row in the target result set, instead of creating
-		// new objects every time; see SqlXmlUtil.java for more.
-			mb.pushNewStart(
-				"org.apache.derby.impl.sql.execute.SqlXmlExecutor");
-			mb.pushNewComplete(addXmlOpMethodParams(acb, mb));
-		}
-
 		String resultTypeName = 
 			(operatorType == -1)
 				? getTypeCompiler().interfaceName()
@@ -647,25 +627,13 @@ public class UnaryOperatorNode extends O
 			LocalField field = acb.newFieldDeclaration(Modifier.PRIVATE, resultTypeName);
 			mb.getField(field);
 
-			/* If we're calling a method on a class (SqlXmlExecutor) instead
-			 * of calling a method on the operand interface, then we invoke
-			 * VIRTUAL; we then have 2 args (the operand and the local field)
-			 * instead of one, i.e:
-			 *
-			 *  SqlXmlExecutor.method(operand, field)
-			 *
-			 * instead of
-			 *
-			 *  <operand>.method(field).
-			 */
-			if (xmlGen) {
-				mb.callMethod(VMOpcode.INVOKEVIRTUAL, null,
-					methodName, resultTypeName, 2);
-			}
-			else {
-				mb.callMethod(VMOpcode.INVOKEINTERFACE,
-					(String) null, methodName, resultTypeName, 1);
-			}
+            int numArgs = 1;
+
+            // XML operators take extra arguments.
+            numArgs += addXmlOpMethodParams(acb, mb, field);
+
+            mb.callMethod(VMOpcode.INVOKEINTERFACE, null,
+                          methodName, resultTypeName, numArgs);
 
 			/*
 			** Store the result of the method call in the field, so we can re-use
@@ -746,11 +714,14 @@ public class UnaryOperatorNode extends O
     /**
      * Add some additional arguments to our method call for
      * XML related operations like XMLPARSE and XMLSERIALIZE.
+     *
+     * @param acb the builder for the class in which the method lives
      * @param mb The MethodBuilder that will make the call.
+     * @param resultField the field that contains the previous result
      * @return Number of parameters added.
      */
     protected int addXmlOpMethodParams(ExpressionClassBuilder acb,
-		MethodBuilder mb) throws StandardException
+		MethodBuilder mb, LocalField resultField) throws StandardException
     {
         if ((operatorType != XMLPARSE_OP) && (operatorType != XMLSERIALIZE_OP))
         // nothing to do.
@@ -781,11 +752,25 @@ public class UnaryOperatorNode extends O
 
         /* Else we're here for XMLPARSE. */
 
-        // Push the SqlXmlUtil instance as the first argument.
-        pushSqlXmlUtil(acb, mb, null, null);
+        // XMLPARSE is different from other unary operators in that the method
+        // must be called on the result object (the XML value) and not on the
+        // operand (the string value). We must therefore make sure the result
+        // object is not null.
+        MethodBuilder constructor = acb.getConstructor();
+        acb.generateNull(constructor, getTypeCompiler(),
+                         getTypeServices().getCollationType());
+        constructor.setField(resultField);
+
+        // Swap operand and result object so that the method will be called
+        // on the result object.
+        mb.swap();
 
         // Push whether or not we want to preserve whitespace.
         mb.push(((Boolean)additionalArgs[0]).booleanValue());
+
+        // Push the SqlXmlUtil instance as the next argument.
+        pushSqlXmlUtil(acb, mb, null, null);
+
         return 2;
     }
     

Modified: db/derby/code/trunk/tools/jar/extraDBMSclasses.properties
URL: http://svn.apache.org/viewvc/db/derby/code/trunk/tools/jar/extraDBMSclasses.properties?rev=1127883&r1=1127882&r2=1127883&view=diff
==============================================================================
--- db/derby/code/trunk/tools/jar/extraDBMSclasses.properties (original)
+++ db/derby/code/trunk/tools/jar/extraDBMSclasses.properties Thu May 26 12:18:59 2011
@@ -99,7 +99,5 @@ derby.module.store.jardbf=org.apache.der
 derby.module.store.urlf=org.apache.derby.impl.io.URLFile
 derby.module.store.cpf=org.apache.derby.impl.io.CPFile
 
-derby.module.xml.sqlxmle=org.apache.derby.impl.sql.execute.SqlXmlExecutor
-
 derby.module.shared.threaddump=org.apache.derby.shared.common.sanity.ThreadDump
 derby.module.engine.threaddump=org.apache.derby.iapi.error.ThreadDump