Repository: phoenix
Updated Branches:
  refs/heads/4.x-HBase-1.0 8d003ad64 -> 27d78b653


PHOENIX-2058 Check for existence and compatibility of columns being added in 
view


Project: http://git-wip-us.apache.org/repos/asf/phoenix/repo
Commit: http://git-wip-us.apache.org/repos/asf/phoenix/commit/27d78b65
Tree: http://git-wip-us.apache.org/repos/asf/phoenix/tree/27d78b65
Diff: http://git-wip-us.apache.org/repos/asf/phoenix/diff/27d78b65

Branch: refs/heads/4.x-HBase-1.0
Commit: 27d78b65381d22bea1cf76069fa39650e5eb6b2b
Parents: f542971
Author: Thomas D'Silva <tdsi...@salesforce.com>
Authored: Fri Jul 10 11:55:55 2015 -0700
Committer: Thomas D'Silva <tdsi...@salesforce.com>
Committed: Mon Jul 13 17:32:23 2015 -0700

----------------------------------------------------------------------
 .../apache/phoenix/end2end/AlterTableIT.java    | 311 ++++++++++++++++---
 .../java/org/apache/phoenix/end2end/ViewIT.java |   6 +
 .../coprocessor/MetaDataEndpointImpl.java       |  65 ++--
 .../apache/phoenix/schema/MetaDataClient.java   |   9 +-
 4 files changed, 311 insertions(+), 80 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/phoenix/blob/27d78b65/phoenix-core/src/it/java/org/apache/phoenix/end2end/AlterTableIT.java
----------------------------------------------------------------------
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 0425933..607f52a 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
@@ -2009,7 +2009,7 @@ public class AlterTableIT extends 
BaseOwnClusterHBaseManagedTimeIT {
     }
     
     @Test
