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 2014/01/09 15:18:03 UTC

svn commit: r1556809 - in /db/derby/code/trunk/java: engine/org/apache/derby/impl/sql/compile/StaticMethodCallNode.java testing/org/apache/derbyTesting/functionTests/tests/lang/InsertTest.java

Author: kahatlen
Date: Thu Jan  9 14:18:03 2014
New Revision: 1556809

URL: http://svn.apache.org/r1556809
Log:
DERBY-6443: ArrayIndexOutOfBoundsException when calling function from trigger

StaticMethodCallNode.bindExpression() was a no-op if the node had
already been bound. This caused problems for queries that need to bind
expressions multiple times (for example INSERT INTO ... SELECT).

This fix makes StaticMethodCallNode.bindExpression() work if it's
called a second time, while still making it a no-op if it is called
recursively in order to prevent infinite recursion.

Modified:
    db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/compile/StaticMethodCallNode.java
    db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/lang/InsertTest.java

Modified: db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/compile/StaticMethodCallNode.java
URL: http://svn.apache.org/viewvc/db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/compile/StaticMethodCallNode.java?rev=1556809&r1=1556808&r2=1556809&view=diff
==============================================================================
--- db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/compile/StaticMethodCallNode.java (original)
+++ db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/compile/StaticMethodCallNode.java Thu Jan  9 14:18:03 2014
@@ -107,7 +107,12 @@ class StaticMethodCallNode extends Metho
 	private int[]		 applicationParameterNumbers; 
 
 	private boolean		isSystemCode;
-	private boolean		alreadyBound;
+
+    /**
+     * This flag is true while bindExpression() is executing. It is used to
+     * avoid infinite recursion when bindExpression() is reentered.
+     */
+    private boolean isInsideBind;
 
     /**
      * Generated boolean field to hold the indicator
@@ -189,10 +194,24 @@ class StaticMethodCallNode extends Metho
 			throws StandardException
 	{
 		// for a function we can get called recursively
-		if (alreadyBound)
-			return this;
+        if (isInsideBind) {
+            return this;
+        }
 
+        isInsideBind = true;
+        try {
+            return bindExpressionMinion(fromList, subqueryList, aggregates);
+        } finally {
+            isInsideBind = false;
+        }
+    }
 
+    private JavaValueNode bindExpressionMinion(
+            FromList fromList,
+            SubqueryList subqueryList,
+            List<AggregateNode> aggregates)
+        throws StandardException
+    {
         bindParameters(fromList, subqueryList, aggregates);
 
 		
@@ -359,7 +378,6 @@ class StaticMethodCallNode extends Metho
 		resolveMethodCall( javaClassName, true );
 
 
-		alreadyBound = true;
 		if (isPrivilegeCollectionRequired())
 			getCompilerContext().addRequiredRoutinePriv(ad);
 

Modified: db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/lang/InsertTest.java
URL: http://svn.apache.org/viewvc/db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/lang/InsertTest.java?rev=1556809&r1=1556808&r2=1556809&view=diff
==============================================================================
--- db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/lang/InsertTest.java (original)
+++ db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/lang/InsertTest.java Thu Jan  9 14:18:03 2014
@@ -207,4 +207,75 @@ public class InsertTest extends BaseJDBC
                 TOO_MANY_RESULT_COLUMNS,
                 "insert into derby4449 (x) values (default, default)");
     }
+
+    /**
+     * Regression test case for DERBY-6443. INSERT statements bind the
+     * source SELECT statement twice, and the second time it would miss
+     * aggregates and subqueries if they were wrapped in a function call.
+     * This led to inconsistencies in the query tree that caused errors
+     * during execution (or assertion failures during compilation in sane
+     * builds).
+     */
+    public void testDerby6443() throws SQLException {
+        Statement s = createStatement();
+
+        // Disable auto-commit for easy cleanup of test tables (automatically
+        // rolled back in tearDown()), and create a separate schema to avoid
+        // name conflicts with other test cases.
+        setAutoCommit(false);
+        s.execute("CREATE SCHEMA d6443");
+        s.execute("SET SCHEMA d6443");
+
+        // This is the original test case provided in the bug report. It
+        // used to fail with an assert failure when compiling the trigger
+        // (in sane builds), or with an ArrayIndexOutOfBoundsException when
+        // the trigger fired (in insane builds).
+        s.execute("CREATE TABLE foo (name VARCHAR(20), val DOUBLE)");
+        s.execute("CREATE TABLE summary "
+                + "(name VARCHAR(20), aver DOUBLE, size INT)");
+        s.execute("CREATE TRIGGER trg_foo AFTER INSERT ON foo "
+                + "REFERENCING NEW TABLE AS changed FOR EACH STATEMENT "
+                + "INSERT INTO summary (name, aver, size) "
+                + "SELECT name, FLOOR(AVG(LOG10(val))), COUNT(*) "
+                + "FROM changed "
+                + "GROUP BY name");
+        s.execute("INSERT INTO foo (name, val) "
+                + "VALUES ('A', 10), ('A', 20), ('B', 30), ('C', 40)");
+        JDBC.assertFullResultSet(
+                s.executeQuery("select * from foo order by val"),
+                new String[][] {
+                    { "A", "10.0" },
+                    { "A", "20.0" },
+                    { "B", "30.0" },
+                    { "C", "40.0" },
+                });
+        JDBC.assertFullResultSet(
+                s.executeQuery("select * from summary order by name"),
+                new String[][] {
+                    { "A", "1.0", "2" },
+                    { "B", "1.0", "1" },
+                    { "C", "1.0", "1" },
+                });
+
+        // Some other problematic queries...
+
+        s.execute("create table t1(x int)");
+        s.execute("insert into t1 values 1");
+        s.execute("create table t2(x int)");
+
+        // Used to fail with assert or ArrayIndexOutOfBoundsException.
+        s.execute("insert into t2 select floor(avg(x)) from t1");
+
+        // Same here...
+        s.execute("create function f(x int) returns int language java "
+                + "parameter style java external name 'java.lang.Math.abs'");
+        s.execute("insert into t2 select f(avg(x)) from t1");
+
+        // This query used to fail with a NullPointerException.
+        s.execute("insert into t2 select f((select x from t1)) from t1");
+
+        JDBC.assertFullResultSet(
+                s.executeQuery("select * from t2"),
+                new String[][] {{"1"}, {"1"}, {"1"}});
+    }
 }