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/06/23 20:13:15 UTC
svn commit: r957287 - in /db/derby/code/branches/10.6: ./
java/engine/org/apache/derby/impl/sql/compile/
java/testing/org/apache/derbyTesting/functionTests/tests/lang/
Author: dag
Date: Wed Jun 23 18:13:14 2010
New Revision: 957287
URL: http://svn.apache.org/viewvc?rev=957287&view=rev
Log:
DERBY-4679 Several left outer joins causes unstable query with incorrect results
Back-ported two patches from trunk as
svn merge -c 952237 https://svn.apache.org/repos/asf/db/derby/code/trunk
svn merge -c 957260 https://svn.apache.org/repos/asf/db/derby/code/trunk
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.
---
Follow-up patch derby-4679-followup, which makes the original patch
safer by also matching the column name once a candidate result column
has been located using the table number and column number pair to
match an RC. This is to safe-guard against false matches, since
DERBY-4595 shows that the column number can be wrong in certain
situations.
Modified:
db/derby/code/branches/10.6/ (props changed)
db/derby/code/branches/10.6/java/engine/org/apache/derby/impl/sql/compile/ColumnReference.java
db/derby/code/branches/10.6/java/engine/org/apache/derby/impl/sql/compile/ResultColumnList.java
db/derby/code/branches/10.6/java/engine/org/apache/derby/impl/sql/compile/VirtualColumnNode.java
db/derby/code/branches/10.6/java/testing/org/apache/derbyTesting/functionTests/tests/lang/JoinTest.java
Propchange: db/derby/code/branches/10.6/
------------------------------------------------------------------------------
--- svn:mergeinfo (original)
+++ svn:mergeinfo Wed Jun 23 18:13:14 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,952138,954344,954421,954544,955001,956075,956234,956445,956659
+/db/derby/code/trunk:938547,938796,938959,939231,940462,940469,941627,942031,942286,942476,942480,942587,944152,946794,948045,948069,951346,952138,952237,954344,954421,954544,955001,956075,956234,956445,956659,957260
/db/derby/docs/trunk:954344
Modified: db/derby/code/branches/10.6/java/engine/org/apache/derby/impl/sql/compile/ColumnReference.java
URL: http://svn.apache.org/viewvc/db/derby/code/branches/10.6/java/engine/org/apache/derby/impl/sql/compile/ColumnReference.java?rev=957287&r1=957286&r2=957287&view=diff
==============================================================================
--- db/derby/code/branches/10.6/java/engine/org/apache/derby/impl/sql/compile/ColumnReference.java (original)
+++ db/derby/code/branches/10.6/java/engine/org/apache/derby/impl/sql/compile/ColumnReference.java Wed Jun 23 18:13: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,53 @@ 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,
+ columnName);
+
+ 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/branches/10.6/java/engine/org/apache/derby/impl/sql/compile/ResultColumnList.java
URL: http://svn.apache.org/viewvc/db/derby/code/branches/10.6/java/engine/org/apache/derby/impl/sql/compile/ResultColumnList.java?rev=957287&r1=957286&r2=957287&view=diff
==============================================================================
--- db/derby/code/branches/10.6/java/engine/org/apache/derby/impl/sql/compile/ResultColumnList.java (original)
+++ db/derby/code/branches/10.6/java/engine/org/apache/derby/impl/sql/compile/ResultColumnList.java Wed Jun 23 18:13:14 2010
@@ -310,6 +310,106 @@ 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.
+ * <p/>
+ * 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.
+ * <p/>
+ * {@code columnName} is used to assert that we find the right column.
+ * If we found a match on (tn, cn) but columnName is wrong, return null.
+ * Once we trust table numbers and column numbers to always be correct,
+ * cf. DERBY-4695, we can remove this parameter.
+ *
+ * @param tableNumber the table number to look for
+ * @param columnNumber the column number to look for
+ * @param columnName name of the desired column
+ */
+ public ResultColumn getResultColumn(int tableNumber,
+ int columnNumber,
+ String columnName)
+ {
+ 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. Now do sanity check that column
+ // name is correct. Remove when DERBY-4695 is
+ // fixed.
+ if (columnName.equals(
+ vcn.getSourceColumn().getName())) {
+ resultColumn.setReferenced();
+ return resultColumn;
+ } else {
+ if (SanityManager.DEBUG) {
+ SanityManager.ASSERT(
+ false,
+ "wrong (tn,cn) for column " +
+ columnName +
+ " found: this pair points to " +
+ vcn.getSourceColumn().getName());
+ }
+ // Fall back on column name based lookup,
+ // cf. DERBY-4679. See ColumnReference#
+ // remapColumnReferencesToExpressions
+ return null;
+ }
+ } 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/branches/10.6/java/engine/org/apache/derby/impl/sql/compile/VirtualColumnNode.java
URL: http://svn.apache.org/viewvc/db/derby/code/branches/10.6/java/engine/org/apache/derby/impl/sql/compile/VirtualColumnNode.java?rev=957287&r1=957286&r2=957287&view=diff
==============================================================================
--- db/derby/code/branches/10.6/java/engine/org/apache/derby/impl/sql/compile/VirtualColumnNode.java (original)
+++ db/derby/code/branches/10.6/java/engine/org/apache/derby/impl/sql/compile/VirtualColumnNode.java Wed Jun 23 18:13: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/branches/10.6/java/testing/org/apache/derbyTesting/functionTests/tests/lang/JoinTest.java
URL: http://svn.apache.org/viewvc/db/derby/code/branches/10.6/java/testing/org/apache/derbyTesting/functionTests/tests/lang/JoinTest.java?rev=957287&r1=957286&r2=957287&view=diff
==============================================================================
--- db/derby/code/branches/10.6/java/testing/org/apache/derbyTesting/functionTests/tests/lang/JoinTest.java (original)
+++ db/derby/code/branches/10.6/java/testing/org/apache/derbyTesting/functionTests/tests/lang/JoinTest.java Wed Jun 23 18:13: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.
+ * <p/>
+ * 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.
+ * <p/>
+ * 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();
+ }
+
}