This is an automated email from the ASF dual-hosted git repository. pboado pushed a commit to branch 5.x-cdh6 in repository https://gitbox.apache.org/repos/asf/phoenix.git
commit 2ddfa02128dd653b0091033e416659033c306052 Author: Toshihiro Suzuki <brfrn...@gmail.com> AuthorDate: Thu Mar 21 07:46:09 2019 +0000 PHOENIX-1614 ALTER TABLE ADD IF NOT EXISTS doesn't work as expected (cherry picked from commit 69e5bb0b304a53967cef40b2a4cfc66e69ecaa51) --- .../org/apache/phoenix/end2end/AlterTableIT.java | 43 +++++++++++----------- .../org/apache/phoenix/schema/MetaDataClient.java | 29 +++++++-------- 2 files changed, 35 insertions(+), 37 deletions(-) diff --git a/phoenix-core/src/it/java/org/apache/phoenix/end2end/AlterTableIT.java b/phoenix-core/src/it/java/org/apache/phoenix/end2end/AlterTableIT.java index 64f0349..a05132e 100644 --- a/phoenix-core/src/it/java/org/apache/phoenix/end2end/AlterTableIT.java +++ b/phoenix-core/src/it/java/org/apache/phoenix/end2end/AlterTableIT.java @@ -330,17 +330,15 @@ public class AlterTableIT extends ParallelStatsDisabledIT { } - @Test public void testAddVarCols() throws Exception { Properties props = PropertiesUtil.deepCopy(TEST_PROPERTIES); - Connection conn = DriverManager.getConnection(getUrl(), props); - conn.setAutoCommit(false); + try (Connection conn = DriverManager.getConnection(getUrl(), props)) { + conn.setAutoCommit(false); - try { String ddl = "CREATE TABLE " + dataTableFullName + - " (a_string varchar not null, col1 integer" + - " CONSTRAINT pk PRIMARY KEY (a_string)) " + tableDDLOptions; + " (a_string varchar not null, col1 integer" + + " CONSTRAINT pk PRIMARY KEY (a_string)) " + tableDDLOptions; conn.createStatement().execute(ddl); String dml = "UPSERT INTO " + dataTableFullName + " VALUES(?)"; @@ -359,16 +357,18 @@ public class AlterTableIT extends ParallelStatsDisabledIT { assertEquals("b",rs.getString(1)); assertFalse(rs.next()); - query = "SELECT * FROM " + dataTableFullName + " WHERE a_string = 'a' "; rs = conn.createStatement().executeQuery(query); assertTrue(rs.next()); assertEquals("a",rs.getString(1)); - ddl = "ALTER TABLE " + dataTableFullName + " ADD c1.col2 VARCHAR , c1.col3 integer , c2.col4 integer"; + ddl = "ALTER TABLE " + dataTableFullName + " ADD c1.col2 VARCHAR, c1.col3 integer, " + + "c2.col4 integer"; conn.createStatement().execute(ddl); - ddl = "ALTER TABLE " + dataTableFullName + " ADD col5 integer , c1.col2 VARCHAR"; + // If we are adding two columns but one of them already exists, the other one should + // not be added + ddl = "ALTER TABLE " + dataTableFullName + " ADD col5 integer, c1.col2 VARCHAR"; try { conn.createStatement().execute(ddl); fail(); @@ -384,10 +384,7 @@ public class AlterTableIT extends ParallelStatsDisabledIT { assertTrue(e.getMessage(), e.getMessage().contains("ERROR 504 (42703): Undefined column.")); } - ddl = "ALTER TABLE " + dataTableFullName + " ADD IF NOT EXISTS col5 integer , c1.col2 VARCHAR"; - conn.createStatement().execute(ddl); - - dml = "UPSERT INTO " + dataTableFullName + " VALUES(?,?,?,?,?)"; + dml = "UPSERT INTO " + dataTableFullName + " VALUES(?, ?, ?, ?, ?)"; stmt = conn.prepareStatement(dml); stmt.setString(1, "c"); stmt.setInt(2, 100); @@ -407,9 +404,6 @@ public class AlterTableIT extends ParallelStatsDisabledIT { assertEquals(102,rs.getInt(5)); assertFalse(rs.next()); - ddl = "ALTER TABLE " + dataTableFullName + " ADD col5 integer"; - conn.createStatement().execute(ddl); - query = "SELECT c1.* FROM " + dataTableFullName + " WHERE a_string = 'c' "; rs = conn.createStatement().executeQuery(query); assertTrue(rs.next()); @@ -417,8 +411,16 @@ public class AlterTableIT extends ParallelStatsDisabledIT { assertEquals(101,rs.getInt(2)); assertFalse(rs.next()); + // If we are adding two columns with "IF NOT EXISTS" and one of them already exists, + // the other one should be added + ddl = "ALTER TABLE " + dataTableFullName + " ADD IF NOT EXISTS col5 integer, " + + "c1.col2 VARCHAR"; + conn.createStatement().execute(ddl); - dml = "UPSERT INTO " + dataTableFullName + "(a_string,col1,col5) VALUES(?,?,?)"; + query = "SELECT col5 FROM " + dataTableFullName; + conn.createStatement().executeQuery(query); + + dml = "UPSERT INTO " + dataTableFullName + "(a_string, col1, col5) VALUES(?, ?, ?)"; stmt = conn.prepareStatement(dml); stmt.setString(1, "e"); stmt.setInt(2, 200); @@ -426,17 +428,14 @@ public class AlterTableIT extends ParallelStatsDisabledIT { stmt.execute(); conn.commit(); - - query = "SELECT a_string,col1,col5 FROM " + dataTableFullName + " WHERE a_string = 'e' "; + query = "SELECT a_string, col1, col5 FROM " + dataTableFullName + + " WHERE a_string = 'e' "; rs = conn.createStatement().executeQuery(query); assertTrue(rs.next()); assertEquals("e",rs.getString(1)); assertEquals(200,rs.getInt(2)); assertEquals(201,rs.getInt(3)); assertFalse(rs.next()); - - } finally { - conn.close(); } } diff --git a/phoenix-core/src/main/java/org/apache/phoenix/schema/MetaDataClient.java b/phoenix-core/src/main/java/org/apache/phoenix/schema/MetaDataClient.java index 0527664..965f637 100644 --- a/phoenix-core/src/main/java/org/apache/phoenix/schema/MetaDataClient.java +++ b/phoenix-core/src/main/java/org/apache/phoenix/schema/MetaDataClient.java @@ -3523,7 +3523,8 @@ public class MetaDataClient { throws SQLException { connection.rollback(); boolean wasAutoCommit = connection.getAutoCommit(); - List<PColumn> columns = Lists.newArrayListWithExpectedSize(origColumnDefs != null ? origColumnDefs.size() : 0); + List<PColumn> columns = Lists.newArrayListWithExpectedSize(origColumnDefs != null ? + origColumnDefs.size() : 0); PName tenantId = connection.getTenantId(); String schemaName = table.getSchemaName().getString(); String tableName = table.getTableName().getString(); @@ -3537,40 +3538,38 @@ public class MetaDataClient { try { connection.setAutoCommit(false); - List<ColumnDef> columnDefs = null; - if (table.isAppendOnlySchema()) { + List<ColumnDef> columnDefs; + if (table.isAppendOnlySchema() || ifNotExists) { // only make the rpc if we are adding new columns columnDefs = Lists.newArrayList(); for (ColumnDef columnDef : origColumnDefs) { String familyName = columnDef.getColumnDefName().getFamilyName(); String columnName = columnDef.getColumnDefName().getColumnName(); - if (familyName!=null) { + if (familyName != null) { try { PColumnFamily columnFamily = table.getColumnFamily(familyName); columnFamily.getPColumnForColumnName(columnName); if (!ifNotExists) { - throw new ColumnAlreadyExistsException(schemaName, tableName, columnName); + throw new ColumnAlreadyExistsException(schemaName, tableName, + columnName); } - } - catch (ColumnFamilyNotFoundException | ColumnNotFoundException e){ + } catch (ColumnFamilyNotFoundException | ColumnNotFoundException e) { columnDefs.add(columnDef); } - } - else { + } else { try { table.getColumnForColumnName(columnName); if (!ifNotExists) { - throw new ColumnAlreadyExistsException(schemaName, tableName, columnName); + throw new ColumnAlreadyExistsException(schemaName, tableName, + columnName); } - } - catch (ColumnNotFoundException e){ + } catch (ColumnNotFoundException e) { columnDefs.add(columnDef); } } } - } - else { - columnDefs = origColumnDefs == null ? Collections.<ColumnDef>emptyList() : origColumnDefs; + } else { + columnDefs = origColumnDefs == null ? Collections.emptyList() : origColumnDefs; } boolean retried = false;