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 da...@apache.org on 2010/09/16 17:27:22 UTC

svn commit: r997790 - in /db/derby/code/branches/10.6: ./ java/engine/org/apache/derby/impl/sql/compile/ java/testing/org/apache/derbyTesting/functionTests/tests/lang/ java/testing/org/apache/derbyTesting/junit/

Author: dag
Date: Thu Sep 16 15:27:22 2010
New Revision: 997790

URL: http://svn.apache.org/viewvc?rev=997790&view=rev
Log:
DERBY-4736 ASSERT FAIL when code generating a column reference in a join predicate in presence of other outer join reordering

Backport to 10.6 as

svn merge -c 987539

Patch derby-4736-1d, which fixes this bug, by adding a missing call to
bindResultColumns in SelectNode#preprocess if we have detected that
the outer join reordering has kicked in, cf call to LOJ_reorderable.

A new test case, testDerby_4736 was added to OuterJoinTest.

*and*
svn merge -c 989099

Follow-up patch derby-4736-followup-a.

In some cases, with this fix, the nullability of columns from the
null-producing (right) side of the LOJ gets set to NOT NULL after
reassociation.

The problem is that the added call to SelectNode#bindResultColumns, in
addition to calling fromList.bindResultColumns, which what we need in
for this issue, also calls super.bindResultColumns, which sets up the
datatypes over again, erroneously (i.e. without taking into
consideration the nature of outer join which can make values stemming
from otherwise NOT NULL columns be null in the final result set).

Replacing the call to SelectNode#bindResultColumns with
fromTable.bindResultColumns avoids this problem.


Modified:
    db/derby/code/branches/10.6/   (props changed)
    db/derby/code/branches/10.6/java/engine/org/apache/derby/impl/sql/compile/SelectNode.java
    db/derby/code/branches/10.6/java/testing/org/apache/derbyTesting/functionTests/tests/lang/OuterJoinTest.java
    db/derby/code/branches/10.6/java/testing/org/apache/derbyTesting/junit/RuntimeStatisticsParser.java

Propchange: db/derby/code/branches/10.6/
------------------------------------------------------------------------------
--- svn:mergeinfo (original)
+++ svn:mergeinfo Thu Sep 16 15:27:22 2010
@@ -1,2 +1,2 @@
-/db/derby/code/trunk:938547,938796,938959,939231,940462,940469,941627,942031,942286,942476,942480,942587,944152,946794,948045,948069,951346,951366,952138,952237,952581,954344,954421,954544,954748,955001,955540,955634,956075,956234,956445,956569,956659,957260,958163,958522,958555,958618,958939,959550,962716,963206,963705,964115,965647,967304,980684,986689,986834
+/db/derby/code/trunk:938547,938796,938959,939231,940462,940469,941627,942031,942286,942476,942480,942587,944152,946794,948045,948069,951346,951366,952138,952237,952581,954344,954421,954544,954748,955001,955540,955634,956075,956234,956445,956569,956659,957260,958163,958522,958555,958618,958939,959550,962716,963206,963705,964115,965647,967304,980684,986689,986834,987539,989099
 /db/derby/docs/trunk:954344

Modified: db/derby/code/branches/10.6/java/engine/org/apache/derby/impl/sql/compile/SelectNode.java
URL: http://svn.apache.org/viewvc/db/derby/code/branches/10.6/java/engine/org/apache/derby/impl/sql/compile/SelectNode.java?rev=997790&r1=997789&r2=997790&view=diff
==============================================================================
--- db/derby/code/branches/10.6/java/engine/org/apache/derby/impl/sql/compile/SelectNode.java (original)
+++ db/derby/code/branches/10.6/java/engine/org/apache/derby/impl/sql/compile/SelectNode.java Thu Sep 16 15:27:22 2010
@@ -1000,6 +1000,7 @@ public class SelectNode extends ResultSe
 																	 getNodeFactory().doJoinOrderOptimization(),
 																	 getContextManager());
 			bindExpressions(afromList);
