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

chinmayskulkarni pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/phoenix.git


The following commit(s) were added to refs/heads/master by this push:
     new 197b6e3  PHOENIX-4743: ALTER TABLE ADD COLUMN for global index should 
not modify HBase metadata if failed
197b6e3 is described below

commit 197b6e30c894b657758c5d0cb3c6182d6c8d4723
Author: Sandeep Pal <sandeep....@salesforce.com>
AuthorDate: Sat Aug 24 13:45:26 2019 -0700

    PHOENIX-4743: ALTER TABLE ADD COLUMN for global index should not modify 
HBase metadata if failed
    
    Signed-off-by: Chinmay Kulkarni <chinmayskulka...@apache.org>
---
 .../org/apache/phoenix/end2end/AlterTableIT.java   | 32 ++++++++++++++++++++++
 .../phoenix/query/ConnectionQueryServicesImpl.java | 10 +++++--
 2 files changed, 39 insertions(+), 3 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 7912c58..c2c02de 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
@@ -17,6 +17,7 @@
  */
 package org.apache.phoenix.end2end;
 
+import static 
org.apache.phoenix.exception.SQLExceptionCode.CANNOT_MUTATE_TABLE;
 import static org.apache.phoenix.jdbc.PhoenixDatabaseMetaData.COLUMN_FAMILY;
 import static org.apache.phoenix.jdbc.PhoenixDatabaseMetaData.COLUMN_NAME;
 import static org.apache.phoenix.jdbc.PhoenixDatabaseMetaData.COLUMN_QUALIFIER;
@@ -51,6 +52,7 @@ import org.apache.hadoop.hbase.client.Admin;
 import org.apache.hadoop.hbase.client.ColumnFamilyDescriptor;
 import org.apache.hadoop.hbase.client.TableDescriptor;
 import org.apache.hadoop.hbase.util.Bytes;
+import org.apache.phoenix.exception.PhoenixParserException;
 import org.apache.phoenix.exception.SQLExceptionCode;
 import org.apache.phoenix.jdbc.PhoenixConnection;
 import org.apache.phoenix.jdbc.PhoenixDatabaseMetaData;
@@ -852,6 +854,36 @@ public class AlterTableIT extends ParallelStatsDisabledIT {
         conn1.close();
     }
 
+    @Test
+    public void testAlterTableOnGlobalIndex() throws Exception {
+        try (Connection conn = DriverManager.getConnection(getUrl());
+             Statement stmt = conn.createStatement()) {
+            conn.setAutoCommit(false);
+            Admin admin = 
conn.unwrap(PhoenixConnection.class).getQueryServices().getAdmin();
+            String tableName = generateUniqueName();
+            String globalIndexTableName = generateUniqueName();
+
+            stmt.execute("CREATE TABLE " + tableName +
+                " (ID INTEGER PRIMARY KEY, COL1 VARCHAR(10), COL2 BOOLEAN)");
+
+            stmt.execute("CREATE INDEX " + globalIndexTableName + " on " + 
tableName + " (COL2)");
+            TableDescriptor originalDesc = 
admin.getDescriptor(TableName.valueOf(globalIndexTableName));
+            int expectedErrorCode = 0;
+            try {
+                stmt.execute("ALTER TABLE " + globalIndexTableName + " ADD 
CF1.AGE INTEGER ");
+                conn.commit();
+                fail("The alter table did not fail as expected");
+            } catch (SQLException e) {
+                assertEquals(e.getErrorCode(), 
CANNOT_MUTATE_TABLE.getErrorCode());
+            }
+
+            TableDescriptor finalDesc = 
admin.getDescriptor(TableName.valueOf(globalIndexTableName));
+            assertTrue(finalDesc.equals(originalDesc));
+
+            // drop the table
+            stmt.execute("DROP TABLE " + tableName);
+        }
+    }
 
     @Test
     public void testAlterStoreNulls() throws SQLException {
diff --git 
a/phoenix-core/src/main/java/org/apache/phoenix/query/ConnectionQueryServicesImpl.java
 
b/phoenix-core/src/main/java/org/apache/phoenix/query/ConnectionQueryServicesImpl.java
index 4112984..e5c935d 100644
--- 
a/phoenix-core/src/main/java/org/apache/phoenix/query/ConnectionQueryServicesImpl.java
+++ 
b/phoenix-core/src/main/java/org/apache/phoenix/query/ConnectionQueryServicesImpl.java
@@ -2060,9 +2060,6 @@ public class ConnectionQueryServicesImpl extends 
DelegateQueryServices implement
                 // When adding a column to a view, base physical table should 
only be modified when new column families are being added.
                 modifyHTable = canViewsAddNewCF && 
!existingColumnFamiliesForBaseTable(table.getPhysicalName()).containsAll(colFamiliesForPColumnsToBeAdded);
             }
-            if (modifyHTable) {
-                sendHBaseMetaData(tableDescriptors, pollingNeeded);
-            }
 
             // Special case for call during drop table to ensure that the 
empty column family exists.
             // In this, case we only include the table header row, as until we 
add schemaBytes and tableBytes
@@ -2070,6 +2067,9 @@ public class ConnectionQueryServicesImpl extends 
DelegateQueryServices implement
             // TODO: change to  if (tableMetaData.isEmpty()) once we pass 
through schemaBytes and tableBytes
             // Also, could be used to update property values on ALTER TABLE t 
SET prop=xxx
             if ((tableMetaData.isEmpty()) || (tableMetaData.size() == 1 && 
tableMetaData.get(0).isEmpty())) {
+                if (modifyHTable) {
+                    sendHBaseMetaData(tableDescriptors, pollingNeeded);
+                }
                 return new MetaDataMutationResult(MutationCode.NO_OP, 
EnvironmentEdgeManager.currentTimeMillis(), table);
             }
             byte[][] rowKeyMetaData = new byte[3][];
@@ -2127,6 +2127,10 @@ public class ConnectionQueryServicesImpl extends 
DelegateQueryServices implement
                     }
                 }
             }
+
+            if (modifyHTable && result.getMutationCode() != 
MutationCode.UNALLOWED_TABLE_MUTATION) {
+                sendHBaseMetaData(tableDescriptors, pollingNeeded);
+            }
         } finally {
             // If we weren't successful with our metadata update
             // and we've already pushed the HBase metadata changes to the 
server

Reply via email to