You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@phoenix.apache.org by ja...@apache.org on 2019/03/24 01:17:57 UTC

[phoenix] branch 4.x-HBase-1.2 updated: PHOENIX-1614 ALTER TABLE ADD IF NOT EXISTS doesn't work as expected

This is an automated email from the ASF dual-hosted git repository.

jaanai pushed a commit to branch 4.x-HBase-1.2
in repository https://gitbox.apache.org/repos/asf/phoenix.git
The following commit(s) were added to refs/heads/4.x-HBase-1.2 by this push:
     new 03ce151  PHOENIX-1614 ALTER TABLE ADD IF NOT EXISTS doesn't work as expected
03ce151 is described below

commit 03ce1516c6ee5dd878df6566a0b356e350027468
Author: Toshihiro Suzuki <br...@gmail.com>
AuthorDate: Thu Mar 21 16:46:09 2019 +0900

    PHOENIX-1614 ALTER TABLE ADD IF NOT EXISTS doesn't work as expected
---
 .../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 7af62b3..c9b2ab1 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
@@ -329,17 +329,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(?)";
@@ -358,16 +356,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();
@@ -383,10 +383,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);
@@ -406,9 +403,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());
@@ -416,8 +410,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);
@@ -425,17 +427,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 c50f644..be6d286 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
@@ -3533,7 +3533,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();
@@ -3547,40 +3548,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;