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 bp...@apache.org on 2016/06/18 16:00:21 UTC

svn commit: r1749069 - in /db/derby/code/trunk/java: engine/org/apache/derby/impl/sql/execute/ testing/org/apache/derbyTesting/functionTests/tests/lang/

Author: bpendleton
Date: Sat Jun 18 16:00:20 2016
New Revision: 1749069

URL: http://svn.apache.org/viewvc?rev=1749069&view=rev
Log:
DERBY-6890: ALTER TABLE DROP COLUMN corrupts secondary index collation data.

During ALTER TABLE DROP COLUMN, we rebuild all the indexes on the table. Some
indexes may be entirely dropped, some indexes may be rebuilt with a subset
of columns, some indexes are simply rebuild with essentially the same content.

The issue is that the index rebuild logic was incorrectly setting the
collation data for each index. So the indexes had the right data, but the
wrong collation information, causing them to be damaged.

This change moves the computation of the index collation ids out of the
setUpAllSorts method, into the getAffectedIndexes method, where it is simpler
to compute the index collation ids appropriately, because the code in that
location already has logic to manipulate both the old (pre-DROP) and new
(post-DROP) table descriptions, and so it is straightforward to compute the
correct collation ids there.

As part of this change, an old test case in CollationTest2, which was marked
with the comment "this test does not work yet", and was disabled, is changed
(un-commented) and is now enabled.

I found no new problems with this test case. I believe that, at the time
that comment was written, there were bugs in Derby that have since been
repaired, and hence it is appropriate to enable this test case at this time.


Modified:
    db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/execute/AlterTableConstantAction.java
    db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/lang/CollationTest.java
    db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/lang/CollationTest2.java

Modified: db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/execute/AlterTableConstantAction.java
URL: http://svn.apache.org/viewvc/db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/execute/AlterTableConstantAction.java?rev=1749069&r1=1749068&r2=1749069&view=diff
==============================================================================
--- db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/execute/AlterTableConstantAction.java (original)
+++ db/derby/code/trunk/java/engine/org/apache/derby/impl/sql/execute/AlterTableConstantAction.java Sat Jun 18 16:00:20 2016
@@ -2922,12 +2922,18 @@ class AlterTableConstantAction extends D
 
 			IndexRowGenerator[] newIRGs = new IndexRowGenerator[numIndexes];
 			long[] newIndexConglomNumbers = new long[numIndexes];
+			collation = new int[numIndexes][]; 
 
 			for (int i = 0, j = 0; i < numIndexes; i++, j++)
 			{
 				while (compressIRGs[j] == null)
 					j++;
 
+				// Setup collation id array to be passed in on call to create index.
+				collation[i] = 
+					compressIRGs[j].getColumnCollationIds(
+						td.getColumnDescriptorList());
+
 				int[] baseColumnPositions = compressIRGs[j].baseColumnPositions();
 				newIRGs[i] = compressIRGs[j];
 				newIndexConglomNumbers[i] = indexConglomerateNumbers[j];
@@ -2960,22 +2966,35 @@ class AlterTableConstantAction extends D
 					size--;
 					int[] newBCP = new int[size];
 					boolean[] newIsAscending = new boolean[size];
+					int[] newCollation = new int[collation[i].length - 1];
 					for (int k = 0, step = 0; k < size; k++)
 					{
 						if (step == 0 && baseColumnPositions[k + step] == 0)
 							step++;
 						newBCP[k] = baseColumnPositions[k + step];
 						newIsAscending[k] = isAscending[k + step];
+						newCollation[k] = collation[i][k + step];
 					}
 					IndexDescriptor id = compressIRGs[j].getIndexDescriptor();
 					id.setBaseColumnPositions(newBCP);
 					id.setIsAscending(newIsAscending);
 					id.setNumberOfOrderedColumns(id.numberOfOrderedColumns() - 1);
+					collation[i] = newCollation;
 				}
 			}
 			compressIRGs = newIRGs;
 			indexConglomerateNumbers = newIndexConglomNumbers;
 		}
