phoenix-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From jamestay...@apache.org
Subject git commit: PHOENIX-1171 Dropping the index is not verifying the associated table
Date Fri, 15 Aug 2014 03:37:14 GMT
Repository: phoenix
Updated Branches:
  refs/heads/master ca39e2d1b -> c85d4c6ad


PHOENIX-1171 Dropping the index is not verifying the associated table

Conflicts:
	phoenix-core/src/main/java/org/apache/phoenix/coprocessor/MetaDataEndpointImpl.java


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

Branch: refs/heads/master
Commit: c85d4c6ad145babcd5eca2fde1dc632071105b77
Parents: ca39e2d
Author: James Taylor <jtaylor@salesforce.com>
Authored: Thu Aug 14 12:58:12 2014 -0700
Committer: James Taylor <jtaylor@salesforce.com>
Committed: Thu Aug 14 20:38:52 2014 -0700

----------------------------------------------------------------------
 .../java/org/apache/phoenix/end2end/ViewIT.java | 37 ++++++++++++++++++++
 .../phoenix/end2end/index/IndexMetadataIT.java  | 12 ++++++-
 .../coprocessor/MetaDataEndpointImpl.java       | 22 ++++++------
 .../org/apache/phoenix/util/MetaDataUtil.java   |  4 ++-
 .../phoenix/compile/ViewCompilerTest.java       | 12 +++----
 5 files changed, 69 insertions(+), 18 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/phoenix/blob/c85d4c6a/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 1d022e5..d79535a 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
@@ -29,6 +29,7 @@ import java.sql.SQLException;
 
 import org.apache.phoenix.exception.SQLExceptionCode;
 import org.apache.phoenix.schema.ReadOnlyTableException;
+import org.apache.phoenix.schema.TableNotFoundException;
 import org.junit.Test;
 import org.junit.experimental.categories.Category;
 
@@ -233,4 +234,40 @@ public class ViewIT extends BaseViewIT {
         }
         assertEquals(4, count);
     }
+    
+    @Test
+    public void testViewAndTableInDifferentSchemas() throws Exception {
+        Connection conn = DriverManager.getConnection(getUrl());
+        String ddl = "CREATE TABLE s1.t (k INTEGER NOT NULL PRIMARY KEY, v1 DATE)";
+        conn.createStatement().execute(ddl);
+        ddl = "CREATE VIEW s2.v1 (v2 VARCHAR) AS SELECT * FROM s1.t WHERE k > 5";
+        conn.createStatement().execute(ddl);
+        ddl = "CREATE VIEW v2 (v2 VARCHAR) AS SELECT * FROM s1.t WHERE k > 5";
+        conn.createStatement().execute(ddl);
+        ddl = "DROP VIEW v1";
+        try {
+            conn.createStatement().execute(ddl);
+            fail();
+        } catch (TableNotFoundException ignore) {
+        }
+        ddl = "DROP VIEW s2.v1";
+        conn.createStatement().execute(ddl);
+        ddl = "DROP VIEW s2.v2";
+        try {
+            conn.createStatement().execute(ddl);
+            fail();
+        } catch (TableNotFoundException ignore) {
+        }
+        ddl = "DROP TABLE s1.t";
+        try {
+            conn.createStatement().execute(ddl);
+            fail();
+        } catch (SQLException e) {
+            assertEquals(SQLExceptionCode.CANNOT_MUTATE_TABLE.getErrorCode(), e.getErrorCode());
+        }
+        ddl = "DROP VIEW v2";
+        conn.createStatement().execute(ddl);
+        ddl = "DROP TABLE s1.t";
+        conn.createStatement().execute(ddl);
+    }
 }

http://git-wip-us.apache.org/repos/asf/phoenix/blob/c85d4c6a/phoenix-core/src/it/java/org/apache/phoenix/end2end/index/IndexMetadataIT.java
----------------------------------------------------------------------
diff --git a/phoenix-core/src/it/java/org/apache/phoenix/end2end/index/IndexMetadataIT.java
b/phoenix-core/src/it/java/org/apache/phoenix/end2end/index/IndexMetadataIT.java
index 35232b5..2547844 100644
--- a/phoenix-core/src/it/java/org/apache/phoenix/end2end/index/IndexMetadataIT.java
+++ b/phoenix-core/src/it/java/org/apache/phoenix/end2end/index/IndexMetadataIT.java
@@ -45,6 +45,7 @@ import org.apache.phoenix.schema.AmbiguousColumnException;
 import org.apache.phoenix.schema.PIndexState;
 import org.apache.phoenix.schema.PTableKey;
 import org.apache.phoenix.schema.PTableType;
