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 ba...@apache.org on 2006/06/03 09:02:32 UTC

svn commit: r411393 - in /db/derby/code/trunk/java: engine/org/apache/derby/impl/sql/compile/CurrentOfNode.java testing/org/apache/derbyTesting/functionTests/master/update.out testing/org/apache/derbyTesting/functionTests/tests/lang/update.sql

Author: bandaram
Date: Sat Jun  3 00:02:32 2006
New Revision: 411393

URL: http://svn.apache.org/viewvc?rev=411393&view=rev
Log:
DERBY-1329: Set ColumnReference in CurrentOfNode when a match is found.

Attaching a patch to address this issue. In a word, the problem is that the ColumnReference in a CurrentOfNode can, in certain situations, end up with a tableNumber that is never set, and hence it defaults to -1. The fix I've made ensures that the ColumnReference's tableNumber will always be set when necessary--i.e. when we've found the ResultColumn that matches the received ColumnReference. I think this is the correct fix for two reasons:

1) In FromList.bindColumnReferences(), there is the following comment:

  /* TableNumbers are set in the CR in the underlying
   * FromTable. This ensures that they get the table
   * number from the underlying table, not the join node.
   * This is important for beging able to push predicates
   * down through join nodes.
   */

The place where "TableNumbers are set" is in the getMatchingColumn() call, which means that the underlying FromTable (which includes CurrentOfNode) is responsible for setting the table number.

2) Inspection of all other FromTables that implement getMatchingColumn() shows that they all set the ColumnReference's table number if the corresponding ResultColumn is found. The one exception is JoinNode, but the getMatchingColumn() method in JoinNode in turn calls the method of the same name on the join's left and right nodes, so we know that, eventually, the ColumnReference's tableNumber will get set by one of the other FromTable's getMatchingColumn() calls. So the only FromTable that does not set the tableNumber is CurrentOfNode, and that's the reason for the failure described in this issue.

The change seems fairly minor but if anyone has a chance to double-check it, that'd be great. I also added a test case (using the repro posted in the above comments) to lang/update.sql.

I ran derbyall on Linux Red Hat (RHEL4) using ibm142 and saw no new failures.

Submitted by Army Brown (gozinx@gmail.com)

Modified:
    db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/compile/CurrentOfNode.java
    db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/master/update.out
    db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/lang/update.sql

Modified: db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/compile/CurrentOfNode.java
URL: http://svn.apache.org/viewvc/db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/compile/CurrentOfNode.java?rev=411393&r1=411392&r2=411393&view=diff
==============================================================================
--- db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/compile/CurrentOfNode.java (original)
+++ db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/compile/CurrentOfNode.java Sat Jun  3 00:02:32 2006
@@ -347,6 +347,16 @@
 
 			if (resultColumn != null) 
 			{
+				// If we found the ResultColumn, set the ColumnReference's
+				// table number accordingly.  Note: we used to only set
+				// the tableNumber for correlated references (as part of
+				// changes for DERBY-171) but inspection of code (esp.
+				// the comments in FromList.bindColumnReferences() and
+				// the getMatchingColumn() methods on other FromTables)
+				// suggests that we should always set the table number
+				// if we've found the ResultColumn.  So we do that here.
+				columnReference.setTableNumber( tableNumber );
+
 				// If there is a result column, are we really updating it?
 				// If so, verify that the column is updatable as well
 				notfound = 
@@ -368,15 +378,6 @@
 			}
 		}
 
-		/*
-		 * Patch up the table number for correlated references.
-		 * Part of the fix for bug 171.
-		 */
-		if ( (correlationName != null) && (columnReference.getTableNumber() < 0) )
-		{
-			columnReference.setTableNumber( tableNumber );
-		}
-		
 		return resultColumn;
 	}
 