+		else
+		{
+			collation = new int[numIndexes][]; 
+			for (int i = 0; i < numIndexes; i++)
+			{
+				collation[i] = 
+					compressIRGs[i].getColumnCollationIds(
+						td.getColumnDescriptorList());
+			}
+		}
 
 		/* Now we are done with updating each index descriptor entry directly
 		 * in SYSCONGLOMERATES (for duplicate index as well), from now on, our
@@ -3103,7 +3122,6 @@ class AlterTableConstantAction extends D
 		throws StandardException
     {
 		ordering        = new ColumnOrdering[numIndexes][];
-        collation       = new int[numIndexes][]; 
 		needToDropSort  = new boolean[numIndexes];
 		sortIds         = new long[numIndexes];
 
@@ -3120,11 +3138,6 @@ class AlterTableConstantAction extends D
 			compressIRGs[index].getIndexRow(
                 sourceRow, rl, indexRows[index], (FormatableBitSet) null);
 
-            // Setup collation id array to be passed in on call to create index.
-            collation[index] = 
-                compressIRGs[index].getColumnCollationIds(
-                    td.getColumnDescriptorList());
-
 			/* For non-unique indexes, we order by all columns + the RID.
 			 * For unique indexes, we just order by the columns.
 			 * No need to try to enforce uniqueness here as

Modified: db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/lang/CollationTest.java
URL: http://svn.apache.org/viewvc/db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/lang/CollationTest.java?rev=1749069&r1=1749068&r2=1749069&view=diff
==============================================================================
--- db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/lang/CollationTest.java (original)
+++ db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/lang/CollationTest.java Sat Jun 18 16:00:20 2016
@@ -73,6 +73,7 @@ public class CollationTest extends BaseJ
         "testUsingClauseAndNaturalJoin",
         "testNullColumnInInsert",
         "testDerby6227",
+        "testDerby6890",
     };
 
     /** Test cases to run with Norwegian case-sensitive collation. */
@@ -2287,4 +2288,42 @@ public void testMissingCollatorSupport()
         Statement s = createStatement();
         JDBC.assertFullResultSet(s.executeQuery(sql), expectedRows);
     }
+    public void testDerby6890()
+                throws SQLException
+    {
+        Statement s = createStatement();
+        s.execute( "CREATE TABLE Module (id BIGINT NOT NULL," +
+                       " title VARCHAR(200)," +
+                       " CONSTRAINT PK_MODULE PRIMARY KEY (id))" );
+        s.execute( "CREATE INDEX Module_title ON Module(title)" );
+        s.execute( "ALTER TABLE MODULE ADD COLUMN ID_TEMP BIGINT" +
+                       " GENERATED BY DEFAULT AS IDENTITY" );
+        s.execute( "UPDATE MODULE SET ID_TEMP = ID" );
+        s.execute( "ALTER TABLE MODULE ALTER COLUMN ID_TEMP NOT NULL" );
+        s.execute( "ALTER TABLE MODULE DROP ID" );
+        s.execute( "RENAME COLUMN MODULE.ID_TEMP TO ID" );
+        s.execute( "ALTER TABLE MODULE ADD CONSTRAINT PK_MODULE" +
+                       " PRIMARY KEY (ID)" );
+
+//          !!!!!!!!!!! WORKAROUND !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
+//            'DROP INDEX Module_title;\n' +
+//            'CREATE INDEX Module_title ON Module(title);\n' +
+
+        PreparedStatement pSt =
+                prepareStatement("insert into Module(title) values(?)");
+
+        int i = 0;
+        setAutoCommit(false);
+        while (i < 295) {
+            pSt.setString(1, "1234567890123456789012345678901234567890" +
+                             "1234567890123456789012345678901234567890" +
+                             "1234567890123456789012345678901234567890" +
+                             "1234567890123456789012345678901234567890" +
+                             "1234567890123456789012345678901234567890" );
+            pSt.executeUpdate();
+            i++;
+        }
+        commit();
+    }
+
 }