+import org.apache.phoenix.schema.TableNotFoundException;
 import org.apache.phoenix.util.PropertiesUtil;
 import org.apache.phoenix.util.SchemaUtil;
 import org.apache.phoenix.util.StringUtil;
@@ -115,7 +116,7 @@ public class IndexMetadataIT extends BaseHBaseManagedTimeIT {
     }
     
     @Test
-    public void testIndexCreation() throws Exception {
+    public void testIndexCreateDrop() throws Exception {
         Properties props = PropertiesUtil.deepCopy(TEST_PROPERTIES);
         Connection conn = DriverManager.getConnection(getUrl(), props);
         conn.setAutoCommit(false);
@@ -259,6 +260,15 @@ public class IndexMetadataIT extends BaseHBaseManagedTimeIT {
             assertIndexInfoMetadata(rs, INDEX_DATA_SCHEMA, MUTABLE_INDEX_DATA_TABLE, "IDX2",
8, "B:INT_COL2", null);
             assertFalse(rs.next());
             
+            // Create another table in the same schema
+            String diffTableNameInSameSchema = INDEX_DATA_SCHEMA + QueryConstants.NAME_SEPARATOR
+ MUTABLE_INDEX_DATA_TABLE + "2";
+            conn.createStatement().execute("CREATE TABLE " + diffTableNameInSameSchema +
"(k INTEGER PRIMARY KEY)");
+            try {
+                conn.createStatement().execute("DROP INDEX IDX1 ON " + diffTableNameInSameSchema);
+                fail("Should have realized index IDX1 is not on the table");
+            } catch (TableNotFoundException ignore) {
+                
+            }
             ddl = "DROP TABLE " + INDEX_DATA_SCHEMA + QueryConstants.NAME_SEPARATOR + MUTABLE_INDEX_DATA_TABLE;
             stmt = conn.prepareStatement(ddl);
             stmt.execute();

http://git-wip-us.apache.org/repos/asf/phoenix/blob/c85d4c6a/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 4066070..b99483b 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
@@ -30,6 +30,7 @@ import static org.apache.phoenix.jdbc.PhoenixDatabaseMetaData.DEFAULT_COLUMN_FAM
 import static org.apache.phoenix.jdbc.PhoenixDatabaseMetaData.DISABLE_WAL_BYTES;
 import static org.apache.phoenix.jdbc.PhoenixDatabaseMetaData.FAMILY_NAME_INDEX;
 import static org.apache.phoenix.jdbc.PhoenixDatabaseMetaData.IMMUTABLE_ROWS_BYTES;
+import static org.apache.phoenix.jdbc.PhoenixDatabaseMetaData.INDEX_DISABLE_TIMESTAMP_BYTES;
 import static org.apache.phoenix.jdbc.PhoenixDatabaseMetaData.INDEX_STATE_BYTES;
 import static org.apache.phoenix.jdbc.PhoenixDatabaseMetaData.INDEX_TYPE_BYTES;
 import static org.apache.phoenix.jdbc.PhoenixDatabaseMetaData.IS_VIEW_REFERENCED_BYTES;
@@ -50,7 +51,6 @@ import static org.apache.phoenix.jdbc.PhoenixDatabaseMetaData.VIEW_CONSTANT_BYTE
 import static org.apache.phoenix.jdbc.PhoenixDatabaseMetaData.VIEW_INDEX_ID_BYTES;
 import static org.apache.phoenix.jdbc.PhoenixDatabaseMetaData.VIEW_STATEMENT_BYTES;
 import static org.apache.phoenix.jdbc.PhoenixDatabaseMetaData.VIEW_TYPE_BYTES;
-import static org.apache.phoenix.jdbc.PhoenixDatabaseMetaData.INDEX_DISABLE_TIMESTAMP_BYTES;
 import static org.apache.phoenix.schema.PTableType.INDEX;
 import static org.apache.phoenix.util.SchemaUtil.getVarCharLength;
 import static org.apache.phoenix.util.SchemaUtil.getVarChars;
@@ -928,7 +928,7 @@ public class MetaDataEndpointImpl extends MetaDataProtocol implements
Coprocesso
                 }
                 List<ImmutableBytesPtr> invalidateList = new ArrayList<ImmutableBytesPtr>();
                 result =
-                        doDropTable(key, tenantIdBytes, schemaName, tableName,
+                        doDropTable(key, tenantIdBytes, schemaName, tableName, parentTableName,
                             PTableType.fromSerializedValue(tableType), tableMetadata,
                             invalidateList, locks, tableNamesToDelete);
                 if (result.getMutationCode() != MutationCode.TABLE_ALREADY_EXISTS) {
@@ -959,7 +959,7 @@ public class MetaDataEndpointImpl extends MetaDataProtocol implements
Coprocesso
     }
 
     private MetaDataMutationResult doDropTable(byte[] key, byte[] tenantId, byte[] schemaName,
-        byte[] tableName, PTableType tableType, List<Mutation> rowsToDelete,
+        byte[] tableName, byte[] parentTableName, PTableType tableType, List<Mutation>
rowsToDelete,
         List<ImmutableBytesPtr> invalidateList, List<RowLock> locks,
         List<byte[]> tableNamesToDelete) throws IOException, SQLException {
         long clientTimeStamp = MetaDataUtil.getClientTimeStamp(rowsToDelete);
@@ -990,6 +990,10 @@ public class MetaDataEndpointImpl extends MetaDataProtocol implements
Coprocesso
             }
             return new MetaDataMutationResult(MutationCode.TABLE_NOT_FOUND, EnvironmentEdgeManager.currentTimeMillis(),
null);
         }
+        // Make sure we're not deleting the "wrong" child
+        if (!Arrays.equals(parentTableName, table.getParentTableName() == null ? null : table.getParentTableName().getBytes()))
{
+            return new MetaDataMutationResult(MutationCode.TABLE_NOT_FOUND, EnvironmentEdgeManager.currentTimeMillis(),
null);
+        }
         // Since we don't allow back in time DDL, we know if we have a table it's the one
         // we want to delete. FIXME: we shouldn't need a scan here, but should be able to
         // use the table to generate the Delete markers.
@@ -1040,7 +1044,7 @@ public class MetaDataEndpointImpl extends MetaDataProtocol implements
Coprocesso
             rowsToDelete.add(delete);
             acquireLock(region, indexKey, locks);
             MetaDataMutationResult result =
-                    doDropTable(indexKey, tenantId, schemaName, indexName, PTableType.INDEX,
+                    doDropTable(indexKey, tenantId, schemaName, indexName, tableName, PTableType.INDEX,
                         rowsToDelete, invalidateList, locks, tableNamesToDelete);
             if (result.getMutationCode() != MutationCode.TABLE_ALREADY_EXISTS) {
                 return result;
@@ -1215,11 +1219,8 @@ public class MetaDataEndpointImpl extends MetaDataProtocol implements
Coprocesso
                             } catch (ColumnNotFoundException e) {
                                 if (addingPKColumn) {
                                     // Add all indexes to invalidate list, as they will all
be
-                                    // adding
-                                    // the same PK column
-                                    // No need to lock them, as we have the parent table
lock at
-                                    // this
-                                    // point
+                                    // adding the same PK column. No need to lock them, as
we
+                                    // have the parent table lock at this point.
                                     for (PTable index : table.getIndexes()) {
                                         invalidateList.add(new ImmutableBytesPtr(SchemaUtil
                                                 .getTableKey(tenantId, index.getSchemaName()
@@ -1315,7 +1316,7 @@ public class MetaDataEndpointImpl extends MetaDataProtocol implements
Coprocesso
                                                 // Drop the link between the data table and
the
                                                 // index table
                                                 additionalTableMetaData.add(new Delete(linkKey,
clientTimeStamp));
-                                                doDropTable(indexKey, tenantId, index.getSchemaName().getBytes(),
index.getTableName().getBytes(),
+                                                doDropTable(indexKey, tenantId, index.getSchemaName().getBytes(),
index.getTableName().getBytes(), tableName,
                                                     index.getType(), additionalTableMetaData,
invalidateList, locks, tableNamesToDelete);
                                                 // TODO: return in result?
                                             } else {
@@ -1384,6 +1385,7 @@ public class MetaDataEndpointImpl extends MetaDataProtocol implements
Coprocesso
         done.run(builder.build());
     }
 
+    @SuppressWarnings("deprecation")
     @Override
     public void updateIndexState(RpcController controller, UpdateIndexStateRequest request,
             RpcCallback<MetaDataResponse> done) {

http://git-wip-us.apache.org/repos/asf/phoenix/blob/c85d4c6a/phoenix-core/src/main/java/org/apache/phoenix/util/MetaDataUtil.java
----------------------------------------------------------------------
diff --git a/phoenix-core/src/main/java/org/apache/phoenix/util/MetaDataUtil.java b/phoenix-core/src/main/java/org/apache/phoenix/util/MetaDataUtil.java
index 215a2b1..545394d 100644
--- a/phoenix-core/src/main/java/org/apache/phoenix/util/MetaDataUtil.java
+++ b/phoenix-core/src/main/java/org/apache/phoenix/util/MetaDataUtil.java
@@ -141,10 +141,12 @@ public class MetaDataUtil {
         }
         byte[][] rowKeyMetaData = new byte[3][];
         getTenantIdAndSchemaAndTableName(tableMetadata, rowKeyMetaData);
+        byte[] schemaName = rowKeyMetaData[PhoenixDatabaseMetaData.SCHEMA_NAME_INDEX];
         byte[] tableName = rowKeyMetaData[PhoenixDatabaseMetaData.TABLE_NAME_INDEX];
         Mutation m = getParentTableHeaderRow(tableMetadata);
         getVarChars(m.getRow(), 3, rowKeyMetaData);
-        if (Bytes.compareTo(tableName, rowKeyMetaData[PhoenixDatabaseMetaData.TABLE_NAME_INDEX])
== 0) {
+        if (   Bytes.compareTo(schemaName, rowKeyMetaData[PhoenixDatabaseMetaData.SCHEMA_NAME_INDEX])
== 0
+            && Bytes.compareTo(tableName, rowKeyMetaData[PhoenixDatabaseMetaData.TABLE_NAME_INDEX])
== 0) {
             return null;
         }
         return rowKeyMetaData[PhoenixDatabaseMetaData.TABLE_NAME_INDEX];

http://git-wip-us.apache.org/repos/asf/phoenix/blob/c85d4c6a/phoenix-core/src/test/java/org/apache/phoenix/compile/ViewCompilerTest.java
----------------------------------------------------------------------
diff --git a/phoenix-core/src/test/java/org/apache/phoenix/compile/ViewCompilerTest.java b/phoenix-core/src/test/java/org/apache/phoenix/compile/ViewCompilerTest.java
index 651d58d..7a0bac6 100644
--- a/phoenix-core/src/test/java/org/apache/phoenix/compile/ViewCompilerTest.java
+++ b/phoenix-core/src/test/java/org/apache/phoenix/compile/ViewCompilerTest.java
@@ -80,25 +80,25 @@ public class ViewCompilerTest extends BaseConnectionlessQueryTest {
     public void testViewInvalidation() throws Exception {
         Properties props = PropertiesUtil.deepCopy(TEST_PROPERTIES);
         PhoenixConnection conn = DriverManager.getConnection(getUrl(), props).unwrap(PhoenixConnection.class);
-        String ct = "CREATE TABLE t (k1 INTEGER NOT NULL, k2 VARCHAR, v VARCHAR, CONSTRAINT
pk PRIMARY KEY (k1,k2))";
+        String ct = "CREATE TABLE s1.t (k1 INTEGER NOT NULL, k2 VARCHAR, v VARCHAR, CONSTRAINT
pk PRIMARY KEY (k1,k2))";
         conn.createStatement().execute(ct);
-        conn.createStatement().execute("CREATE VIEW v3 AS SELECT * FROM t WHERE v = 'bar'");
+        conn.createStatement().execute("CREATE VIEW s2.v3 AS SELECT * FROM s1.t WHERE v =
'bar'");
         
         // TODO: should it be an error to remove columns from a VIEW that we're defined there?
         // TOOD: should we require an ALTER VIEW instead of ALTER TABLE?
-        conn.createStatement().execute("ALTER VIEW v3 DROP COLUMN v");
+        conn.createStatement().execute("ALTER VIEW s2.v3 DROP COLUMN v");
         try {
-            conn.createStatement().executeQuery("SELECT * FROM v3");
+            conn.createStatement().executeQuery("SELECT * FROM s2.v3");
             fail();
         } catch (ColumnNotFoundException e) {
             
         }
         
         // No error, as v still exists in t
-        conn.createStatement().execute("CREATE VIEW v4 AS SELECT * FROM t WHERE v = 'bas'");
+        conn.createStatement().execute("CREATE VIEW s2.v4 AS SELECT * FROM s1.t WHERE v =
'bas'");
 
         // No error, even though view is invalid
-        conn.createStatement().execute("DROP VIEW v3");
+        conn.createStatement().execute("DROP VIEW s2.v3");
     }
 
 


Mime
View raw message