fixing incorrect index reuse in AlterTableAddConstraint.java…

fixing incorrect index reuse in AlterTableAddConstraint.java ALTER_TABLE_ADD_CONSTRAINT_REFERENTIAL and ALTER_TABLE_ADD_CONSTRAINT_UNIQUE

problem in ALTER_TABLE_ADD_CONSTRAINT_REFERENTIAL was caused by commit 08b98324 by Thomas Mueller <mueller@adobe.com>

he intended to reuse existing index... but it prevents that index from being dropped which can cause problems later
Correct solution would be to rewrite how automatically created indexes (referential/constraint) are managed to create new automatic index when dropping the reused existing index. Until this is solved it's IMHO better to not reuse existing index as it causes database to be effectively corrupted.

Also removing test code Thomas added to TestCases.java

problem in ALTER_TABLE_ADD_CONSTRAINT_UNIQUE was caused by canUseUniqueIndex allowing to use index with more columns than requested as unique index which caused unique constraint to fail as ConstraintUnique checkRow comment says that 'unique index check is enough' and the index is not unique for given constraint if it has more columns

h2/src/main/org/h2/command/ddl/AlterTableAddConstraint.java

@@ -212,7 +212,7 @@ public class AlterTableAddConstraint extends SchemaCommand {
                 isOwner = true;
                 index.getIndexType().setBelongsToConstraint(true);
             } else {
-                index = getIndex(table, indexColumns, true);
+                index = getIndex(table, indexColumns, false);
                 if (index == null) {
                     index = createIndex(table, indexColumns, false);
                     isOwner = true;
@@ -330,27 +330,28 @@ public class AlterTableAddConstraint extends SchemaCommand {
         return null;
     }
     
+
+    // all cols must be in the index key, the order doesn't matter and there must be no other fields in the index key
     private static boolean canUseUniqueIndex(Index idx, Table table,
         IndexColumn[] cols) {
         if (idx.getTable() != table || !idx.getIndexType().isUnique()) {
             return false;
         }
         Column[] indexCols = idx.getColumns();
-        if (indexCols.length > cols.length) {
-            return false;
-        }
-        HashSet<Column> set = New.hashSet();
-        for (IndexColumn c : cols) {
-            set.add(c.column);
-        }
+
+        HashSet<Column> indexColsSet = New.hashSet();
+
         for (Column c : indexCols) {
-            // all columns of the index must be part of the list,
-            // but not all columns of the list need to be part of the index
-            if (!set.contains(c)) {
-                return false;
+            indexColsSet.add(c);
         }
+
+        HashSet<Column> colsSet = New.hashSet();
+
+        for (IndexColumn c : cols) {
+            colsSet.add(c.column);
         }
-        return true;
+    
+      return colsSet.equals(indexColsSet);
     }        
 
     private static boolean canUseIndex(Index existingIndex, Table table,

h2/src/test/org/h2/test/db/TestCases.java

@@ -150,18 +150,6 @@ public class TestCases extends TestBase {
                 "foreign key(a_id) references a(id)");
         stat.execute("update a set x=200");
         stat.execute("drop table if exists a, b");
-
-        stat.execute("drop all objects");
-        stat.execute("create table parent(id int primary key)");
-        stat.execute("create table child(id int, parent_id int, x int)");
-        stat.execute("create index y on child(parent_id, x)");
-        stat.execute("alter table child add constraint z " +
-                "foreign key(parent_id) references parent(id)");
-        ResultSet rs = stat.executeQuery(
-                "select * from information_schema.indexes where table_name = 'CHILD'");
-        while (rs.next()) {
-            assertEquals("Y", rs.getString("index_name"));
-        }
         conn.close();
     }
 

h2/src/test/org/h2/test/scripts/TestScript.java

@@ -77,7 +77,9 @@ public class TestScript extends TestBase {
             return;
         }
         reconnectOften = !config.memory && config.big;
+        
         testScript("testScript.sql");
+        testScript("altertable-index-reuse.sql");
         testScript("query-optimisations.sql");
         testScript("commands-dml-script.sql");
         testScript("commands-dml-create-view.sql");

h2/src/test/org/h2/test/scripts/altertable-index-reuse.sql

+-- Copyright 2004-2014 H2 Group. Multiple-Licensed under the MPL 2.0,
+-- and the EPL 1.0 (http://h2database.com/html/license.html).
+-- Initial Developer: H2 Group
+--
+
+CREATE TABLE "domains" ("id" bigint NOT NULL auto_increment PRIMARY KEY);
+> ok
+
+CREATE TABLE "users" ("id" bigint NOT NULL auto_increment PRIMARY KEY,"username" varchar_ignorecase(255),"domain" bigint,"desc" varchar_ignorecase(255));
+> ok
+
+-- adds constraint on (domain,username) and generates unique index domainusername_key_INDEX_xxx
+ALTER TABLE "users" ADD CONSTRAINT "domainusername_key" UNIQUE ("domain","username");
+> ok
+
+-- adds foreign key on domain - if domainusername_key didn't exist it would create unique index on domain, but it reuses the existing index  
+ALTER TABLE "users"  ADD CONSTRAINT "udomain_fkey" FOREIGN KEY ("domain") REFERENCES "domains"("id") ON DELETE RESTRICT;
+> ok
+
+-- now we drop the domainusername_key, but domainusername_key_INDEX_xxx is used by udomain_fkey and was not being dropped 
+-- this was an issue, because it's a unique index and still enforcing constraint on (domain,username)
+ALTER TABLE "users" DROP CONSTRAINT "domainusername_key";
+> ok
+
+insert into "domains" ("id") VALUES (1);
+> update count: 1
+
+insert into "users" ("username","domain","desc") VALUES ('test',1,'first user');
+> update count: 1
+
+-- should work,because we dropped domainusername_key, but failed: Unique index or primary key violation
+INSERT INTO "users" ("username","domain","desc") VALUES ('test',1,'second user'); 
+> update count: 1