-    public void testAddNewColumnToBaseTableWithViews() throws Exception {
+    public void testAddNewColumnsToBaseTableWithViews() throws Exception {
         Connection conn = DriverManager.getConnection(getUrl());
         try {       
             conn.createStatement().execute("CREATE TABLE IF NOT EXISTS 
TABLEWITHVIEW ("
@@ -2018,13 +2018,15 @@ public class AlterTableIT extends 
BaseOwnClusterHBaseManagedTimeIT {
                     + " COL2 bigint NOT NULL,"
                     + " CONSTRAINT NAME_PK PRIMARY KEY (ID, COL1, COL2)"
                     + " )");
-            assertTableDefinition(conn, "TABLEWITHVIEW", PTableType.TABLE, 
null, 0, 3, -1, "ID", "COL1", "COL2");
+            assertTableDefinition(conn, "TABLEWITHVIEW", PTableType.TABLE, 
null, 0, 3, QueryConstants.BASE_TABLE_BASE_COLUMN_COUNT, "ID", "COL1", "COL2");
             
             conn.createStatement().execute("CREATE VIEW VIEWOFTABLE ( 
VIEW_COL1 DECIMAL(10,2), VIEW_COL2 VARCHAR ) AS SELECT * FROM TABLEWITHVIEW");
             assertTableDefinition(conn, "VIEWOFTABLE", PTableType.VIEW, 
"TABLEWITHVIEW", 0, 5, 3, "ID", "COL1", "COL2", "VIEW_COL1", "VIEW_COL2");
             
-            conn.createStatement().execute("ALTER TABLE TABLEWITHVIEW ADD COL3 
char(10)");
-            assertTableDefinition(conn, "VIEWOFTABLE", PTableType.VIEW, 
"TABLEWITHVIEW", 1, 6, 4, "ID", "COL1", "COL2", "COL3", "VIEW_COL1", 
"VIEW_COL2");
+            // adding a new pk column and a new regular column
+            conn.createStatement().execute("ALTER TABLE TABLEWITHVIEW ADD COL3 
varchar(10) PRIMARY KEY, COL4 integer");
+            assertTableDefinition(conn, "TABLEWITHVIEW", PTableType.TABLE, 
null, 1, 5, QueryConstants.BASE_TABLE_BASE_COLUMN_COUNT, "ID", "COL1", "COL2", 
"COL3", "COL4");
+            assertTableDefinition(conn, "VIEWOFTABLE", PTableType.VIEW, 
"TABLEWITHVIEW", 1, 7, 5, "ID", "COL1", "COL2", "COL3", "COL4", "VIEW_COL1", 
"VIEW_COL2");
         } finally {
             conn.close();
         }
@@ -2040,13 +2042,13 @@ public class AlterTableIT extends 
BaseOwnClusterHBaseManagedTimeIT {
                     + " COL2 bigint NOT NULL,"
                     + " CONSTRAINT NAME_PK PRIMARY KEY (ID, COL1, COL2)"
                     + " )");
-            assertTableDefinition(conn, "TABLEWITHVIEW", PTableType.TABLE, 
null, 0, 3, -1, "ID", "COL1", "COL2");
+            assertTableDefinition(conn, "TABLEWITHVIEW", PTableType.TABLE, 
null, 0, 3, QueryConstants.BASE_TABLE_BASE_COLUMN_COUNT, "ID", "COL1", "COL2");
             
-            conn.createStatement().execute("CREATE VIEW VIEWOFTABLE ( 
VIEW_COL1 DECIMAL(10,2), VIEW_COL2 VARCHAR(256), VIEW_COL3 VARCHAR, VIEW_COL4 
DECIMAL ) AS SELECT * FROM TABLEWITHVIEW");
-            assertTableDefinition(conn, "VIEWOFTABLE", PTableType.VIEW, 
"TABLEWITHVIEW", 0, 7, 3, "ID", "COL1", "COL2", "VIEW_COL1", "VIEW_COL2", 
"VIEW_COL3", "VIEW_COL4");
+            conn.createStatement().execute("CREATE VIEW VIEWOFTABLE ( 
VIEW_COL1 DECIMAL(10,2), VIEW_COL2 VARCHAR(256), VIEW_COL3 VARCHAR, VIEW_COL4 
DECIMAL, VIEW_COL5 DECIMAL(10,2), VIEW_COL6 VARCHAR, CONSTRAINT pk PRIMARY KEY 
(VIEW_COL5, VIEW_COL6) ) AS SELECT * FROM TABLEWITHVIEW");
+            assertTableDefinition(conn, "VIEWOFTABLE", PTableType.VIEW, 
"TABLEWITHVIEW", 0, 9, 3, "ID", "COL1", "COL2", "VIEW_COL1", "VIEW_COL2", 
"VIEW_COL3", "VIEW_COL4", "VIEW_COL5", "VIEW_COL6");
             
             // upsert single row into view
-            String dml = "UPSERT INTO VIEWOFTABLE VALUES(?,?,?,?,?, ?, ?)";
+            String dml = "UPSERT INTO VIEWOFTABLE VALUES(?,?,?,?,?, ?, ?, ?, 
?)";
             PreparedStatement stmt = conn.prepareStatement(dml);
             stmt.setString(1, "view1");
             stmt.setInt(2, 12);
@@ -2055,6 +2057,8 @@ public class AlterTableIT extends 
BaseOwnClusterHBaseManagedTimeIT {
             stmt.setString(5, "view5");
             stmt.setString(6, "view6");
             stmt.setInt(7, 17);
+            stmt.setInt(8, 18);
+            stmt.setString(9, "view9");
             stmt.execute();
             conn.commit();
             
@@ -2068,7 +2072,7 @@ public class AlterTableIT extends 
BaseOwnClusterHBaseManagedTimeIT {
             }           
             
             try {
-               // should fail because there is already a view column with same 
name with a higher scale
+               // should fail because there is already a view column with same 
name with different scale
                conn.createStatement().execute("ALTER TABLE TABLEWITHVIEW ADD 
VIEW_COL1 DECIMAL(10,1)");
                fail();
             }
@@ -2077,8 +2081,8 @@ public class AlterTableIT extends 
BaseOwnClusterHBaseManagedTimeIT {
             } 
             
             try {
-               // should fail because there is already a view column with same 
name with a greater length
-               conn.createStatement().execute("ALTER TABLE TABLEWITHVIEW ADD 
VIEW_COL1 VARCHAR(256)");
+               // should fail because there is already a view column with same 
name with different length
+               conn.createStatement().execute("ALTER TABLE TABLEWITHVIEW ADD 
VIEW_COL1 DECIMAL(9,2)");
                fail();
             }
             catch (SQLException e) {
@@ -2086,8 +2090,8 @@ public class AlterTableIT extends 
BaseOwnClusterHBaseManagedTimeIT {
             } 
             
             try {
-               // should fail because there is already a view column with null 
length
-               conn.createStatement().execute("ALTER TABLE TABLEWITHVIEW ADD 
VIEW_COL2 DECIMAL(9,2)");
+               // should fail because there is already a view column with 
different length
+               conn.createStatement().execute("ALTER TABLE TABLEWITHVIEW ADD 
VIEW_COL2 VARCHAR");
                fail();
             }
             catch (SQLException e) {
@@ -2095,35 +2099,36 @@ public class AlterTableIT extends 
BaseOwnClusterHBaseManagedTimeIT {
             }
             
             // validate that there were no columns added to the table or view
-            assertTableDefinition(conn, "TABLEWITHVIEW", PTableType.TABLE, 
null, 0, 3, -1, "ID", "COL1", "COL2");
-            assertTableDefinition(conn, "VIEWOFTABLE", PTableType.VIEW, 
"TABLEWITHVIEW", 0, 7, 3, "ID", "COL1", "COL2", "VIEW_COL1", "VIEW_COL2", 
"VIEW_COL3", "VIEW_COL4");
+            assertTableDefinition(conn, "TABLEWITHVIEW", PTableType.TABLE, 
null, 0, 3, QueryConstants.BASE_TABLE_BASE_COLUMN_COUNT, "ID", "COL1", "COL2");
+            assertTableDefinition(conn, "VIEWOFTABLE", PTableType.VIEW, 
"TABLEWITHVIEW", 0, 9, 3, "ID", "COL1", "COL2", "VIEW_COL1", "VIEW_COL2", 
"VIEW_COL3", "VIEW_COL4", "VIEW_COL5", "VIEW_COL6");
             
-            // should succeed because even though there already is a view 
column with same name,
-            // the column being added has a scale and max length that is equal 
to or greater
-            conn.createStatement().execute("ALTER TABLE TABLEWITHVIEW ADD 
VIEW_COL2 VARCHAR, VIEW_COL4 DECIMAL");
-            assertTableDefinition(conn, "TABLEWITHVIEW", PTableType.TABLE, 
null, 1, 4, -1, "ID", "COL1", "COL2", "VIEW_COL2", "VIEW_COL4");
-            assertTableDefinition(conn, "VIEWOFTABLE", PTableType.VIEW, 
"TABLEWITHVIEW", 1, 7, 5, "ID", "COL1", "COL2", "VIEW_COL1", "VIEW_COL2", 
"VIEW_COL3", "VIEW_COL4");
+            // should succeed 
+            conn.createStatement().execute("ALTER TABLE TABLEWITHVIEW ADD 
VIEW_COL4 DECIMAL, VIEW_COL2 VARCHAR(256)");
+            assertTableDefinition(conn, "TABLEWITHVIEW", PTableType.TABLE, 
null, 1, 5, QueryConstants.BASE_TABLE_BASE_COLUMN_COUNT, "ID", "COL1", "COL2", 
"VIEW_COL4", "VIEW_COL2");
+            assertTableDefinition(conn, "VIEWOFTABLE", PTableType.VIEW, 
"TABLEWITHVIEW", 1, 9, 5, "ID", "COL1", "COL2", "VIEW_COL1", "VIEW_COL2", 
"VIEW_COL3", "VIEW_COL4", "VIEW_COL5", "VIEW_COL6");
             
             // query table
             ResultSet rs = stmt.executeQuery("SELECT * FROM TABLEWITHVIEW");
             assertTrue(rs.next());
-            assertEquals(rs.getString(1), "view1");
-            assertEquals(rs.getInt(2), 12);
-            assertEquals(rs.getInt(3), 13);
-            assertEquals(rs.getString(4), "view5");
-            assertEquals(rs.getInt(5), 17);
+            assertEquals("view1", rs.getString("ID"));
+            assertEquals(12, rs.getInt("COL1"));
+            assertEquals(13, rs.getInt("COL2"));
+            assertEquals("view5", rs.getString("VIEW_COL2"));
+            assertEquals(17, rs.getInt("VIEW_COL4"));
             assertFalse(rs.next());
 
             // query view
             rs = stmt.executeQuery("SELECT * FROM VIEWOFTABLE");
             assertTrue(rs.next());
-            assertEquals(rs.getString(1), "view1");
-            assertEquals(rs.getInt(2), 12);
-            assertEquals(rs.getInt(3), 13);
-            assertEquals(rs.getInt(4), 14);
-            assertEquals(rs.getString(5),"view5");
-            assertEquals(rs.getString(6),"view6");
-            assertEquals(rs.getInt(7), 17);
+            assertEquals("view1", rs.getString("ID"));
+            assertEquals(12, rs.getInt("COL1"));
+            assertEquals(13, rs.getInt("COL2"));
+            assertEquals(14, rs.getInt("VIEW_COL1"));
+            assertEquals("view5", rs.getString("VIEW_COL2"));
+            assertEquals("view6", rs.getString("VIEW_COL3"));
+            assertEquals(17, rs.getInt("VIEW_COL4"));
+            assertEquals(18, rs.getInt("VIEW_COL5"));
+            assertEquals("view9", rs.getString("VIEW_COL6"));
             assertFalse(rs.next());
         } finally {
             conn.close();
@@ -2140,7 +2145,7 @@ public class AlterTableIT extends 
BaseOwnClusterHBaseManagedTimeIT {
                     + " COL2 integer NOT NULL,"
                     + " CONSTRAINT NAME_PK PRIMARY KEY (ID, COL1, COL2)"
                     + " )");
-            assertTableDefinition(conn, "TABLEWITHVIEW", PTableType.TABLE, 
null, 0, 3, -1, "ID", "COL1", "COL2");
+            assertTableDefinition(conn, "TABLEWITHVIEW", PTableType.TABLE, 
null, 0, 3, QueryConstants.BASE_TABLE_BASE_COLUMN_COUNT, "ID", "COL1", "COL2");
             
             conn.createStatement().execute("CREATE VIEW VIEWOFTABLE ( 
VIEW_COL1 DECIMAL(10,2), VIEW_COL2 VARCHAR(256) CONSTRAINT pk PRIMARY KEY 
(VIEW_COL1, VIEW_COL2)) AS SELECT * FROM TABLEWITHVIEW");
             assertTableDefinition(conn, "VIEWOFTABLE", PTableType.VIEW, 
"TABLEWITHVIEW", 0, 5, 3, "ID", "COL1", "COL2", "VIEW_COL1", "VIEW_COL2");
@@ -2157,8 +2162,53 @@ public class AlterTableIT extends 
BaseOwnClusterHBaseManagedTimeIT {
             conn.commit();
             
             try {
-               // should fail because there is already a pk view column with 
same name is a different slot 
-               conn.createStatement().execute("ALTER TABLE TABLEWITHVIEW ADD 
VIEW_COL2 VARCHAR PRIMARY KEY");
+               // should fail because there we have to add both VIEW_COL1 and 
VIEW_COL2 to the pk
+               conn.createStatement().execute("ALTER TABLE TABLEWITHVIEW ADD 
VIEW_COL2 VARCHAR(256) PRIMARY KEY");
+               fail();
+            }
+            catch (SQLException e) {
+               assertEquals("Unexpected exception", 
CANNOT_MUTATE_TABLE.getErrorCode(), e.getErrorCode());
+            }
+            
+            try {
+               // should fail because there we have to add both VIEW_COL1 and 
VIEW_COL2  to the pk 
+               conn.createStatement().execute("ALTER TABLE TABLEWITHVIEW ADD 
VIEW_COL1 DECIMAL(10,2) PRIMARY KEY");
+               fail();
+            }
+            catch (SQLException e) {
+               assertEquals("Unexpected exception", 
CANNOT_MUTATE_TABLE.getErrorCode(), e.getErrorCode());
+            }
+            
+            try {
+               // should fail because there we have to add both VIEW_COL1 and 
VIEW_COL2 to the pk
+               conn.createStatement().execute("ALTER TABLE TABLEWITHVIEW ADD 
VIEW_COL1 DECIMAL(10,2), VIEW_COL2 VARCHAR(256) PRIMARY KEY");
+               fail();
+            }
+            catch (SQLException e) {
+               assertEquals("Unexpected exception", 
CANNOT_MUTATE_TABLE.getErrorCode(), e.getErrorCode());
+            }
+            
+            try {
+               // should fail because there we have to add both VIEW_COL1 and 
VIEW_COL2  to the pk 
+               conn.createStatement().execute("ALTER TABLE TABLEWITHVIEW ADD 
VIEW_COL1 DECIMAL(10,2) PRIMARY KEY, VIEW_COL2 VARCHAR(256)");
+               fail();
+            }
+            catch (SQLException e) {
+               assertEquals("Unexpected exception", 
CANNOT_MUTATE_TABLE.getErrorCode(), e.getErrorCode());
+            }
+            
+            try {
+               // should fail because there we have to add both VIEW_COL1 and 
VIEW_COL2 to the pk in the right order
+               conn.createStatement().execute("ALTER TABLE TABLEWITHVIEW ADD 
VIEW_COL2 VARCHAR(256) PRIMARY KEY, VIEW_COL1 DECIMAL(10,2) PRIMARY KEY");
+               fail();
+            }
+            catch (SQLException e) {
+               assertEquals("Unexpected exception", 
CANNOT_MUTATE_TABLE.getErrorCode(), e.getErrorCode());
+            }
+            
+            try {
+               // should fail because there we have to add both VIEW_COL1 and 
VIEW_COL2 with the right sort order
+               conn.createStatement().execute("ALTER TABLE TABLEWITHVIEW ADD 
VIEW_COL1 DECIMAL(10,2) PRIMARY KEY DESC, VIEW_COL2 VARCHAR(256) PRIMARY KEY");
                fail();
             }
             catch (SQLException e) {
@@ -2166,33 +2216,194 @@ public class AlterTableIT extends 
BaseOwnClusterHBaseManagedTimeIT {
             }
             
             // add the pk column of the view to the base table
-            conn.createStatement().execute("ALTER TABLE TABLEWITHVIEW ADD 
VIEW_COL1 DECIMAL(10,2) PRIMARY KEY");
-            assertTableDefinition(conn, "TABLEWITHVIEW", PTableType.TABLE, 
null, 1, 4, -1, "ID", "COL1", "COL2", "VIEW_COL1");
-            assertTableDefinition(conn, "VIEWOFTABLE", PTableType.VIEW, 
"TABLEWITHVIEW", 1, 5, 4, "ID", "COL1", "COL2", "VIEW_COL1", "VIEW_COL2");
+            conn.createStatement().execute("ALTER TABLE TABLEWITHVIEW ADD 
VIEW_COL1 DECIMAL(10,2) PRIMARY KEY, VIEW_COL2 VARCHAR(256) PRIMARY KEY");
+            assertTableDefinition(conn, "TABLEWITHVIEW", PTableType.TABLE, 
null, 1, 5, QueryConstants.BASE_TABLE_BASE_COLUMN_COUNT, "ID", "COL1", "COL2", 
"VIEW_COL1", "VIEW_COL2");
+            assertTableDefinition(conn, "VIEWOFTABLE", PTableType.VIEW, 
"TABLEWITHVIEW", 1, 5, 5, "ID", "COL1", "COL2", "VIEW_COL1", "VIEW_COL2");
             
             // query table
             ResultSet rs = stmt.executeQuery("SELECT * FROM TABLEWITHVIEW");
             assertTrue(rs.next());
-            assertEquals(rs.getString(1), "view1");
-            assertEquals(rs.getInt(2), 12);
-            assertEquals(rs.getInt(3), 13);
-            assertEquals(rs.getBigDecimal(4).intValue(), 14);
+            assertEquals("view1", rs.getString("ID"));
+            assertEquals(12, rs.getInt("COL1"));
+            assertEquals(13, rs.getInt("COL2"));
+            assertEquals(14, rs.getInt("VIEW_COL1"));
+            assertEquals("view5", rs.getString("VIEW_COL2"));
             assertFalse(rs.next());
 
             // query view
             rs = stmt.executeQuery("SELECT * FROM VIEWOFTABLE");
             assertTrue(rs.next());
-            assertEquals(rs.getString(1), "view1");
-            assertEquals(rs.getInt(2), 12);
-            assertEquals(rs.getInt(3), 13);
-            assertEquals(rs.getInt(4), 14);
-            assertEquals(rs.getString(5),"view5");
+            assertEquals("view1", rs.getString("ID"));
+            assertEquals(12, rs.getInt("COL1"));
+            assertEquals(13, rs.getInt("COL2"));
+            assertEquals(14, rs.getInt("VIEW_COL1"));
+            assertEquals("view5", rs.getString("VIEW_COL2"));
             assertFalse(rs.next());
         } finally {
             conn.close();
         }
     }
+    
+    @Test
+    public void testAddExistingViewPkColumnToBaseTableWithMultipleViews() 
throws Exception {
+        Connection conn = DriverManager.getConnection(getUrl());
+        try {       
+            conn.createStatement().execute("CREATE TABLE IF NOT EXISTS 
TABLEWITHVIEW ("
+                    + " ID char(10) NOT NULL,"
+                    + " COL1 integer NOT NULL,"
+                    + " COL2 integer NOT NULL,"
+                    + " CONSTRAINT NAME_PK PRIMARY KEY (ID, COL1, COL2)"
+                    + " )");
+            assertTableDefinition(conn, "TABLEWITHVIEW", PTableType.TABLE, 
null, 0, 3, QueryConstants.BASE_TABLE_BASE_COLUMN_COUNT, "ID", "COL1", "COL2");
+            
+            conn.createStatement().execute("CREATE VIEW VIEWOFTABLE1 ( 
VIEW_COL1 DECIMAL(10,2), VIEW_COL2 VARCHAR(256) CONSTRAINT pk PRIMARY KEY 
(VIEW_COL1, VIEW_COL2)) AS SELECT * FROM TABLEWITHVIEW");
+            assertTableDefinition(conn, "VIEWOFTABLE1", PTableType.VIEW, 
"TABLEWITHVIEW", 0, 5, 3, "ID", "COL1", "COL2", "VIEW_COL1", "VIEW_COL2");
+            
+            conn.createStatement().execute("CREATE VIEW VIEWOFTABLE2 ( 
VIEW_COL3 VARCHAR(256), VIEW_COL4 DECIMAL(10,2) CONSTRAINT pk PRIMARY KEY 
(VIEW_COL3, VIEW_COL4)) AS SELECT * FROM TABLEWITHVIEW");
+            assertTableDefinition(conn, "VIEWOFTABLE2", PTableType.VIEW, 
"TABLEWITHVIEW", 0, 5, 3, "ID", "COL1", "COL2", "VIEW_COL3", "VIEW_COL4");
+            
+            try {
+               // should fail because there are two view with different pk 
columns
+               conn.createStatement().execute("ALTER TABLE TABLEWITHVIEW ADD 
VIEW_COL1 DECIMAL PRIMARY KEY, VIEW_COL2 VARCHAR PRIMARY KEY");
+               fail();
+            }
+            catch (SQLException e) {
+               assertEquals("Unexpected exception", 
CANNOT_MUTATE_TABLE.getErrorCode(), e.getErrorCode());
+            }
+            
+            try {
+               // should fail because there are two view with different pk 
columns
+               conn.createStatement().execute("ALTER TABLE TABLEWITHVIEW ADD 
VIEW_COL3 VARCHAR PRIMARY KEY, VIEW_COL4 DECIMAL PRIMARY KEY");
+               fail();
+            }
+            catch (SQLException e) {
+               assertEquals("Unexpected exception", 
CANNOT_MUTATE_TABLE.getErrorCode(), e.getErrorCode());
+            }
+            
+            try {
+               // should fail because slot positions of pks are different
+               conn.createStatement().execute("ALTER TABLE TABLEWITHVIEW ADD 
VIEW_COL1 DECIMAL PRIMARY KEY, VIEW_COL2 VARCHAR PRIMARY KEY, VIEW_COL3 VARCHAR 
PRIMARY KEY, VIEW_COL4 DECIMAL PRIMARY KEY");
+               fail();
+            }
+            catch (SQLException e) {
+               assertEquals("Unexpected exception", 
CANNOT_MUTATE_TABLE.getErrorCode(), e.getErrorCode());
+            }
+            
+            try {
+               // should fail because slot positions of pks are different
+               conn.createStatement().execute("ALTER TABLE TABLEWITHVIEW ADD 
VIEW_COL3 VARCHAR PRIMARY KEY, VIEW_COL4 DECIMAL PRIMARY KEY, VIEW_COL1 DECIMAL 
PRIMARY KEY, VIEW_COL2 VARCHAR PRIMARY KEY");
+               fail();
+            }
+            catch (SQLException e) {
+               assertEquals("Unexpected exception", 
CANNOT_MUTATE_TABLE.getErrorCode(), e.getErrorCode());
+            }
+        } finally {
+            conn.close();
+        }
+    }
+    
+    @Test
+    public void 
testAddExistingViewPkColumnToBaseTableWithMultipleViewsHavingSamePks() throws 
Exception {
+        Connection conn = DriverManager.getConnection(getUrl());
+        try {       
+            conn.createStatement().execute("CREATE TABLE IF NOT EXISTS 
TABLEWITHVIEW ("
+                    + " ID char(10) NOT NULL,"
+                    + " COL1 integer NOT NULL,"
+                    + " COL2 integer NOT NULL,"
+                    + " CONSTRAINT NAME_PK PRIMARY KEY (ID, COL1, COL2)"
+                    + " )");
+            assertTableDefinition(conn, "TABLEWITHVIEW", PTableType.TABLE, 
null, 0, 3, QueryConstants.BASE_TABLE_BASE_COLUMN_COUNT, "ID", "COL1", "COL2");
+            
+            conn.createStatement().execute("CREATE VIEW VIEWOFTABLE1 ( 
VIEW_COL1 DECIMAL(10,2), VIEW_COL2 VARCHAR(256) CONSTRAINT pk PRIMARY KEY 
(VIEW_COL1, VIEW_COL2)) AS SELECT * FROM TABLEWITHVIEW");
+            assertTableDefinition(conn, "VIEWOFTABLE1", PTableType.VIEW, 
"TABLEWITHVIEW", 0, 5, 3, "ID", "COL1", "COL2", "VIEW_COL1", "VIEW_COL2");
+            
+            conn.createStatement().execute("CREATE VIEW VIEWOFTABLE2 ( 
VIEW_COL1 DECIMAL(10,2), VIEW_COL2 VARCHAR(256) CONSTRAINT pk PRIMARY KEY 
(VIEW_COL1, VIEW_COL2)) AS SELECT * FROM TABLEWITHVIEW");
+            assertTableDefinition(conn, "VIEWOFTABLE2", PTableType.VIEW, 
"TABLEWITHVIEW", 0, 5, 3, "ID", "COL1", "COL2", "VIEW_COL1", "VIEW_COL2");
+            
+            // upsert single row into both view
+            String dml = "UPSERT INTO VIEWOFTABLE1 VALUES(?,?,?,?,?)";
+            PreparedStatement stmt = conn.prepareStatement(dml);
+            stmt.setString(1, "view1");
+            stmt.setInt(2, 12);
+            stmt.setInt(3, 13);
+            stmt.setInt(4, 14);
+            stmt.setString(5, "view5");
+            stmt.execute();
+            conn.commit();
+            dml = "UPSERT INTO VIEWOFTABLE2 VALUES(?,?,?,?,?)";
+            stmt = conn.prepareStatement(dml);
+            stmt.setString(1, "view1");
+            stmt.setInt(2, 12);
+            stmt.setInt(3, 13);
+            stmt.setInt(4, 14);
+            stmt.setString(5, "view5");
+            stmt.execute();
+            conn.commit();
+            
+            try {
+               // should fail because the view have two extra columns in their 
pk
+               conn.createStatement().execute("ALTER TABLE TABLEWITHVIEW ADD 
VIEW_COL1 DECIMAL(10,2) PRIMARY KEY");
+               fail();
+            }
+            catch (SQLException e) {
+               assertEquals("Unexpected exception", 
CANNOT_MUTATE_TABLE.getErrorCode(), e.getErrorCode());
+            }
+            
+            try {
+               // should fail because the view have two extra columns in their 
pk
+               conn.createStatement().execute("ALTER TABLE TABLEWITHVIEW ADD 
VIEW_COL2 VARCHAR(256) PRIMARY KEY");
+               fail();
+            }
+            catch (SQLException e) {
+               assertEquals("Unexpected exception", 
CANNOT_MUTATE_TABLE.getErrorCode(), e.getErrorCode());
+            }
+            
+            try {
+               // should fail because slot positions of pks are different
+               conn.createStatement().execute("ALTER TABLE TABLEWITHVIEW ADD 
VIEW_COL2 DECIMAL(10,2) PRIMARY KEY, VIEW_COL1 VARCHAR(256) PRIMARY KEY");
+               fail();
+            }
+            catch (SQLException e) {
+               assertEquals("Unexpected exception", 
CANNOT_MUTATE_TABLE.getErrorCode(), e.getErrorCode());
+            }
+            
+            conn.createStatement().execute("ALTER TABLE TABLEWITHVIEW ADD 
VIEW_COL1 DECIMAL(10,2) PRIMARY KEY, VIEW_COL2 VARCHAR(256) PRIMARY KEY");
+            assertTableDefinition(conn, "TABLEWITHVIEW", PTableType.TABLE, 
null, 1, 5, QueryConstants.BASE_TABLE_BASE_COLUMN_COUNT, "ID", "COL1", "COL2", 
"VIEW_COL1", "VIEW_COL2");
+            assertTableDefinition(conn, "VIEWOFTABLE1", PTableType.VIEW, 
"TABLEWITHVIEW", 1, 5, 5, "ID", "COL1", "COL2", "VIEW_COL1", "VIEW_COL2");
+            assertTableDefinition(conn, "VIEWOFTABLE2", PTableType.VIEW, 
"TABLEWITHVIEW", 1, 5, 5, "ID", "COL1", "COL2", "VIEW_COL1", "VIEW_COL2");
+            
+            // query table
+            ResultSet rs = stmt.executeQuery("SELECT * FROM TABLEWITHVIEW");
+            assertTrue(rs.next());
+            assertEquals("view1", rs.getString("ID"));
+            assertEquals(12, rs.getInt("COL1"));
+            assertEquals(13, rs.getInt("COL2"));
+            assertEquals(14, rs.getInt("VIEW_COL1"));
+            assertEquals("view5", rs.getString("VIEW_COL2"));
+            assertFalse(rs.next());
 
+            // query both views
+            rs = stmt.executeQuery("SELECT * FROM VIEWOFTABLE1");
+            assertTrue(rs.next());
+            assertEquals("view1", rs.getString("ID"));
+            assertEquals(12, rs.getInt("COL1"));
+            assertEquals(13, rs.getInt("COL2"));
+            assertEquals(14, rs.getInt("VIEW_COL1"));
+            assertEquals("view5", rs.getString("VIEW_COL2"));
+            assertFalse(rs.next());
+            rs = stmt.executeQuery("SELECT * FROM VIEWOFTABLE2");
+            assertTrue(rs.next());
+            assertEquals("view1", rs.getString("ID"));
+            assertEquals(12, rs.getInt("COL1"));
+            assertEquals(13, rs.getInt("COL2"));
+            assertEquals(14, rs.getInt("VIEW_COL1"));
+            assertEquals("view5", rs.getString("VIEW_COL2"));
+            assertFalse(rs.next());
+        } finally {
+            conn.close();
+        }
+    }
+    
     private void assertTableDefinition(Connection conn, String tableName, 
PTableType tableType, String parentTableName, int sequenceNumber, int 
columnCount, int baseColumnCount, String... columnName) throws Exception {
         PreparedStatement p = conn.prepareStatement("SELECT * FROM 
SYSTEM.CATALOG WHERE TABLE_NAME=? AND TABLE_TYPE=?");
         p.setString(1, tableName);
@@ -2234,10 +2445,8 @@ public class AlterTableIT extends 
BaseOwnClusterHBaseManagedTimeIT {
                                 assertEquals(parentTableName, 
parentTableColumnValue);
                                 assertEquals(tableName, viewColumnValue);
                             } 
-                            else if 
(parentTableColumnsMetadata.getColumnName(columnIndex).equals(PhoenixDatabaseMetaData.ORDINAL_POSITION)
 && parentTableColumnsRs.getString("COLUMN_FAMILY")!=null) {
-                               // its ok if the ordinal positions don't match 
for non-pk columns 
-                            }
-                            else {
+                            // its ok if the ordinal positions don't match for 
non-pk columns
+                            else if 
(!(parentTableColumnsMetadata.getColumnName(columnIndex).equals(PhoenixDatabaseMetaData.ORDINAL_POSITION)
 && 
parentTableColumnsRs.getString(PhoenixDatabaseMetaData.COLUMN_FAMILY)!=null)) {
                                 
fail(parentTableColumnsMetadata.getColumnName(columnIndex) + " of base table " 
+ parentTableColumnValue + " does not match view "+viewColumnValue) ;
                             }
                         }

http://git-wip-us.apache.org/repos/asf/phoenix/blob/27d78b65/phoenix-core/src/it/java/org/apache/phoenix/end2end/ViewIT.java
----------------------------------------------------------------------
diff --git a/phoenix-core/src/it/java/org/apache/phoenix/end2end/ViewIT.java 
b/phoenix-core/src/it/java/org/apache/phoenix/end2end/ViewIT.java
index 1d8af35..3254c2e 100644
--- a/phoenix-core/src/it/java/org/apache/phoenix/end2end/ViewIT.java
+++ b/phoenix-core/src/it/java/org/apache/phoenix/end2end/ViewIT.java
@@ -537,6 +537,12 @@ public class ViewIT extends BaseViewIT {
         } catch (SQLException e) {
             assertEquals(CANNOT_MODIFY_VIEW_PK.getErrorCode(), 
e.getErrorCode());
         }
+        ddl = "CREATE VIEW v2 (k3 VARCHAR PRIMARY KEY)  AS SELECT * FROM tp 
WHERE v1 = 1.0";
+        try {
+               conn.createStatement().execute(ddl);
+        } catch (SQLException e) {
+            assertEquals(CANNOT_MODIFY_VIEW_PK.getErrorCode(), 
e.getErrorCode());
+        }
     }
     
     @Test(expected=ColumnAlreadyExistsException.class)

http://git-wip-us.apache.org/repos/asf/phoenix/blob/27d78b65/phoenix-core/src/main/java/org/apache/phoenix/coprocessor/MetaDataEndpointImpl.java
----------------------------------------------------------------------
diff --git 
a/phoenix-core/src/main/java/org/apache/phoenix/coprocessor/MetaDataEndpointImpl.java
 
b/phoenix-core/src/main/java/org/apache/phoenix/coprocessor/MetaDataEndpointImpl.java
index c5e2335..f286bdc 100644
--- 
a/phoenix-core/src/main/java/org/apache/phoenix/coprocessor/MetaDataEndpointImpl.java
+++ 
b/phoenix-core/src/main/java/org/apache/phoenix/coprocessor/MetaDataEndpointImpl.java
@@ -186,6 +186,7 @@ import org.apache.phoenix.util.ServerUtil;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import com.google.common.base.Objects;
 import com.google.common.cache.Cache;
 import com.google.common.collect.Lists;
 import com.google.protobuf.ByteString;
@@ -1588,19 +1589,14 @@ public class MetaDataEndpointImpl extends 
MetaDataProtocol implements Coprocesso
         }
     }
     
-    private boolean isValidLengthOrScale(Integer viewColVal, Cell cell) {
-       Integer baseColValue = cell.getValueArray()==null ? null :
-                       
PInteger.INSTANCE.getCodec().decodeInt(cell.getValueArray(), 
cell.getValueOffset(), SortOrder.getDefault());
-       if ( (baseColValue!=null && viewColVal!=null && 
baseColValue<viewColVal) || // if the base column has a scale/length less then 
the view column
-                       (viewColVal==null && baseColValue!=null)) {  // or if 
the view column has no scale/length, but the base column has a scale/length
-               return false;
-       }
-       return true;
+    private boolean isvalidAttribute(Object obj1, Object obj2) {
+       return (obj1!=null && obj1.equals(obj2)) || (obj1==null && obj2==null); 
     }
 
-    private MetaDataMutationResult addRowsToChildViews(List<Mutation> 
tableMetadata, List<Mutation> mutationsForAddingColumnsToViews, byte[] 
schemaName, byte[] tableName,
+    private MetaDataMutationResult addRowsToChildViews(PTable table, 
List<Mutation> tableMetadata, List<Mutation> mutationsForAddingColumnsToViews, 
byte[] schemaName, byte[] tableName,
             List<ImmutableBytesPtr> invalidateList, long clientTimeStamp, 
TableViewFinderResult childViewsResult,
             HRegion region, List<RowLock> locks) throws IOException, 
SQLException {
+       
         for (Result viewResult : childViewsResult.getResults()) {
             byte[][] rowViewKeyMetaData = new byte[3][];
             getVarChars(viewResult.getRow(), 3, rowViewKeyMetaData);
@@ -1620,6 +1616,8 @@ public class MetaDataEndpointImpl extends 
MetaDataProtocol implements Coprocesso
             int numColsAddedToView = 0;
             short deltaNumPkColsSoFar = 0;
             PColumn existingViewColumn = null;
+            List<PColumn> viewPkColumns = 
Lists.newArrayList(view.getPKColumns());
+            boolean addingPKColumn=false;
             for (Mutation m : tableMetadata) {
                 byte[][] rkmd = new byte[5][];
                 int pkCount = getVarChars(m.getRow(), rkmd);
@@ -1632,7 +1630,10 @@ public class MetaDataEndpointImpl extends 
MetaDataProtocol implements Coprocesso
                         existingViewColumn = rkmd[FAMILY_NAME_INDEX] == null 
                             ? view.getPKColumn(columnName) 
                             : 
view.getColumnFamily(rkmd[FAMILY_NAME_INDEX]).getColumn(columnName);
-                    } catch (ColumnNotFoundException e) {
+                    } 
+                       catch (ColumnFamilyNotFoundException e) {
+                    }
+                       catch (ColumnNotFoundException e) {
                     } // Ignore - means column family or column name don't 
exist
                        
                     Put p = (Put)m;
@@ -1660,6 +1661,7 @@ public class MetaDataEndpointImpl extends 
MetaDataProtocol implements Coprocesso
                                         return new 
MetaDataMutationResult(MutationCode.UNALLOWED_TABLE_MUTATION, 
EnvironmentEdgeManager.currentTimeMillis(), null);
                                }
                                }
+                               // if adding a pk column to the base table
                                if (rkmd[FAMILY_NAME_INDEX] == null && 
rkmd[COLUMN_NAME_INDEX] != null) {
                                List<Cell> keySeqCells = 
viewColumnDefinitionPut.get(
                                        
PhoenixDatabaseMetaData.TABLE_FAMILY_BYTES,
@@ -1672,35 +1674,45 @@ public class MetaDataEndpointImpl extends 
MetaDataProtocol implements Coprocesso
                                                return new 
MetaDataMutationResult(MutationCode.UNALLOWED_TABLE_MUTATION, 
EnvironmentEdgeManager.currentTimeMillis(), null);
                                        }
                                }
+                               List<Cell> sortOrders = 
viewColumnDefinitionPut.get(PhoenixDatabaseMetaData.TABLE_FAMILY_BYTES,
+                                    PhoenixDatabaseMetaData.SORT_ORDER_BYTES);
+                               SortOrder sortOrder = SortOrder.getDefault();
+                            if (sortOrders != null && sortOrders.size() > 0) {
+                               Cell cell = sortOrders.get(0);
+                               int sortOrderInt = 
PInteger.INSTANCE.getCodec().decodeInt(cell.getValueArray(), 
cell.getValueOffset(), SortOrder.getDefault());
+                               sortOrder = 
SortOrder.fromSystemValue(sortOrderInt);                                    
+                            }
+                            if 
(!Objects.equal(sortOrder,existingViewColumn.getSortOrder())) {
+                                       return new 
MetaDataMutationResult(MutationCode.UNALLOWED_TABLE_MUTATION, 
EnvironmentEdgeManager.currentTimeMillis(), null);
+                               }
+                               viewPkColumns.remove(existingViewColumn);
                         }
                                // if there is an existing view column that 
matches the column being added to the base table and if the column being added 
has a null
                        // scale or maxLength, we need to explicitly do a put 
to set the scale or maxLength to null (in case the view column has the scale or 
                        // max length set)
+                               Integer maxLength = null;
                                List<Cell> columnSizes = 
viewColumnDefinitionPut.get(
                                 PhoenixDatabaseMetaData.TABLE_FAMILY_BYTES,
                                 PhoenixDatabaseMetaData.COLUMN_SIZE_BYTES);
                         if (columnSizes != null && columnSizes.size() > 0) {
                             Cell cell = columnSizes.get(0);
-                            if 
(!isValidLengthOrScale(existingViewColumn.getMaxLength(),cell)) {
-                               return new 
MetaDataMutationResult(MutationCode.UNALLOWED_TABLE_MUTATION, 
EnvironmentEdgeManager.currentTimeMillis(), null); 
-                            }
+                            maxLength = cell.getValueArray()==null ? null :
+                               
PInteger.INSTANCE.getCodec().decodeInt(cell.getValueArray(), 
cell.getValueOffset(), SortOrder.getDefault());
                         }
-                        else if (existingViewColumn.getMaxLength()!=null) {
-                               
viewColumnDefinitionPut.add(PhoenixDatabaseMetaData.TABLE_FAMILY_BYTES,
-                                    PhoenixDatabaseMetaData.COLUMN_SIZE_BYTES, 
null);
+                        if 
(!Objects.equal(maxLength,existingViewColumn.getMaxLength())) {
+                               return new 
MetaDataMutationResult(MutationCode.UNALLOWED_TABLE_MUTATION, 
EnvironmentEdgeManager.currentTimeMillis(), null); 
                         }
+                        Integer scale = null;
                         List<Cell> decimalDigits = viewColumnDefinitionPut.get(
                                 PhoenixDatabaseMetaData.TABLE_FAMILY_BYTES,
                                 PhoenixDatabaseMetaData.DECIMAL_DIGITS_BYTES);
                         if (decimalDigits != null && decimalDigits.size() > 0) 
{
                             Cell cell = decimalDigits.get(0);
-                            if 
(!isValidLengthOrScale(existingViewColumn.getScale(),cell)) {
-                               return new 
MetaDataMutationResult(MutationCode.UNALLOWED_TABLE_MUTATION, 
EnvironmentEdgeManager.currentTimeMillis(), null); 
-                            }
+                            scale = cell.getValueArray()==null ? null :
+                               
PInteger.INSTANCE.getCodec().decodeInt(cell.getValueArray(), 
cell.getValueOffset(), SortOrder.getDefault());
                         }
-                        else if (existingViewColumn.getScale()!=null) {
-                               
viewColumnDefinitionPut.add(PhoenixDatabaseMetaData.TABLE_FAMILY_BYTES,
-                                    
PhoenixDatabaseMetaData.DECIMAL_DIGITS_BYTES, null);
+                        if 
(!Objects.equal(scale,existingViewColumn.getScale())) {     
+                               return new 
MetaDataMutationResult(MutationCode.UNALLOWED_TABLE_MUTATION, 
EnvironmentEdgeManager.currentTimeMillis(), null); 
                         }
                        }
                        else {
@@ -1716,6 +1728,7 @@ public class MetaDataEndpointImpl extends 
MetaDataProtocol implements Coprocesso
                          * If adding a pk column to the base table (and hence 
the view), see if there are any indexes on
                          * the view. If yes, then generate puts for the index 
header row and column rows.
                          */
+                       addingPKColumn=true;
                         deltaNumPkColsSoFar++;
                         for (PTable index : view.getIndexes()) {
                             int oldNumberOfColsInIndex = 
index.getColumns().size();
@@ -1804,6 +1817,12 @@ public class MetaDataEndpointImpl extends 
MetaDataProtocol implements Coprocesso
                     }
                 }
             }
+            // allow adding a pk columns to base table :
+            // 1. if all the view pk columns are exactly the same as the base 
table pk columns
+            // 2. if we are adding all the existing view pk columns to the 
base table 
+            if (addingPKColumn && !viewPkColumns.equals(table.getPKColumns())) 
{
+               return new 
MetaDataMutationResult(MutationCode.UNALLOWED_TABLE_MUTATION, 
EnvironmentEdgeManager.currentTimeMillis(), null);
+            }
             if (deltaNumPkColsSoFar > 0) {
                 for (PTable index : view.getIndexes()) {
                     byte[] indexHeaderRowKey = getViewIndexHeaderRowKey(index);
@@ -1916,7 +1935,7 @@ public class MetaDataEndpointImpl extends 
MetaDataProtocol implements Coprocesso
                                         
EnvironmentEdgeManager.currentTimeMillis(), null);
                             } else {
                                 mutationsForAddingColumnsToViews = new 
ArrayList<>(childViewsResult.getResults().size() * tableMetaData.size());
-                                MetaDataMutationResult mutationResult = 
addRowsToChildViews(tableMetaData, mutationsForAddingColumnsToViews, 
schemaName, tableName, invalidateList, clientTimeStamp,
+                                MetaDataMutationResult mutationResult = 
addRowsToChildViews(table, tableMetaData, mutationsForAddingColumnsToViews, 
schemaName, tableName, invalidateList, clientTimeStamp,
                                         childViewsResult, region, locks);
                                 // return if we were not able to add the 
column successfully
                                 if (mutationResult!=null)

http://git-wip-us.apache.org/repos/asf/phoenix/blob/27d78b65/phoenix-core/src/main/java/org/apache/phoenix/schema/MetaDataClient.java
----------------------------------------------------------------------
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 56c1aa2..9ad52a5 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
@@ -2505,7 +2505,7 @@ public class MetaDataClient {
                 }
                 long seqNum = table.getSequenceNumber();
                 if (changingPhoenixTableProperty || columnDefs.size() > 0) { 
-                    seqNum = incrementTableSeqNum(table, 
statement.getTableType(), 1, isImmutableRows, disableWAL, multiTenant, 
storeNulls);
+                    seqNum = incrementTableSeqNum(table, 
statement.getTableType(), columnDefs.size(), isImmutableRows, disableWAL, 
multiTenant, storeNulls);
                     
tableMetaData.addAll(connection.getMutationState().toMutations().next().getSecond());
                     connection.rollback();
                 }
@@ -2977,14 +2977,11 @@ public class MetaDataClient {
     
     private boolean isLastPKVariableLength(PTable table) {
         List<PColumn> pkColumns = table.getPKColumns();
-        if (pkColumns.size() < 1) {
-            return false;
-        } else {
-            return 
!pkColumns.get(pkColumns.size()-1).getDataType().isFixedWidth();
-        }
+        return !pkColumns.get(pkColumns.size()-1).getDataType().isFixedWidth();
     }
     
     private PTable getParentOfView(PTable view) throws SQLException {
+       //TODO just use view.getParentName().getString() after implementing 
https://issues.apache.org/jira/browse/PHOENIX-2114 
         SelectStatement select = new 
SQLParser(view.getViewStatement()).parseQuery();
         String parentName = 
SchemaUtil.normalizeFullTableName(select.getFrom().toString().trim());
         return connection.getMetaDataCache().getTable(new 
PTableKey(view.getTenantId(), parentName));

Reply via email to