Modified: db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/master/update.out
URL: http://svn.apache.org/viewvc/db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/master/update.out?rev=411393&r1=411392&r2=411393&view=diff
==============================================================================
--- db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/master/update.out (original)
+++ db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/master/update.out Sat Jun  3 00:02:32 2006
@@ -518,4 +518,51 @@
 0 rows inserted/updated/deleted
 ij> drop table bug171_bonuses;
 0 rows inserted/updated/deleted
+ij> --
+-- DERBY-1329: Correlated subquery in UPDATE ... SET ... WHERE CURRENT OF
+--
+CREATE TABLE BASICTABLE1(ID INTEGER, C3 CHAR(10));
+0 rows inserted/updated/deleted
+ij> CREATE TABLE BASICTABLE2(IID INTEGER, CC3 CHAR(10));
+0 rows inserted/updated/deleted
+ij> insert into BASICTABLE1 (C3, ID) values ('abc', 1);
+1 row inserted/updated/deleted
+ij> insert into BASICTABLE2 (CC3, IID) values ('def', 1);
+1 row inserted/updated/deleted
+ij> -- Check data.
+select * from BASICTABLE1;
+ID         |C3        
+----------------------
+1          |abc       
+ij> select * from BASICTABLE2;
+IID        |CC3       
+----------------------
+1          |def       
+ij> autocommit off;
+ij> get cursor c1 as 'select c3, id from basictable1 for update';
+ij> next c1;
+C3        |ID         
+----------------------
+abc       |1          
+ij> -- Before fix for DERBY-1329 the following statement would fail with
+-- an ASSERT failure or an IndexOutOfBoundsException; after the fix
+-- the statement should succeed and the update as well.
+update BASICTABLE1 set C3 = (SELECT CC3 FROM BASICTABLE2
+  WHERE BASICTABLE1.ID=BASICTABLE2.IID) where current of c1;
+1 row inserted/updated/deleted
+ij> -- Check data; BASICTABLE1 should have been updated.
+select * from BASICTABLE1;
+ID         |C3        
+----------------------
+1          |def       
+ij> select * from BASICTABLE2;
+IID        |CC3       
+----------------------
+1          |def       
+ij> -- Cleanup.
+rollback;
+ij> drop table BASICTABLE1;
+0 rows inserted/updated/deleted
+ij> drop table BASICTABLE2;
+0 rows inserted/updated/deleted
 ij> 

Modified: db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/lang/update.sql
URL: http://svn.apache.org/viewvc/db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/lang/update.sql?rev=411393&r1=411392&r2=411393&view=diff
==============================================================================
--- db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/lang/update.sql (original)
+++ db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/lang/update.sql Sat Jun  3 00:02:32 2006
@@ -310,3 +310,34 @@
 
 drop table bug171_employee;
 drop table bug171_bonuses;
+
+--
+-- DERBY-1329: Correlated subquery in UPDATE ... SET ... WHERE CURRENT OF
+--
+CREATE TABLE BASICTABLE1(ID INTEGER, C3 CHAR(10));
+CREATE TABLE BASICTABLE2(IID INTEGER, CC3 CHAR(10));
+insert into BASICTABLE1 (C3, ID) values ('abc', 1);
+insert into BASICTABLE2 (CC3, IID) values ('def', 1);
+
+-- Check data.
+select * from BASICTABLE1;
+select * from BASICTABLE2;
+
+autocommit off;
+get cursor c1 as 'select c3, id from basictable1 for update';
+next c1;
+
+-- Before fix for DERBY-1329 the following statement would fail with
+-- an ASSERT failure or an IndexOutOfBoundsException; after the fix
+-- the statement should succeed and the update as well.
+update BASICTABLE1 set C3 = (SELECT CC3 FROM BASICTABLE2
+  WHERE BASICTABLE1.ID=BASICTABLE2.IID) where current of c1;
+
+-- Check data; BASICTABLE1 should have been updated.
+select * from BASICTABLE1;
+select * from BASICTABLE2;
+
+-- Cleanup.
+rollback;
+drop table BASICTABLE1;
+drop table BASICTABLE2;