Modified: db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/lang/CollationTest2.java
URL: http://svn.apache.org/viewvc/db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/lang/CollationTest2.java?rev=1749069&r1=1749068&r2=1749069&view=diff
==============================================================================
--- db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/lang/CollationTest2.java (original)
+++ db/derby/code/trunk/java/testing/org/apache/derbyTesting/functionTests/tests/lang/CollationTest2.java Sat Jun 18 16:00:20 2016
@@ -736,6 +736,42 @@ public class CollationTest2 extends Base
         commit();
     }
 
+    private void addSomeMoreCustomers( int counter ) throws SQLException
+    { addSomeMoreCustomers( counter, true, true ); }
+
+    private void addSomeMoreCustomers( int counter, boolean useD1, boolean useD2 ) throws SQLException
+    {
+        PreparedStatement ps;
+	if( useD1 && useD2 )
+            ps = prepareStatement("INSERT INTO CUSTOMER VALUES(?,?,?,?,?,?,?)");
+	else if( useD2 )
+            ps = prepareStatement("INSERT INTO CUSTOMER VALUES(?,?,?,?,?,?)");
+	else
+            ps = prepareStatement("INSERT INTO CUSTOMER VALUES(?,?,?,?,?)");
+
+	int colNo = 1;
+        for (int i = 0; i < NAMES.length; i++)
+        {
+	    if( useD1 )
+                ps.setString(colNo++, "Another " + counter + NAMES[i]);
+	    if( useD2 )
+                ps.setString(colNo++, "Another " + counter + NAMES[i]);
+            ps.setString(colNo++, "Another " + counter + NAMES[i]);
+            ps.setInt( colNo++,   NAMES.length + counter + i);
+            ps.setInt( colNo++,   NAMES.length + counter + i);
+            ps.setString(colNo++, "Another " + counter + NAMES[i]);
+            ps.setString(colNo++, "Another " + counter + NAMES[i]);
+            ps.executeUpdate();
+	    colNo = 1;
+        }
+    }
+    private void dropExtraCustomers( int counter ) throws SQLException
+    {
+        PreparedStatement ps = prepareStatement("DELETE FROM CUSTOMER WHERE ID >= ?");
+        ps.setInt( 1, counter );
+        ps.executeUpdate();
+    }
+
     private void setUpLikeTable() throws SQLException 
     {
         Statement s = createStatement();
@@ -1432,6 +1468,8 @@ public class CollationTest2 extends Base
 
         runQueries(db_index, null, null);
 
+        addSomeMoreCustomers( 100 );
+
         dropTable();
         commit();
     }
@@ -1447,21 +1485,24 @@ public class CollationTest2 extends Base
      * T11: alter table drop column with indexes
      **/
     private void runAlterTableDropColumn(
-    Connection  conn,
     int         db_index)
         throws SQLException 
     {
-        Statement s = conn.createStatement();
+        Statement s = createStatement();
 
         setUpTable();
 
         s.execute("ALTER TABLE CUSTOMER DROP COLUMN D1");
         runQueries(db_index, null, null);
+        addSomeMoreCustomers( 100, false, true );
+	dropExtraCustomers( 100 );
 
         s.execute("CREATE INDEX IDX1 ON CUSTOMER (NAME)");
         s.execute("ALTER TABLE CUSTOMER DROP COLUMN D2");
         runQueries(db_index, null, null);
-        conn.rollback();
+        addSomeMoreCustomers( 100, false, false );
+
+        rollback();
 
         dropTable();
         commit();
@@ -1493,6 +1534,8 @@ public class CollationTest2 extends Base
         s.execute("CREATE INDEX IDX1 ON CUSTOMER (NAME)");
         runQueries(db_index, null, null);
 
+        addSomeMoreCustomers( 100 );
+
         dropTable();
 
         commit();
@@ -1924,12 +1967,11 @@ public class CollationTest2 extends Base
         runLikeTests(db_index);
 
         runDerby5530TruncateNoIndex();
+
         runDerby5530TruncateIndex();
 
-        /*
-        TODO -MIKEM, this test does not work yet.
-        runAlterTableDropColumn(conn, db_index);
-        */
+        dropTable();
+        runAlterTableDropColumn(db_index);
 
         commit();
     }