Return-Path: Delivered-To: apmail-db-derby-commits-archive@www.apache.org Received: (qmail 51715 invoked from network); 7 Jun 2010 14:18:41 -0000 Received: from unknown (HELO mail.apache.org) (140.211.11.3) by 140.211.11.9 with SMTP; 7 Jun 2010 14:18:41 -0000 Received: (qmail 35935 invoked by uid 500); 7 Jun 2010 14:18:41 -0000 Delivered-To: apmail-db-derby-commits-archive@db.apache.org Received: (qmail 35912 invoked by uid 500); 7 Jun 2010 14:18:40 -0000 Mailing-List: contact derby-commits-help@db.apache.org; run by ezmlm Precedence: bulk list-help: list-unsubscribe: List-Post: Reply-To: "Derby Development" List-Id: Delivered-To: mailing list derby-commits@db.apache.org Received: (qmail 35902 invoked by uid 99); 7 Jun 2010 14:18:40 -0000 Received: from nike.apache.org (HELO nike.apache.org) (192.87.106.230) by apache.org (qpsmtpd/0.29) with ESMTP; Mon, 07 Jun 2010 14:18:40 +0000 X-ASF-Spam-Status: No, hits=-2000.0 required=10.0 tests=ALL_TRUSTED X-Spam-Check-By: apache.org Received: from [140.211.11.4] (HELO eris.apache.org) (140.211.11.4) by apache.org (qpsmtpd/0.29) with ESMTP; Mon, 07 Jun 2010 14:18:37 +0000 Received: by eris.apache.org (Postfix, from userid 65534) id 1EDB5238897A; Mon, 7 Jun 2010 14:18:15 +0000 (UTC) Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: svn commit: r952237 - in /db/derby/code/trunk/java: engine/org/apache/derby/impl/sql/compile/ testing/org/apache/derbyTesting/functionTests/tests/lang/ Date: Mon, 07 Jun 2010 14:18:14 -0000 To: derby-commits@db.apache.org From: dag@apache.org X-Mailer: svnmailer-1.0.8 Message-Id: <20100607141815.1EDB5238897A@eris.apache.org> X-Virus-Checked: Checked by ClamAV on apache.org Author: dag Date: Mon Jun 7 14:18:14 2010 New Revision: 952237 URL: http://svn.apache.org/viewvc?rev=952237&view=rev Log: DERBY-4679 Several left outer joins causes unstable query with incorrect results Patch derby-4679b, which solves the following problem: When transitive closure generates new criteria into the query, it is sometimes confused by situations where the same column name appears in a result column list multiple times due to flattening of sub-queries. Flattening requires remapping of (table, column) numbers in column references. In cases where the same column name appears in a result column list multiple times, this lead to remapping (reassigning) wrong (table, column) numbers to column references in join predicates transformed to where clauses as a result of the flattening. See also DERBY-2526 and DERBY-3023 whose fixes which were partial solutions to the problem of wrong column number remappings confusing the transitive closure of search predicates performed by the preprocessing step of the optimizer. Modified: db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/compile/ColumnReference.java db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/compile/ResultColumnList.java db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/compile/VirtualColumnNode.java db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/lang/JoinTest.java Modified: db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/compile/ColumnReference.java URL: http://svn.apache.org/viewvc/db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/compile/ColumnReference.java?rev=952237&r1=952236&r2=952237&view=diff ============================================================================== --- db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/compile/ColumnReference.java (original) +++ db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/compile/ColumnReference.java Mon Jun 7 14:18:14 2010 @@ -71,6 +71,10 @@ public class ColumnReference extends Val int origTableNumber = -1; int origColumnNumber = -1; + /* For remembering original (tn,cn) of this CR during join flattening. */ + private int tableNumberBeforeFlattening = -1; + private int columnNumberBeforeFlattening = -1; + /* Reuse generated code where possible */ //Expression genResult; @@ -850,12 +854,6 @@ public class ColumnReference extends Val if (rsn instanceof FromTable) { FromTable ft = (FromTable)rsn; - tableNumber = ft.getTableNumber(); - if (SanityManager.DEBUG) - { - SanityManager.ASSERT(tableNumber != -1, - "tableNumber not expected to be -1"); - } /* It's not enough to just set the table number. Depending * on the original query specified and on whether or not @@ -865,15 +863,52 @@ public class ColumnReference extends Val * we got here. In that case we also need to update the * columnNumber to point to the correct column in "ft". * See DERBY-2526 for details. + * See DERBY-3023 and DERBY-4679 for further improvement + * details. */ - ResultColumn ftRC = - ft.getResultColumns().getResultColumn(columnName); - if (SanityManager.DEBUG) - { - SanityManager.ASSERT(ftRC != null, - "Failed to find column '" + columnName + "' in the " + - "RCL for '" + ft.getTableName() + "'."); + ResultColumnList rcl = ft.getResultColumns(); + + ResultColumn ftRC = null; + + + // Need to save original (tn,cn) in case we have several + // flattenings so we can relocate the correct column many + // times. After the first flattening, the (tn,cn) pair points + // to the top RCL which is going away.. + if (tableNumberBeforeFlattening == -1) { + tableNumberBeforeFlattening = tableNumber; + columnNumberBeforeFlattening = columnNumber; + } + + // Covers references to a table not being flattened out, e.g. + // inside a join tree, which can have many columns in the rcl + // with the same name, so looking up via column name can give + // the wrong column. DERBY-4679. + ftRC = rcl.getResultColumn( + tableNumberBeforeFlattening, + columnNumberBeforeFlattening); + + if (ftRC == null) { + // The above lookup won't work for references to a base + // column, so fall back on column name, which is unique + // then. + ftRC = rcl.getResultColumn(columnName); + } + + if (SanityManager.DEBUG) { + SanityManager.ASSERT( + ftRC != null, + "Failed to find column '" + columnName + + "' in the " + "RCL for '" + ft.getTableName() + + "'."); + } + + tableNumber = ft.getTableNumber(); + + if (SanityManager.DEBUG) { + SanityManager.ASSERT(tableNumber != -1, + "tableNumber not expected to be -1"); } /* Use the virtual column id if the ResultColumn's expression Modified: db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/compile/ResultColumnList.java URL: http://svn.apache.org/viewvc/db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/compile/ResultColumnList.java?rev=952237&r1=952236&r2=952237&view=diff ============================================================================== --- db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/compile/ResultColumnList.java (original) +++ db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/compile/ResultColumnList.java Mon Jun 7 14:18:14 2010 @@ -310,6 +310,80 @@ public class ResultColumnList extends Qu return null; } + /** + * Return a result column, if any found, which contains in its + * expression/{VCN,CR} chain a result column with the given + * columnNumber from a FromTable with the given tableNumber. + *

