virajjasani commented on a change in pull request #929:
URL: https://github.com/apache/phoenix/pull/929#discussion_r516787757



##########
File path: 
phoenix-core/src/main/java/org/apache/phoenix/schema/MetaDataClient.java
##########
@@ -1685,12 +1685,16 @@ public MutationState createIndex(CreateIndexStatement 
statement, byte[][] splits
                 for (ColumnName colName : requiredCols) {
                     // acquire the mutex using the global physical table name 
to
                     // prevent this column from being dropped while the view 
is being created
+                    String colNameSeparatedByDot = SchemaUtil
+                            .replaceColonToDot(colName.getColumnName());

Review comment:
       This seem preferred way based on existing usages:
   ```
   String colNameSeparatedByDot = 
colName.getColumnName().replace(QueryConstants.NAMESPACE_SEPARATOR, 
QueryConstants.NAME_SEPARATOR)
   ```

##########
File path: 
phoenix-core/src/it/java/org/apache/phoenix/end2end/index/ViewIndexIT.java
##########
@@ -280,6 +282,64 @@ public void testCoprocsOnGlobalNonMTImmutableViewIndex() 
throws Exception {
         testCoprocsOnGlobalViewIndexHelper(false, false);
     }
 
+    @Test
+    public void testDroppingColumnWhileCreatingIndex() throws Exception {
+        String schemaName = "S1";
+        String tableName = generateUniqueName();
+        String viewSchemaName = "S1";
+        String fullTableName = SchemaUtil.getTableName(schemaName, tableName);
+        String indexName = "IND_" + generateUniqueName();
+        String viewName = "VIEW_" + generateUniqueName();
+        String fullViewName = SchemaUtil.getTableName(viewSchemaName, 
viewName);
+
+        createBaseTable(schemaName, tableName, false, null, null, true);
+        try (Connection conn = getConnection()) {
+            conn.setAutoCommit(true);
+            conn.createStatement().execute("CREATE VIEW " + fullViewName + " 
AS SELECT * FROM " + fullTableName);
+            conn.commit();
+
+            final int[] exceptionCode = new int[1];

Review comment:
       Not a strong opinion but since we want threads to mutate exceptionCode, 
maybe we can use `AtomicInteger` for exceptionCode rather than array with size 
1, threads can set values and finally we can assert with 
`assertEquals(exceptionCode.get(), 301)` if you would prefer :)




----------------------------------------------------------------
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:
[email protected]


Reply via email to