From 18e19ebb9af0c64ba71cd5e76eae80c9759e6d52 Mon Sep 17 00:00:00 2001 From: Zhen Li Date: Tue, 20 Jul 2021 14:04:26 +0200 Subject: [PATCH] DB-12365 Fix a regression caused by DB-12158 The check on column names has to be removed due to the possibility of renaming columns names in defining views. --- .../db/impl/sql/compile/FromSubquery.java | 9 --- .../execute/operations/DropAddColumnIT.java | 80 ++++++++++++++++++- 2 files changed, 76 insertions(+), 13 deletions(-) diff --git a/db-engine/src/main/java/com/splicemachine/db/impl/sql/compile/FromSubquery.java b/db-engine/src/main/java/com/splicemachine/db/impl/sql/compile/FromSubquery.java index 9e4f72a94b6..3744c49976d 100644 --- a/db-engine/src/main/java/com/splicemachine/db/impl/sql/compile/FromSubquery.java +++ b/db-engine/src/main/java/com/splicemachine/db/impl/sql/compile/FromSubquery.java @@ -316,15 +316,6 @@ public void bindExpressions(FromList fromListParam) } else if (resultColumns.size() > subqueryRCL.size()) { // columns dropped from underlying table throw StandardException.newException(SQLState.LANG_DERIVED_COLUMN_LIST_MISMATCH, correlationName); - } else { - // number of columns is OK, check column names - for (int i = 0; i < resultColumns.size(); ++i) { - ResultColumn thisRC = resultColumns.elementAt(i); - ResultColumn subqRC = subqueryRCL.elementAt(i); - if (thisRC.getName() != null && !thisRC.getName().equals(subqRC.getName())) { - throw StandardException.newException(SQLState.LANG_DERIVED_COLUMN_NAME_USE, thisRC.getName()); - } - } } } diff --git a/splice_machine/src/test/java/com/splicemachine/derby/impl/sql/execute/operations/DropAddColumnIT.java b/splice_machine/src/test/java/com/splicemachine/derby/impl/sql/execute/operations/DropAddColumnIT.java index 1f4b9ad951d..2181fee22bd 100644 --- a/splice_machine/src/test/java/com/splicemachine/derby/impl/sql/execute/operations/DropAddColumnIT.java +++ b/splice_machine/src/test/java/com/splicemachine/derby/impl/sql/execute/operations/DropAddColumnIT.java @@ -395,6 +395,69 @@ public void testAutomaticViewRefreshingOn_SelectStarView() throws Exception { methodWatcher.execute("call syscs_util.syscs_set_global_database_property('splice.execution.alterTable.autoViewRefreshing', null)"); } + @Test + public void testAutomaticViewRefreshingOn_SelectStarViewWithRenamedColumns() throws Exception { + // Turn on alter table auto view refreshing. + methodWatcher.execute("call syscs_util.syscs_set_global_database_property('splice.execution.alterTable.autoViewRefreshing', true)"); + + Harness harness = new Harness(connection); + + Harness.Table table = harness.createTable("t33", "val1", "val2") + .begin() + .addRow(10, 10) + .addRow(20, 20); + + methodWatcher.execute("create view v33 (v1, v2) as select * from t33"); // rename columns to v1, v2 + String viewQuery = "select * from v33"; + + try (ResultSet rs = methodWatcher.executeQuery(viewQuery)) { + String expected = "V1 |V2 |\n" + + "--------\n" + + "10 |10 |\n" + + "20 |20 |"; + Assert.assertEquals(expected, TestUtils.FormattedResult.ResultFactory.toString(rs)); + } + + try { + table = table.addColumn("val3", INT); + Assert.fail("expect failure because it's unknown how to rename columns"); + } catch (SQLException sqle) { + Assert.assertEquals("42X56", sqle.getSQLState()); + Assert.assertEquals("The number of columns in the view column list does not match the number of columns in the underlying query expression in the view definition for 'V33'.", + sqle.getMessage()); + } + + try { + table = table.dropColumn("val2"); + Assert.fail("expect failure because it's unknown how to rename columns"); + } catch (SQLException sqle) { + Assert.assertEquals("42X56", sqle.getSQLState()); + Assert.assertEquals("The number of columns in the view column list does not match the number of columns in the underlying query expression in the view definition for 'V33'.", + sqle.getMessage()); + } + + // select v33 should not change + try (ResultSet rs = methodWatcher.executeQuery(viewQuery)) { + String expected = "V1 |V2 |\n" + + "--------\n" + + "10 |10 |\n" + + "20 |20 |"; + Assert.assertEquals(expected, TestUtils.FormattedResult.ResultFactory.toString(rs)); + } + + // select t33 should not change, new column is not added + try (ResultSet rs = methodWatcher.executeQuery("select * from t33")) { + String expected = "VAL1 |VAL2 |\n" + + "------------\n" + + " 10 | 10 |\n" + + " 20 | 20 |"; + Assert.assertEquals(expected, TestUtils.FormattedResult.ResultFactory.toString(rs)); + } + + // Turn off alter table auto view refreshing. + methodWatcher.execute("call syscs_util.syscs_set_global_database_property('splice.execution.alterTable.autoViewRefreshing', null)"); + } + @Test public void testAutomaticViewRefreshingOn_SecondLevelSelectStarView() throws Exception { // Turn on alter table auto view refreshing. @@ -531,12 +594,21 @@ public void testAutomaticViewRefreshingOff() throws Exception { Assert.assertEquals(expected, TestUtils.FormattedResult.ResultFactory.toString(rs)); } + // DB-12365 + // Unfortunately, we cannot check for the following case because of the possibility to rename + // view columns. In select statement, there is no way to tell if the column names are different + // due to alter table operations or just renaming in view definition. Checking column positions + // are also not possible because view columns are always 1, 2, 3, .... It has to be this way + // because underlying select could have generated columns that do no exists in any base tables + // at all. As a result, the following case returns a result with wrong column names if views + // are not automatically refreshed. table = table.dropColumn("val1"); try (ResultSet rs = methodWatcher.executeQuery(viewQuery)) { - Assert.fail("expect to fail because view definition is on column val1 and val2, but val1 is dropped"); - } catch (SQLException sqle) { - Assert.assertEquals("42X21", sqle.getSQLState()); - Assert.assertTrue(sqle.getMessage().contains("'VAL1' is a special derived column which may not be defined in DDL statements.")); + String tmpExpected = "VAL1 |VAL2 |\n" + + "------------\n" + + " 10 |NULL |\n" + + " 20 |NULL |"; + Assert.assertEquals(tmpExpected, TestUtils.FormattedResult.ResultFactory.toString(rs)); } table = table.dropColumn("val3");