+ * Used by the optimizer preprocess phase when it is flattening queries, + * which has to use the pair {table number, column number} to + * uniquely distinguish the column desired in situations where the same + * table may appear multiple times in the queries with separate correlation + * names, and/or column names from different tables may be the same (hence + * looking up by column name will not always work), cf DERBY-4679. + * + * @param tableNumber the table number to look for + * @param columnNumber the column number to look for + */ + public ResultColumn getResultColumn(int tableNumber, int columnNumber) + { + int size = size(); + + for (int index = 0; index < size; index++) + { + ResultColumn resultColumn = (ResultColumn)elementAt(index); + ResultColumn rc = resultColumn; + + while (rc != null) { + ValueNode exp = rc.getExpression(); + + if (exp instanceof VirtualColumnNode) { + VirtualColumnNode vcn = (VirtualColumnNode)exp; + ResultSetNode rsn = vcn.getSourceResultSet(); + + if (rsn instanceof FromTable) { + FromTable ft = (FromTable)rsn; + + if (ft.getTableNumber() == tableNumber && + rc.getColumnPosition() == columnNumber) { + + // Found matching (t,c) within this top resultColumn + resultColumn.setReferenced(); + return resultColumn; + + } else { + rc = vcn.getSourceColumn(); + } + } else { + rc = null; + } + } else if (exp instanceof ColumnReference) { + ColumnReference cr = (ColumnReference)exp; + + if (cr.getTableNumber() == tableNumber && + cr.getColumnNumber() == columnNumber) { + // Found matching (t,c) within this top resultColumn + resultColumn.setReferenced(); + return resultColumn; + } else { + rc = null; + } + } else { + if (SanityManager.DEBUG) { + SanityManager.ASSERT( + exp instanceof BaseColumnNode, + "expected BaseColumnNode, found: " + + exp.getClass()); + } + rc = null; + } + } + + } + return null; + } + + /** * Get a ResultColumn that matches the specified columnName and * mark the ResultColumn as being referenced. Modified: db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/compile/VirtualColumnNode.java URL: http://svn.apache.org/viewvc/db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/compile/VirtualColumnNode.java?rev=952237&r1=952236&r2=952237&view=diff ============================================================================== --- db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/compile/VirtualColumnNode.java (original) +++ db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/compile/VirtualColumnNode.java Mon Jun 7 14:18:14 2010 @@ -94,6 +94,8 @@ public class VirtualColumnNode extends V printLabel(depth, "sourceColumn: "); sourceColumn.treePrint(depth + 1); + printLabel(depth, "sourceResultSet: "); + sourceResultSet.treePrint(depth + 1); } } Modified: db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/lang/JoinTest.java URL: http://svn.apache.org/viewvc/db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/lang/JoinTest.java?rev=952237&r1=952236&r2=952237&view=diff ============================================================================== --- db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/lang/JoinTest.java (original) +++ db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/lang/JoinTest.java Mon Jun 7 14:18:14 2010 @@ -1828,4 +1828,86 @@ public class JoinTest extends BaseJDBCTe ps.setString(3, c); ps.execute(); } + + + /** + * DERBY-4679. Verify that when transitive closure generates new criteria + * into the query, it isn't confused by situations where the same column + * name appears in a result column list multiple times due to flattening of + * sub-queries. + *

+ * Flattening requires remapping of (table, column) numbers in column + * references. In cases where the same column name appears in a result + * column list multiple times, this might earlier lead to remapping + * (reassigning) wrong (table, column) numbers to column references in join + * predicates transformed to where clauses as a result of the flattening. + *

+ * See also DERBY-2526 and DERBY-3023 whose fixes which were partial + * solutions to the problem of wrong column number remappings confusing + * the transitive closure of search predicates performed by the + * preprocessing step of the optimizer. + */ + public void testDerby_4679() throws SQLException { + setAutoCommit(false); + Statement s = createStatement(); + + s.execute("create table abstract_instance (" + + " jz_discriminator int, " + + " item_id char(32), " + + " family_item_id char(32), " + + " state_id char(32), " + + " visibility bigint)"); + + s.execute("create table lab_resource_operatingsystem (" + + " jz_parent_id char(32), " + + " item_id char(32))"); + + s.execute("create table operating_system_software_install (" + + " jz_parent_id char(32), " + + " item_id char(32))"); + + s.execute("create table family (" + + " item_id char(32), " + + " root_item_id char(32))"); + + s.execute("insert into abstract_instance (" + + " jz_discriminator, " + + " item_id, " + + " family_item_id, " + + " visibility) " + + "values (238, 'aaaa', 'bbbb', 0)," + + " (0, 'cccc', 'dddd', 0)," + + " (1, 'eeee', '_5VetVWTeEd-Q8aOqWJPEIQ', 0)"); + + s.execute("insert into lab_resource_operatingsystem " + + "values ('aaaa', 'cccc')"); + + + s.execute("insert into operating_system_software_install " + + "values ('cccc', 'eeee')"); + + s.execute("insert into family " + + "values ('dddd', '_5ZDlwWTeEd-Q8aOqWJPEIQ')," + + " ('bbbb', '_5nN9mmTeEd-Q8aOqWJPEIQ')"); + + ResultSet rs = + s.executeQuery( + "select distinct t1.ITEM_ID, t1.state_id, t1.JZ_DISCRIMINATOR from " + + "((((((select * from ABSTRACT_INSTANCE z1 where z1.JZ_DISCRIMINATOR = 238) t1 " + + " left outer join LAB_RESOURCE_OPERATINGSYSTEM j1 on (t1.ITEM_ID = j1.JZ_PARENT_ID)) " + + " left outer join ABSTRACT_INSTANCE t2 on (j1.ITEM_ID = t2.ITEM_ID)) " + + " left outer join OPERATING_SYSTEM_SOFTWARE_INSTALL j2 on (t2.ITEM_ID = j2.JZ_PARENT_ID))" + + " left outer join ABSTRACT_INSTANCE t3 on (j2.ITEM_ID = t3.ITEM_ID) " + + " inner join FAMILY t5 on (t2.FAMILY_ITEM_ID = t5.ITEM_ID)) " + + " inner join FAMILY t7 on (t1.FAMILY_ITEM_ID = t7.ITEM_ID)) " + + "where (t3.FAMILY_ITEM_ID IN('_5VetVWTeEd-Q8aOqWJPEIQ') and " + + " (t5.ROOT_ITEM_ID = '_5ZDlwWTeEd-Q8aOqWJPEIQ') and " + + " (t7.ROOT_ITEM_ID ='_5nN9mmTeEd-Q8aOqWJPEIQ') and " + + " (t1.VISIBILITY = 0))"); + JDBC.assertFullResultSet( + rs, + new String[][]{{"aaaa", null, "238"}}); + rollback(); + } + }