shahrs87 commented on a change in pull request #935: URL: https://github.com/apache/phoenix/pull/935#discussion_r525324954
########## File path: phoenix-core/src/it/java/org/apache/phoenix/end2end/AlterTableIT.java ########## @@ -1326,7 +1328,69 @@ public void testAddingColumnsToTablesAndViews() throws Exception { assertSequenceNumber(schemaName, viewName, PTable.INITIAL_SEQ_NUM + 1); } } - + + @Test + public void testAddThenDropColumnTableDDLTimestamp() throws Exception { + Properties props = new Properties(); + String tableDDL = "CREATE TABLE IF NOT EXISTS " + dataTableFullName + " (" + + " ENTITY_ID integer NOT NULL," + + " COL1 integer NOT NULL," + + " COL2 bigint NOT NULL," + + " CONSTRAINT NAME_PK PRIMARY KEY (ENTITY_ID, COL1, COL2)" + + " ) " + generateDDLOptions(""); + + String columnAddDDL = "ALTER TABLE " + dataTableFullName + " ADD COL3 varchar(50) NULL "; + String columnDropDDL = "ALTER TABLE " + dataTableFullName + " 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(schemaName, dataTableName, + dataTableFullName, startTS, + conn); + Thread.sleep(1); + //now add a column and make sure the timestamp updates + conn.createStatement().execute(columnAddDDL); + tableDDLTimestamp = CreateTableIT.verifyLastDDLTimestamp(schemaName, dataTableName, + dataTableFullName, + tableDDLTimestamp + 1, conn); + Thread.sleep(1); + conn.createStatement().execute(columnDropDDL); + CreateTableIT.verifyLastDDLTimestamp(schemaName, dataTableName, + dataTableFullName, + tableDDLTimestamp + 1 , conn); + } + } + + @Test + public void testSetPropertyDoesntUpdateDDLTimestamp() throws Exception { + Properties props = new Properties(); + String tableDDL = "CREATE TABLE IF NOT EXISTS " + dataTableFullName + " (" + + " ENTITY_ID integer NOT NULL," + + " COL1 integer NOT NULL," + + " COL2 bigint NOT NULL," + + " CONSTRAINT NAME_PK PRIMARY KEY (ENTITY_ID, COL1, COL2)" + + " ) " + generateDDLOptions(""); + + String setPropertyDDL = "ALTER TABLE " + dataTableFullName + + " SET UPDATE_CACHE_FREQUENCY=300000 "; + 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(schemaName, dataTableName, + dataTableFullName, startTS, + conn); + Thread.sleep(1); + //now add a column and make sure the timestamp updates Review comment: @gjacoby126 looks like this comment was missed ? ########## File path: phoenix-core/src/it/java/org/apache/phoenix/end2end/AlterTableWithViewsIT.java ########## @@ -1216,5 +1218,248 @@ 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(schemaName, dataTableName, + dataTableFullName, startTS, + conn); + Thread.sleep(1); + conn.createStatement().execute(viewDDL); + tableDDLTimestamp = CreateTableIT.verifyLastDDLTimestamp(schemaName, viewName, + viewFullName, tableDDLTimestamp + 1, conn); + Thread.sleep(1); + //now add a column and make sure the timestamp updates + conn.createStatement().execute(columnAddDDL); + tableDDLTimestamp = CreateTableIT.verifyLastDDLTimestamp(schemaName, viewName, + viewFullName, + tableDDLTimestamp + 1, conn); + Thread.sleep(1); + conn.createStatement().execute(columnDropDDL); + CreateTableIT.verifyLastDDLTimestamp(schemaName, viewName, + 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 + // extends to DDL timestamp + 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," + + " CONSTRAINT NAME_PK PRIMARY KEY (%s ID, COL1)" + + " ) %s"); + + String viewDDL = "CREATE VIEW " + viewFullName + " AS SELECT * FROM " + dataTableFullName; + + String divergeDDL = "ALTER VIEW " + viewFullName + " DROP COLUMN COL2"; + String viewColumnAddDDL = "ALTER VIEW " + viewFullName + " ADD COL3 varchar(50) NULL "; + String viewColumnDropDDL = "ALTER VIEW " + viewFullName + " DROP COLUMN COL3 "; + String tableColumnAddDDL = "ALTER TABLE " + dataTableFullName + " ADD COL4 varchar" + + "(50) NULL"; + String tableColumnDropDDL = "ALTER TABLE " + dataTableFullName + " DROP COLUMN COL4 "; + try (Connection conn = DriverManager.getConnection(getUrl())) { + conn.createStatement().execute(tableDDL); + conn.createStatement().execute(viewDDL); + long viewDDLTimestamp = getLastDDLTimestamp(conn, viewFullName); + Thread.sleep(1); + conn.createStatement().execute(divergeDDL); + //verify DDL timestamp changed + viewDDLTimestamp = CreateTableIT.verifyLastDDLTimestamp(schemaName, viewName, + viewFullName, viewDDLTimestamp, conn); + conn.createStatement().execute(viewColumnAddDDL); + //verify DDL timestamp changed because we added a column to the view + viewDDLTimestamp = CreateTableIT.verifyLastDDLTimestamp(schemaName, viewName, + viewFullName, viewDDLTimestamp, conn); + conn.createStatement().execute(viewColumnDropDDL); + //verify DDL timestamp changed because we dropped a column from the view + viewDDLTimestamp = CreateTableIT.verifyLastDDLTimestamp(schemaName, viewName, + viewFullName, viewDDLTimestamp, conn); + conn.createStatement().execute(tableColumnAddDDL); + //verify DDL timestamp DID NOT change because we added a column from the base table + assertEquals(viewDDLTimestamp, getLastDDLTimestamp(conn, viewFullName)); + conn.createStatement().execute(tableColumnDropDDL); + assertEquals(viewDDLTimestamp, getLastDDLTimestamp(conn, viewFullName)); + } + } + + @Test + public void testLastDDLTimestampWithChildViews() throws Exception { + Assume.assumeTrue(isMultiTenant); Review comment: @gjacoby126 This is not a comment for changing anything. This is just for my knowledge. ########## File path: phoenix-core/src/it/java/org/apache/phoenix/end2end/AlterTableWithViewsIT.java ########## @@ -1216,5 +1218,248 @@ 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(schemaName, dataTableName, + dataTableFullName, startTS, + conn); + Thread.sleep(1); + conn.createStatement().execute(viewDDL); + tableDDLTimestamp = CreateTableIT.verifyLastDDLTimestamp(schemaName, viewName, + viewFullName, tableDDLTimestamp + 1, conn); + Thread.sleep(1); + //now add a column and make sure the timestamp updates + conn.createStatement().execute(columnAddDDL); + tableDDLTimestamp = CreateTableIT.verifyLastDDLTimestamp(schemaName, viewName, + viewFullName, + tableDDLTimestamp + 1, conn); + Thread.sleep(1); + conn.createStatement().execute(columnDropDDL); + CreateTableIT.verifyLastDDLTimestamp(schemaName, viewName, + 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 + // extends to DDL timestamp + 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," + + " CONSTRAINT NAME_PK PRIMARY KEY (%s ID, COL1)" + + " ) %s"); + + String viewDDL = "CREATE VIEW " + viewFullName + " AS SELECT * FROM " + dataTableFullName; + + String divergeDDL = "ALTER VIEW " + viewFullName + " DROP COLUMN COL2"; + String viewColumnAddDDL = "ALTER VIEW " + viewFullName + " ADD COL3 varchar(50) NULL "; + String viewColumnDropDDL = "ALTER VIEW " + viewFullName + " DROP COLUMN COL3 "; + String tableColumnAddDDL = "ALTER TABLE " + dataTableFullName + " ADD COL4 varchar" + + "(50) NULL"; + String tableColumnDropDDL = "ALTER TABLE " + dataTableFullName + " DROP COLUMN COL4 "; + try (Connection conn = DriverManager.getConnection(getUrl())) { + conn.createStatement().execute(tableDDL); + conn.createStatement().execute(viewDDL); + long viewDDLTimestamp = getLastDDLTimestamp(conn, viewFullName); + Thread.sleep(1); + conn.createStatement().execute(divergeDDL); + //verify DDL timestamp changed + viewDDLTimestamp = CreateTableIT.verifyLastDDLTimestamp(schemaName, viewName, + viewFullName, viewDDLTimestamp, conn); + conn.createStatement().execute(viewColumnAddDDL); + //verify DDL timestamp changed because we added a column to the view + viewDDLTimestamp = CreateTableIT.verifyLastDDLTimestamp(schemaName, viewName, + viewFullName, viewDDLTimestamp, conn); + conn.createStatement().execute(viewColumnDropDDL); + //verify DDL timestamp changed because we dropped a column from the view + viewDDLTimestamp = CreateTableIT.verifyLastDDLTimestamp(schemaName, viewName, + viewFullName, viewDDLTimestamp, conn); + conn.createStatement().execute(tableColumnAddDDL); + //verify DDL timestamp DID NOT change because we added a column from the base table + assertEquals(viewDDLTimestamp, getLastDDLTimestamp(conn, viewFullName)); + conn.createStatement().execute(tableColumnDropDDL); + assertEquals(viewDDLTimestamp, getLastDDLTimestamp(conn, viewFullName)); + } + } + + @Test + public void testLastDDLTimestampWithChildViews() throws Exception { + Assume.assumeTrue(isMultiTenant); + Properties props = new Properties(); + String schemaName = SCHEMA1; + String dataTableName = "T_" + generateUniqueName(); + String globalViewName = "V_" + generateUniqueName(); + String tenantViewName = "V_" + generateUniqueName(); + String dataTableFullName = SchemaUtil.getTableName(schemaName, dataTableName); + String globalViewFullName = SchemaUtil.getTableName(schemaName, globalViewName); + String tenantViewFullName = SchemaUtil.getTableName(schemaName, tenantViewName); + + 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"); + + //create a table with a child global view, who then has a child tenant view + String globalViewDDL = + "CREATE VIEW " + globalViewFullName + " AS SELECT * FROM " + dataTableFullName; + + String tenantViewDDL = + "CREATE VIEW " + tenantViewFullName + " AS SELECT * FROM " + globalViewFullName; + + long startTS = EnvironmentEdgeManager.currentTimeMillis(); + long tableDDLTimestamp, globalViewDDLTimestamp; + + try (Connection conn = DriverManager.getConnection(getUrl())) { + conn.createStatement().execute(tableDDL); + conn.createStatement().execute(globalViewDDL); + tableDDLTimestamp = getLastDDLTimestamp(conn, dataTableFullName); + globalViewDDLTimestamp = getLastDDLTimestamp(conn, globalViewFullName); + } + props.setProperty(TENANT_ID_ATTRIB, TENANT1); + try (Connection tenantConn = DriverManager.getConnection(getUrl(), props)) { + tenantConn.createStatement().execute(tenantViewDDL); + } + // First, check that adding a child view didn't change the timestamps Review comment: @gjacoby126 looks like this comment was missed ? ########## File path: phoenix-core/src/it/java/org/apache/phoenix/end2end/AlterTableIT.java ########## @@ -1326,7 +1328,69 @@ public void testAddingColumnsToTablesAndViews() throws Exception { assertSequenceNumber(schemaName, viewName, PTable.INITIAL_SEQ_NUM + 1); } } - + + @Test + public void testAddThenDropColumnTableDDLTimestamp() throws Exception { + Properties props = new Properties(); + String tableDDL = "CREATE TABLE IF NOT EXISTS " + dataTableFullName + " (" + + " ENTITY_ID integer NOT NULL," + + " COL1 integer NOT NULL," + + " COL2 bigint NOT NULL," + + " CONSTRAINT NAME_PK PRIMARY KEY (ENTITY_ID, COL1, COL2)" + + " ) " + generateDDLOptions(""); + + String columnAddDDL = "ALTER TABLE " + dataTableFullName + " ADD COL3 varchar(50) NULL "; + String columnDropDDL = "ALTER TABLE " + dataTableFullName + " DROP COLUMN COL3 "; + long startTS = EnvironmentEdgeManager.currentTimeMillis(); + try (Connection conn = DriverManager.getConnection(getUrl(), props)) { Review comment: @gjacoby126 looks like this comment was missed ? ---------------------------------------------------------------- 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