Alter table is now supported even if a table has views defined.

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

@@ -7,6 +7,9 @@
 package org.h2.command.ddl;
 
 import java.sql.SQLException;
+import java.util.ArrayList;
+import java.util.List;
+
 import org.h2.command.Parser;
 import org.h2.command.Prepared;
 import org.h2.constant.ErrorCode;
@@ -28,6 +31,7 @@ import org.h2.schema.TriggerObject;
 import org.h2.table.Column;
 import org.h2.table.Table;
 import org.h2.table.TableData;
+import org.h2.table.TableView;
 import org.h2.util.ObjectArray;
 
 /**
@@ -139,7 +143,6 @@ public class AlterTableAlterColumn extends SchemaCommand {
             break;
         }
         case CHANGE_TYPE: {
-            checkNoViews();
             oldColumn.setSequence(null);
             oldColumn.setDefaultExpression(session, null);
             oldColumn.setConvertNullToDefault(false);
@@ -153,13 +156,11 @@ public class AlterTableAlterColumn extends SchemaCommand {
             break;
         }
         case ADD: {
-            checkNoViews();
             convertAutoIncrementColumn(newColumn);
             copyData();
             break;
         }
         case DROP: {
-            checkNoViews();
             if (table.getColumns().length == 1) {
                 throw Message.getSQLException(ErrorCode.CANNOT_DROP_LAST_COLUMN, oldColumn.getSQL());
             }
@@ -200,23 +201,54 @@ public class AlterTableAlterColumn extends SchemaCommand {
         }
     }
 
-    private void checkNoViews() throws SQLException {
-        for (DbObject child : table.getChildren()) {
-            if (child.getType() == DbObject.TABLE_OR_VIEW) {
-                throw Message.getSQLException(ErrorCode.OPERATION_NOT_SUPPORTED_WITH_VIEWS_2,
-                        table.getName(), child.getName());
-            }
-        }
-    }
-
     private void copyData() throws SQLException {
         if (table.isTemporary()) {
             throw Message.getUnsupportedException("TEMP TABLE");
         }
         Database db = session.getDatabase();
-        String tempName = db.getTempTableName(session.getId());
+        String tempName = db.getTempTableName(session);
         Column[] columns = table.getColumns();
         ObjectArray<Column> newColumns = ObjectArray.newInstance();
+        TableData newTable = cloneTableStructure(columns, db, tempName, newColumns);
+        List<String> views;
+        try {
+            views = checkViews(table, newTable);
+        } catch (SQLException e) {
+            execute("DROP TABLE " + newTable.getName(), true);
+            throw Message.getSQLException(ErrorCode.VIEW_IS_INVALID_2, e, getSQL(), e.getMessage());
+        }
+        String tableName = table.getName();
+        execute("DROP TABLE " + table.getSQL(), true);
+        db.renameSchemaObject(session, newTable, tableName);
+        for (DbObject child : newTable.getChildren()) {
+            if (child instanceof Sequence) {
+                continue;
+            }
+            String name = child.getName();
+            if (name == null || child.getCreateSQL() == null) {
+                continue;
+            }
+            if (name.startsWith(tempName + "_")) {
+                name = name.substring(tempName.length() + 1);
+                SchemaObject so = (SchemaObject) child;
+                if (so instanceof Constraint) {
+                    if (so.getSchema().findConstraint(session, name) != null) {
+                        name = so.getSchema().getUniqueConstraintName(session, newTable);
+                    }
+                } else if (so instanceof Index) {
+                    if (so.getSchema().findIndex(session, name) != null) {
+                        name = so.getSchema().getUniqueIndexName(session, newTable, name);
+                    }
+                }
+                db.renameSchemaObject(session, so, name);
+            }
+        }
+        for (String view : views) {
+            execute(view, true);
+        }
+    }
+
+    private TableData cloneTableStructure(Column[] columns, Database db, String tempName, ObjectArray<Column> newColumns) throws SQLException {
         for (Column col : columns) {
             newColumns.add(col.getClone());
         }
@@ -270,7 +302,7 @@ public class AlterTableAlterColumn extends SchemaCommand {
         }
         buff.append(" AS SELECT ");
         if (columnList.length() == 0) {
-            // special case insert into test select * from test
+            // special case: insert into test select * from
             buff.append('*');
         } else {
             buff.append(columnList);
@@ -297,7 +329,9 @@ public class AlterTableAlterColumn extends SchemaCommand {
             if (createSQL == null) {
                 continue;
             }
-            if (child.getType() == DbObject.TABLE_OR_VIEW) {
+            if (child instanceof TableView) {
+                continue;
+            } else if (child.getType() == DbObject.TABLE_OR_VIEW) {
                 Message.throwInternalError();
             }
             String quotedName = Parser.quoteIdentifier(tempName + "_" + child.getName());
@@ -319,7 +353,6 @@ public class AlterTableAlterColumn extends SchemaCommand {
                 }
             }
         }
-        String tableName = table.getName();
         table.setModified();
         // remove the sequences from the columns (except dropped columns)
         // otherwise the sequence is dropped if the table is dropped
@@ -333,29 +366,54 @@ public class AlterTableAlterColumn extends SchemaCommand {
         for (String sql : triggers) {
             execute(sql, true);
         }
-        execute("DROP TABLE " + table.getSQL(), true);
-        db.renameSchemaObject(session, newTable, tableName);
-        for (DbObject child : newTable.getChildren()) {
-            if (child instanceof Sequence) {
-                continue;
-            }
-            String name = child.getName();
-            if (name == null || child.getCreateSQL() == null) {
-                continue;
+        return newTable;
+    }
+
+    /**
+     * Check that all views are still valid.
+     *
+     * @param the list of SQL statements to re-create views that depend on this table
+     */
+    private List<String> checkViews(SchemaObject sourceTable, SchemaObject newTable) throws SQLException {
+        List<String> viewSql = new ArrayList<String>();
+        String sourceTableName = sourceTable.getName();
+        String newTableName = newTable.getName();
+        Database db = sourceTable.getDatabase();
+        // save the real table under a temporary name
+        db.renameSchemaObject(session, sourceTable, db.getTempTableName(session));
+        try {
+            // have our new table impersonate the target table
+            db.renameSchemaObject(session, newTable, sourceTableName);
+            checkViewsAreValid(sourceTable, viewSql);
+        } finally {
+            // always put the source tables back with their proper names
+            try {
+                db.renameSchemaObject(session, newTable, newTableName);
+            } finally {
+                db.renameSchemaObject(session, sourceTable, sourceTableName);
             }
-            if (name.startsWith(tempName + "_")) {
-                name = name.substring(tempName.length() + 1);
-                SchemaObject so = (SchemaObject) child;
-                if (so instanceof Constraint) {
-                    if (so.getSchema().findConstraint(session, name) != null) {
-                        name = so.getSchema().getUniqueConstraintName(session, newTable);
-                    }
-                } else if (so instanceof Index) {
-                    if (so.getSchema().findIndex(session, name) != null) {
-                        name = so.getSchema().getUniqueIndexName(session, newTable, name);
-                    }
-                }
-                db.renameSchemaObject(session, so, name);
+        }
+        return viewSql;
+    }
+
+    /**
+     * Check that a table or view is still valid.
+     *
+     * @param tableOrView the table or view to check
+     * @param recreate the list of SQL statements to re-create views that depend
+     *            on this table
+     */
+    private void checkViewsAreValid(DbObject tableOrView, List<String> recreate) throws SQLException {
+        for (DbObject view : tableOrView.getChildren()) {
+            if (view instanceof TableView) {
+                String sql = ((TableView) view).getQuery();
+                // check if the query is still valid
+                // do not execute, not even with limit 1, because that could
+                // have side effects or take a very long time
+                session.prepare(sql);
+                recreate.add(view.getDropSQL());
+                recreate.add(view.getCreateSQL());
+                checkViewsAreValid(view, recreate);
             }
         }
     }

h2/src/test/org/h2/test/TestAll.java

@@ -59,6 +59,7 @@ import org.h2.test.db.TestTransaction;
 import org.h2.test.db.TestTriggersConstraints;
 import org.h2.test.db.TestTwoPhaseCommit;
 import org.h2.test.db.TestView;
+import org.h2.test.db.TestViewAlterTable;
 import org.h2.test.jaqu.AliasMapTest;
 import org.h2.test.jaqu.SamplesTest;
 import org.h2.test.jaqu.UpdateTest;
@@ -298,7 +299,7 @@ java org.h2.test.TestAll timer
 
 /*
 
-AlterTableAlterColumn todo
+update copyright to 2010
 
 outer join bug
 
@@ -516,6 +517,7 @@ kill -9 `jps -l | grep "org.h2.test." | cut -d " " -f 1`
         new TestTriggersConstraints().runTest(this);
         new TestTwoPhaseCommit().runTest(this);
         new TestView().runTest(this);
+        new TestViewAlterTable().runTest(this);
 
         // jaqu
         new AliasMapTest().runTest(this);

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

+/*
+ * Copyright 2004-2009 H2 Group. Multiple-Licensed under the H2 License,
+ * Version 1.0, and under the Eclipse Public License, Version 1.0
+ * (http://h2database.com/html/license.html).
+ * Initial Developer: H2 Group
+ */
+package org.h2.test.db;
+
+import java.sql.Connection;
+import java.sql.ResultSet;
+import java.sql.SQLException;
+import java.sql.Statement;
+import org.h2.test.TestBase;
+import org.h2.constant.ErrorCode;
+
+/**
+ * Test the impact of ALTER TABLE statements on views.
+ */
+public class TestViewAlterTable extends TestBase {
+
+    private Connection conn;
+    private Statement stat;
+
+    /**
+     * Run just this test.
+     *
+     * @param a ignored
+     */
+    public static void main(String... a) throws Exception {
+        TestBase.createCaller().init().test();
+    }
+
+    public void test() throws Exception {
+        deleteDb("alter");
+        conn = getConnection("alter");
+        stat = conn.createStatement();
+
+        testDropColumnWithoutViews();
+        testViewsAreWorking();
+        testAlterTableDropColumnNotInView();
+        testAlterTableDropColumnInView();
+        testAlterTableAddColumnWithView();
+        testAlterTableAlterColumnDataTypeWithView();
+        testSelectStar();
+        testJoinAndAlias();
+        testSubSelect();
+        testForeignKey();
+
+        conn.close();
+        deleteDb("alter");
+    }
+
+    private void testDropColumnWithoutViews() throws SQLException {
+        stat.execute("create table test(a int, b int, c int)");
+        stat.execute("alter table test drop column c");
+        stat.execute("drop table test");
+    }
+
+    private void testViewsAreWorking() throws SQLException {
+        createTestData();
+        checkViewRemainsValid();
+    }
+
+    private void testAlterTableDropColumnNotInView() throws SQLException {
+        createTestData();
+        stat.execute("alter table test drop column c");
+        checkViewRemainsValid();
+    }
+
+    private void testAlterTableDropColumnInView() throws SQLException {
+        // simple
+        stat.execute("create table test(id identity, name varchar) as select x, 'Hello'");
+        stat.execute("create view test_view as select * from test");
+        try {
+            stat.execute("alter table test drop name");
+            fail();
+        } catch (SQLException e) {
+            assertEquals(ErrorCode.VIEW_IS_INVALID_2, e.getErrorCode());
+        }
+        ResultSet rs = stat.executeQuery("select * from test_view");
+        assertTrue(rs.next());
+        stat.execute("drop view test_view");
+        stat.execute("drop table test");
+
+        // nested
+        createTestData();
+        try {
+            stat.execute("alter table test drop column a");
+            fail("Should throw exception because V1 uses column A");
+        } catch (SQLException e) {
+            assertEquals(ErrorCode.VIEW_IS_INVALID_2, e.getErrorCode());
+        }
+        stat.execute("drop table test");
+    }
+
+    private void testAlterTableAddColumnWithView() throws SQLException {
+        createTestData();
+        stat.execute("alter table test add column d int");
+        checkViewRemainsValid();
+    }
+
+    private void testAlterTableAlterColumnDataTypeWithView() throws SQLException {
+        createTestData();
+        stat.execute("alter table test alter b char(1)");
+        checkViewRemainsValid();
+    }
+
+    private void testSelectStar() throws SQLException {
+        createTestData();
+        stat.execute("create view v4 as select * from test");
+        stat.execute("alter table test add d int default 6");
+        try {
+            stat.executeQuery("select d from v4");
+            // H2 doesn't remember v4 as 'select * from test',
+            // it instead remembers each individual column that was in 'test' when the
+            // view was originally created. This is consistent with PostgreSQL.
+        } catch (SQLException e) {
+            assertEquals(ErrorCode.COLUMN_NOT_FOUND_1, e.getErrorCode());
+        }
+        checkViewRemainsValid();
+    }
+
+    private void testJoinAndAlias() throws SQLException {
+        createTestData();
+        stat.execute("create view v4 as select v1.a dog, v3.a cat from v1 join v3 on v1.b = v3.a");
+        // should make no difference
+        stat.execute("alter table test add d int default 6");
+        ResultSet rs = stat.executeQuery("select cat, dog from v4");
+        assertTrue(rs.next());
+        assertEquals(1, rs.getInt(1));
+        assertEquals(2, rs.getInt(2));
+        assertFalse(rs.next());
+        checkViewRemainsValid();
+    }
+
+    private void testSubSelect() throws SQLException {
+        createTestData();
+        stat.execute("create view v4 as select * from v3 where a in (select b from v2)");
+        // should make no difference
+        stat.execute("alter table test add d int default 6");
+        ResultSet rs = stat.executeQuery("select a from v4");
+        assertTrue(rs.next());
+        assertEquals(1, rs.getInt(1));
+        assertFalse(rs.next());
+        checkViewRemainsValid();
+    }
+
+    private void testForeignKey() throws SQLException {
+        createTestData();
+        stat.execute("create table test2(z int, a int, primary key(z), foreign key (a) references TEST(a))");
+        stat.execute("insert into test2(z, a) values (99, 1)");
+        // should make no difference
+        stat.execute("alter table test add d int default 6");
+        ResultSet rs = stat.executeQuery("select z from test2");
+        assertTrue(rs.next());
+        assertEquals(99, rs.getInt(1));
+        assertFalse(rs.next());
+        stat.execute("drop table test2");
+        checkViewRemainsValid();
+    }
+
+    private void createTestData() throws SQLException {
+        stat.execute("create table test(a int, b int, c int)");
+        stat.execute("insert into test(a, b, c) values (1, 2, 3)");
+        stat.execute("create view v1 as select a as b, b as a from test");
+        // child of v1
+        stat.execute("create view v2 as select * from v1");
+        // sibling of v1
+        stat.execute("create view v3 as select a from test");
+    }
+
+    private void checkViewRemainsValid() throws SQLException {
+        ResultSet rs = stat.executeQuery("select b from v1");
+        assertTrue(rs.next());
+        assertEquals(1, rs.getInt(1));
+        assertFalse(rs.next());
+
+        rs = stat.executeQuery("select b from v2");
+        assertTrue(rs.next());
+        assertEquals(1, rs.getInt(1));
+        assertFalse(rs.next());
+
+        rs = stat.executeQuery("select b from test");
+        assertTrue(rs.next());
+        assertEquals(2, rs.getInt(1));
+        assertFalse(rs.next());
+
+        stat.execute("drop table test");
+
+        ResultSet d = conn.getMetaData().getTables(null, null, null, null);
+        while (d.next()) {
+            if (!d.getString(2).equals("INFORMATION_SCHEMA")) {
+                fail("Should have no tables left in the database, not: " + d.getString(2) + "." + d.getString(3));
+            }
+        }
+    }
+}