+            fromList.bindResultColumns(afromList);
 		}
 
 		/* Preprocess the fromList.  For each FromTable, if it is a FromSubquery

Modified: db/derby/code/branches/10.6/java/testing/org/apache/derbyTesting/functionTests/tests/lang/OuterJoinTest.java
URL: http://svn.apache.org/viewvc/db/derby/code/branches/10.6/java/testing/org/apache/derbyTesting/functionTests/tests/lang/OuterJoinTest.java?rev=997790&r1=997789&r2=997790&view=diff
==============================================================================
--- db/derby/code/branches/10.6/java/testing/org/apache/derbyTesting/functionTests/tests/lang/OuterJoinTest.java (original)
+++ db/derby/code/branches/10.6/java/testing/org/apache/derbyTesting/functionTests/tests/lang/OuterJoinTest.java Thu Sep 16 15:27:22 2010
@@ -2390,4 +2390,222 @@ public final class OuterJoinTest extends
        expRS=new String[][]{};
        JDBC.assertFullResultSet(rs, expRS,true);
     }
-}
+
+
+   /**
+    * This fixture would give:
+    * <pre>
+    *   ASSERT FAILED sourceResultSetNumber expected to be >= 0 for T2.X
+    * </pre>
+    * error in sane mode prior to DERBY-4736 due to a missing rebinding
+    * operation as a result a the LOJ reordering.  Schema and query originally
+    * stems from DERBY-4712 (parent issue of DERBY-4736).
+    */
+    public void testDerby_4736() throws SQLException {
+        setAutoCommit(false);
+        Statement s = createStatement();
+
+        s.executeUpdate("create table t0(x int)");
+        s.executeUpdate("create table t1(x int)");
+        s.executeUpdate("create table t2(x int)");
+        s.executeUpdate("create table t3(x int)");
+        s.executeUpdate("create table t4(x int)");
+        s.executeUpdate("insert into t4 values(0)");
+        s.executeUpdate("insert into t4 values(1)");
+        s.executeUpdate("insert into t4 values(2)");
+        s.executeUpdate("insert into t4 values(3)");
+        s.executeUpdate("create table t5(x int)");
+        s.executeUpdate("insert into t5 values(0)");
+        s.executeUpdate("insert into t5 values(1)");
+        s.executeUpdate("insert into t5 values(2)");
+        s.executeUpdate("insert into t5 values(3)");
+        s.executeUpdate("insert into t5 values(4)");
+        s.executeUpdate("create table t6(x int)");
+        s.executeUpdate("insert into t6 values(0)");
+        s.executeUpdate("insert into t6 values(1)");
+        s.executeUpdate("insert into t6 values(2)");
+        s.executeUpdate("insert into t6 values(3)");
+        s.executeUpdate("insert into t6 values(4)");
+        s.executeUpdate("insert into t6 values(5)");
+        s.executeUpdate("create table t7(x int)");
+        s.executeUpdate("insert into t7 values(0)");
+        s.executeUpdate("insert into t7 values(1)");
+        s.executeUpdate("insert into t7 values(2)");
+        s.executeUpdate("insert into t7 values(3)");
+        s.executeUpdate("insert into t7 values(4)");
+        s.executeUpdate("insert into t7 values(5)");
+        s.executeUpdate("insert into t7 values(6)");
+        s.executeUpdate("create table t8(x int)");
+        s.executeUpdate("insert into t8 values(0)");
+        s.executeUpdate("insert into t8 values(1)");
+        s.executeUpdate("insert into t8 values(2)");
+        s.executeUpdate("insert into t8 values(3)");
+        s.executeUpdate("insert into t8 values(4)");
+        s.executeUpdate("insert into t8 values(5)");
+        s.executeUpdate("insert into t8 values(6)");
+        s.executeUpdate("insert into t8 values(7)");
+        s.executeUpdate("create table t9(x int)");
+        s.executeUpdate("insert into t9 values(0)");
+        s.executeUpdate("insert into t9 values(1)");
+        s.executeUpdate("insert into t9 values(2)");
+        s.executeUpdate("insert into t9 values(3)");
+        s.executeUpdate("insert into t9 values(4)");
+        s.executeUpdate("insert into t9 values(5)");
+        s.executeUpdate("insert into t9 values(6)");
+        s.executeUpdate("insert into t9 values(7)");
+        s.executeUpdate("insert into t9 values(8)");
+        s.executeUpdate("insert into t0 values(1)");
+        s.executeUpdate("insert into t1 values(2)");
+        s.executeUpdate("insert into t0 values(3)");
+        s.executeUpdate("insert into t1 values(3)");
+        s.executeUpdate("insert into t2 values(4)");
+        s.executeUpdate("insert into t0 values(5)");
+        s.executeUpdate("insert into t2 values(5)");
+        s.executeUpdate("insert into t1 values(6)");
+        s.executeUpdate("insert into t2 values(6)");
+        s.executeUpdate("insert into t0 values(7)");
+        s.executeUpdate("insert into t1 values(7)");
+        s.executeUpdate("insert into t2 values(7)");
+        s.executeUpdate("insert into t3 values(8)");
+        s.executeUpdate("insert into t0 values(9)");
+        s.executeUpdate("insert into t3 values(9)");
+        s.executeUpdate("insert into t1 values(10)");
+        s.executeUpdate("insert into t3 values(10)");
+        s.executeUpdate("insert into t0 values(11)");
+        s.executeUpdate("insert into t1 values(11)");
+        s.executeUpdate("insert into t3 values(11)");
+        s.executeUpdate("insert into t2 values(12)");
+        s.executeUpdate("insert into t3 values(12)");
+        s.executeUpdate("insert into t0 values(13)");
+        s.executeUpdate("insert into t2 values(13)");
+        s.executeUpdate("insert into t3 values(13)");
+        s.executeUpdate("insert into t1 values(14)");
+        s.executeUpdate("insert into t2 values(14)");
+        s.executeUpdate("insert into t3 values(14)");
+        s.executeUpdate("insert into t0 values(15)");
+        s.executeUpdate("insert into t1 values(15)");
+        s.executeUpdate("insert into t2 values(15)");
+        s.executeUpdate("insert into t3 values(15)");
+
+        s.execute("CALL SYSCS_UTIL.SYSCS_SET_RUNTIMESTATISTICS(1)");
+
+        ResultSet rs = s.executeQuery(
+            "select t0.x , t1.x , t2.x , t3.x , t4.x , t5.x , t6.x from " +
+            "    ((t0 right outer join " +
+            "         (t1 right outer join " +
+            //            t2 LOJ (t3 LOJ t4) will be reordered
+            "             (t2 left outer join " +
+            "                 (t3 left outer join t4 on t3.x = t4.x ) " +
+            "              on t2.x = t3.x ) " +
+            "          on t1.x = t3.x ) " +
+            "      on t0.x = t1.x ) " +
+            "     left outer join " +
+            "      (t5 inner join t6 on t5.x = t6.x ) " +
+            "     on t2.x = t5.x)" );
+
+        // The expected result below has been verified to the one we get if we
+        // don't reorder LOJ.
+        JDBC.assertUnorderedResultSet(
+            rs,
+            new String[][] {
+                {null, null, "4", null, null, "4", "4"},
+                {null, null, "5", null, null, null, null},
+                {null, null, "6", null, null, null, null},
+                {null, null, "7", null, null, null, null},
+                {null, null, "12", "12", null, null, null},
+                {null, null, "13", "13", null, null, null},
+                {null, "14", "14", "14", null, null, null},
+                {"15", "15", "15", "15", null, null, null}});
+
+        rs = s.executeQuery("values SYSCS_UTIL.SYSCS_GET_RUNTIMESTATISTICS()");
+        rs.next();
+        String rts = rs.getString(1);
+
+        // Now verify that we actually *did* reorder
+        RuntimeStatisticsParser rtsp = new RuntimeStatisticsParser(rts);
+        rtsp.assertSequence(
+            new String[] {
+                "_Nested Loop Left Outer Join ResultSet:",
+                "_Left result set:",
+                "__Hash Left Outer Join ResultSet:",
+                "__Left result set:",
+                "___Hash Left Outer Join ResultSet:",
+                "___Left result set:",
+                "____Hash Left Outer Join ResultSet:",
+                "____Left result set:",
+                "_____Hash Left Outer Join ResultSet:",
+                // Note: T2 and T3 are in innermost LOJ as expected
+                // whereas originally it was T3 and T4
+                "_____Left result set:",
+                "______Table Scan ResultSet for T2 ",
+                "_____Right result set:",
+                "______Hash Scan ResultSet for T3 ",
+                "____Right result set:",
+                "_____Hash Scan ResultSet for T4"});
+
+        rs.close();
+    }
+
+
+    /**
+     * Test for a follow-up patch for DERBY-4736: verify that nullability in
+     * result set metadata is correct also for columns for the null-producing
+     * side of the LOJ.
+     */
+    public void testDerby_4736_nullability() throws Exception
+    {
+        setAutoCommit(false);
+
+        Statement st = createStatement();
+        ResultSet rs = null;
+        String [][] expRS;
+        String [] expColNames;
+
+        st.executeUpdate(
+            "CREATE TABLE T (A INT NOT NULL, B DECIMAL(10,3) NOT "
+            + "NULL, C VARCHAR(5) NOT NULL)");
+
+        st.executeUpdate(
+            " INSERT INTO T VALUES (1, 1.0, '1'), (2, 2.0, '2'), "
+            + "(3, 3.0, '3')");
+
+        st.executeUpdate(
+            " CREATE TABLE S (D INT NOT NULL, E DECIMAL(10,3) "
+            + "NOT NULL, F VARCHAR(5) NOT NULL)");
+
+        st.executeUpdate(
+            " INSERT INTO S VALUES (2, 2.0, '2'), (3, 3.0, '3'), "
+            + "(4, 4.0, '4')");
+
+        st.executeUpdate(
+            "create view v1 (fv, ev, dv, cv, bv, av) as (select "
+            + "f, e, d, c, b, a from t left outer join s on b = e)");
+
+        rs = st.executeQuery(
+            " select * from t left outer join (s left outer join "
+            + "v1 on (f = cv)) on (d=a)");
+
+        expColNames = new String [] {"A", "B", "C", "D", "E", "F",
+                                     "FV", "EV", "DV", "CV", "BV", "AV"};
+        JDBC.assertColumnNames(rs, expColNames);
+
+        expRS = new String [][]
+        {
+            // Before the follow-up patch, the three first NULL column values
+            // below would get NOT NULL metadata before the follow-up patch
+            // (caught by JDBC.assertResultColumnNullable called from
+            // JDBC.assertRowInResultSet if a null value is seen).
+            //
+            {"1", "1.000", "1", null, null, null,
+             "1", "1.000", "1", null, null, null},
+
+            {"2", "2.000", "2", "2", "2.000", "2",
+             "2", "2.000", "2", "2", "2.000", "2"},
+
+            {"3", "3.000", "3", "3", "3.000", "3",
+             "3", "3.000", "3", "3", "3.000", "3"}
+        };
+
+        JDBC.assertFullResultSet(rs, expRS);
+    }
+ }

