gjacoby126 commented on a change in pull request #935: URL: https://github.com/apache/phoenix/pull/935#discussion_r525571557
########## File path: phoenix-core/src/it/java/org/apache/phoenix/end2end/AlterTableWithViewsIT.java ########## @@ -1216,5 +1216,251 @@ public void testDroppingIndexedColDropsViewIndex() throws Exception { assertNull(results.next()); } } - + + @Test + public void testAddThenDropColumnTableDDLTimestamp() throws Exception { + Properties props = new Properties(); + String schemaName = SCHEMA1; + String dataTableName = "T_" + generateUniqueName(); + String viewName = "V_" + generateUniqueName(); + String dataTableFullName = SchemaUtil.getTableName(schemaName, dataTableName); + String viewFullName = SchemaUtil.getTableName(schemaName, viewName); + + String tableDDL = generateDDL("CREATE TABLE IF NOT EXISTS " + dataTableFullName + " (" + + " %s ID char(1) NOT NULL," + + " COL1 integer NOT NULL," + + " COL2 bigint NOT NULL," + + " CONSTRAINT NAME_PK PRIMARY KEY (%s ID, COL1, COL2)" + + " ) %s"); + + String viewDDL = "CREATE VIEW " + viewFullName + " AS SELECT * FROM " + dataTableFullName; + + String columnAddDDL = "ALTER VIEW " + viewFullName + " ADD COL3 varchar(50) NULL "; + String columnDropDDL = "ALTER VIEW " + viewFullName + " DROP COLUMN COL3 "; + long startTS = EnvironmentEdgeManager.currentTimeMillis(); + try (Connection conn = DriverManager.getConnection(getUrl(), props)) { + conn.createStatement().execute(tableDDL); + //first get the original DDL timestamp when we created the table + long tableDDLTimestamp = CreateTableIT.verifyLastDDLTimestamp( + dataTableFullName, startTS, + conn); + Thread.sleep(1); + conn.createStatement().execute(viewDDL); + tableDDLTimestamp = CreateTableIT.verifyLastDDLTimestamp( + viewFullName, tableDDLTimestamp + 1, conn); + Thread.sleep(1); + //now add a column and make sure the timestamp updates + conn.createStatement().execute(columnAddDDL); + tableDDLTimestamp = CreateTableIT.verifyLastDDLTimestamp( + viewFullName, + tableDDLTimestamp + 1, conn); + Thread.sleep(1); + conn.createStatement().execute(columnDropDDL); + CreateTableIT.verifyLastDDLTimestamp( + viewFullName, + tableDDLTimestamp + 1 , conn); + } + } + + @Test + public void testLastDDLTimestampForDivergedViews() throws Exception { + //Phoenix allows users to "drop" columns from views that are inherited from their ancestor + // views or tables. These columns are then excluded from the view schema, and the view is + // considered "diverged" from its parents, and so no longer inherit any additional schema + // changes that are applied to their ancestors. This test make sure that this behavior Review comment: That doesn't seem like desirable behavior to me as a feature. As we discussed offline, I think diverged views should have been written to get all ancestor DDL changes. (To be precise, I think we should allow column projection in view definitions -- SELECT COL1, COL2 FROM FOO rather than SELECT * FROM FOO--, rather than having diverged views at all, but that's a whole new feature and out of scope for this discussion.) But setting all that aside, it's the _column drops_ that are the dangerous operations, because they can break existing queries, not _column adds_, which are always benign. (Don't care about a new column? Don't select it!) So if I were trying to allow for a view to split off from its parent and be semi-independent, it's the _drops_ I'd try to shield it from, not additions. ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org