Modified: db/derby/code/branches/10.6/java/testing/org/apache/derbyTesting/junit/RuntimeStatisticsParser.java
URL: http://svn.apache.org/viewvc/db/derby/code/branches/10.6/java/testing/org/apache/derbyTesting/junit/RuntimeStatisticsParser.java?rev=997790&r1=997789&r2=997790&view=diff
==============================================================================
--- db/derby/code/branches/10.6/java/testing/org/apache/derbyTesting/junit/RuntimeStatisticsParser.java (original)
+++ db/derby/code/branches/10.6/java/testing/org/apache/derbyTesting/junit/RuntimeStatisticsParser.java Thu Sep 16 15:27:22 2010
@@ -461,5 +461,45 @@ public class RuntimeStatisticsParser {
             return null;
     }
 
+    /**
+     * Assert that a sequence of string exists in the statistics.
+     * <p>/
+     * The strings in the argument are each assumed to start a line. Leading
+     * underscores are converted to tab characters before comparing.
+     *
+     * @param strings The sequence of string expected to be found.
+     */
+    public void assertSequence(String[] strings) {
+
+        // Make strings ready for comparison:
+        for (int i=0; i < strings.length; i++) {
+            StringBuffer sb = new StringBuffer();
+
+            sb.append('\n');
+            
+            for (int j=0; j < strings[i].length(); j++) {
+                if (strings[i].charAt(j) == '_') {
+                    sb.append('\t');
+                } else {
+                    sb.append(strings[i].substring(j));
+                    break;
+                }
+            }
+            strings[i] = sb.toString();
+        }
+
+        int matchIdx = 0; // which string to match next
+        String window = statistics;
+        for (int i = 0; i < strings.length; i++) {
+            int pos = window.indexOf(strings[i]);
+
+            if (pos == -1) {
+                throw new AssertionError(
+                    "Sequence not found in statistics");
+            }
+
+            window = window.substring(pos + 1);
+        }
+    }
 }