PHOENIX-2110 Addendum to fix test failure and enforcing tenancy switch check
Project: http://git-wip-us.apache.org/repos/asf/phoenix/repo Commit: http://git-wip-us.apache.org/repos/asf/phoenix/commit/f00c777c Tree: http://git-wip-us.apache.org/repos/asf/phoenix/tree/f00c777c Diff: http://git-wip-us.apache.org/repos/asf/phoenix/diff/f00c777c Branch: refs/heads/calcite Commit: f00c777cf5aa84fc3ecbea041756ba7c8f79aa0d Parents: 5b46793 Author: Samarth <samarth.j...@salesforce.com> Authored: Wed Jul 22 15:07:58 2015 -0700 Committer: Samarth <samarth.j...@salesforce.com> Committed: Wed Jul 22 15:07:58 2015 -0700 ---------------------------------------------------------------------- .../phoenix/end2end/AlterTableWithViewsIT.java | 31 ++++++++++++ .../end2end/TenantSpecificTablesDDLIT.java | 47 +++++++++++++++--- .../coprocessor/MetaDataEndpointImpl.java | 50 ++++++++++++++------ 3 files changed, 107 insertions(+), 21 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/phoenix/blob/f00c777c/phoenix-core/src/it/java/org/apache/phoenix/end2end/AlterTableWithViewsIT.java ---------------------------------------------------------------------- diff --git a/phoenix-core/src/it/java/org/apache/phoenix/end2end/AlterTableWithViewsIT.java b/phoenix-core/src/it/java/org/apache/phoenix/end2end/AlterTableWithViewsIT.java index 0399af2..22abd38 100644 --- a/phoenix-core/src/it/java/org/apache/phoenix/end2end/AlterTableWithViewsIT.java +++ b/phoenix-core/src/it/java/org/apache/phoenix/end2end/AlterTableWithViewsIT.java @@ -18,6 +18,7 @@ package org.apache.phoenix.end2end; import static org.apache.phoenix.exception.SQLExceptionCode.CANNOT_MUTATE_TABLE; +import static org.apache.phoenix.query.QueryConstants.BASE_TABLE_BASE_COLUMN_COUNT; import static org.apache.phoenix.query.QueryConstants.DIVERGED_VIEW_BASE_COLUMN_COUNT; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; @@ -1025,6 +1026,36 @@ public class AlterTableWithViewsIT extends BaseHBaseManagedTimeIT { } } + @Test + public void testAlterSaltedBaseTableWithViews() throws Exception { + String baseTable = "testAlterSaltedBaseTableWithViews".toUpperCase(); + String view1 = "view1".toUpperCase(); + try (Connection conn = DriverManager.getConnection(getUrl())) { + String baseTableDDL = "CREATE TABLE " + baseTable + " (TENANT_ID VARCHAR NOT NULL, PK1 VARCHAR NOT NULL, V1 VARCHAR, V2 VARCHAR, V3 VARCHAR CONSTRAINT NAME_PK PRIMARY KEY(TENANT_ID, PK1)) MULTI_TENANT = true "; + conn.createStatement().execute(baseTableDDL); + + try (Connection tenant1Conn = getTenantConnection("tenant1")) { + String view1DDL = "CREATE VIEW " + view1 + " ( VIEW_COL1 DECIMAL(10,2), VIEW_COL2 CHAR(256)) AS SELECT * FROM " + baseTable; + tenant1Conn.createStatement().execute(view1DDL); + } + + assertTableDefinition(conn, baseTable, PTableType.TABLE, null, 0, 5, BASE_TABLE_BASE_COLUMN_COUNT, "TENANT_ID", "PK1", "V1", "V2", "V3"); + assertTableDefinition(conn, view1, PTableType.VIEW, baseTable, 0, 7, 5, "TENANT_ID", "PK1", "V1", "V2", "V3", "VIEW_COL1", "VIEW_COL2"); + + String alterBaseTable = "ALTER TABLE " + baseTable + " ADD KV VARCHAR, PK2 VARCHAR PRIMARY KEY"; + conn.createStatement().execute(alterBaseTable); + + assertTableDefinition(conn, baseTable, PTableType.TABLE, null, 1, 7, BASE_TABLE_BASE_COLUMN_COUNT, "TENANT_ID", "PK1", "V1", "V2", "V3", "KV", "PK2"); + assertTableDefinition(conn, view1, PTableType.VIEW, baseTable, 1, 9, 7, "TENANT_ID", "PK1", "V1", "V2", "V3", "KV", "PK2", "VIEW_COL1", "VIEW_COL2"); + + // verify that the both columns were added to view1 + try (Connection tenant1Conn = getTenantConnection("tenant1")) { + tenant1Conn.createStatement().execute("SELECT KV from " + view1); + tenant1Conn.createStatement().execute("SELECT PK2 from " + view1); + } + } + } + private static long getTableSequenceNumber(PhoenixConnection conn, String tableName) throws SQLException { PTable table = conn.getMetaDataCache().getTable(new PTableKey(conn.getTenantId(), SchemaUtil.normalizeIdentifier(tableName))); return table.getSequenceNumber(); http://git-wip-us.apache.org/repos/asf/phoenix/blob/f00c777c/phoenix-core/src/it/java/org/apache/phoenix/end2end/TenantSpecificTablesDDLIT.java ---------------------------------------------------------------------- diff --git a/phoenix-core/src/it/java/org/apache/phoenix/end2end/TenantSpecificTablesDDLIT.java b/phoenix-core/src/it/java/org/apache/phoenix/end2end/TenantSpecificTablesDDLIT.java index bf86818..05b36c3 100644 --- a/phoenix-core/src/it/java/org/apache/phoenix/end2end/TenantSpecificTablesDDLIT.java +++ b/phoenix-core/src/it/java/org/apache/phoenix/end2end/TenantSpecificTablesDDLIT.java @@ -88,14 +88,49 @@ public class TenantSpecificTablesDDLIT extends BaseTenantSpecificTablesIT { } @Test - public void testAlterMultiTenantWithViewsToGlobal() throws Exception { + public void testAlteringMultiTenancyForTableWithViewsNotAllowed() throws Exception { Properties props = new Properties(); props.setProperty(PhoenixRuntime.CURRENT_SCN_ATTRIB, Long.toString(nextTimestamp())); - Connection conn = DriverManager.getConnection(getUrl(), props); - try { - conn.createStatement().execute("alter table " + PARENT_TABLE_NAME + " set MULTI_TENANT=false"); - } catch (SQLException e) { - assertEquals(SQLExceptionCode.CANNOT_MUTATE_TABLE.getErrorCode(), e.getErrorCode()); + String multiTenantTable = "BASE_MULTI_TENANT_SWITCH"; + String globalTable = "GLOBAL_TABLE_SWITCH"; + // create the two base tables + try (Connection conn = DriverManager.getConnection(getUrl(), props)) { + String ddl = "CREATE TABLE " + multiTenantTable + " (TENANT_ID VARCHAR NOT NULL, PK1 VARCHAR NOT NULL, V1 VARCHAR, V2 VARCHAR, V3 VARCHAR CONSTRAINT NAME_PK PRIMARY KEY(TENANT_ID, PK1)) MULTI_TENANT = true "; + conn.createStatement().execute(ddl); + ddl = "CREATE TABLE " + globalTable + " (TENANT_ID VARCHAR NOT NULL, PK1 VARCHAR NOT NULL, V1 VARCHAR, V2 VARCHAR, V3 VARCHAR CONSTRAINT NAME_PK PRIMARY KEY(TENANT_ID, PK1)) "; + conn.createStatement().execute(ddl); + } + props.setProperty(PhoenixRuntime.CURRENT_SCN_ATTRIB, Long.toString(nextTimestamp())); + props.setProperty(PhoenixRuntime.TENANT_ID_ATTRIB, "tenant1"); + // create view on multi-tenant table + try (Connection tenantConn = DriverManager.getConnection(getUrl(), props)) { + String viewName = "tenantview"; + String viewDDL = "CREATE VIEW " + viewName + " AS SELECT * FROM " + multiTenantTable; + tenantConn.createStatement().execute(viewDDL); + } + props = new Properties(); + props.setProperty(PhoenixRuntime.CURRENT_SCN_ATTRIB, Long.toString(nextTimestamp())); + // create view on global table + try (Connection conn = DriverManager.getConnection(getUrl(), props)) { + String viewName = "globalView"; + conn.createStatement().execute("CREATE VIEW " + viewName + " AS SELECT * FROM " + globalTable); + } + props = new Properties(); + props.setProperty(PhoenixRuntime.CURRENT_SCN_ATTRIB, Long.toString(nextTimestamp())); + try (Connection conn = DriverManager.getConnection(getUrl(), props)) { + try { + conn.createStatement().execute("ALTER TABLE " + globalTable + " SET MULTI_TENANT = " + true); + fail(); + } catch (SQLException e) { + assertEquals(SQLExceptionCode.CANNOT_MUTATE_TABLE.getErrorCode(), e.getErrorCode()); + } + + try { + conn.createStatement().execute("ALTER TABLE " + multiTenantTable + " SET MULTI_TENANT = " + false); + fail(); + } catch (SQLException e) { + assertEquals(SQLExceptionCode.CANNOT_MUTATE_TABLE.getErrorCode(), e.getErrorCode()); + } } } http://git-wip-us.apache.org/repos/asf/phoenix/blob/f00c777c/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 4c17bf1..13788e1 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 @@ -2240,7 +2240,7 @@ public class MetaDataEndpointImpl extends MetaDataProtocol implements Coprocesso TableViewFinderResult childViewsResult = findChildViews(region, tenantId, table, PHYSICAL_TABLE_BYTES); if (childViewsResult.hasViews()) { /* - * Adding a column is not allowed if + * Dis-allow if: * 1) The meta-data for child view/s spans over * more than one region (since the changes cannot be made in a transactional fashion) * @@ -2250,19 +2250,24 @@ public class MetaDataEndpointImpl extends MetaDataProtocol implements Coprocesso * 3) If the request is from a client that is older than 4.5 version of phoenix. * Starting from 4.5, metadata requests have the client version included in them. * We don't want to allow clients before 4.5 to add a column to the base table if it has views. + * + * 4) Trying to swtich tenancy of a table that has views */ - if (!childViewsResult.allViewsInSingleRegion() || table.getBaseColumnCount() == 0 || !request.hasClientVersion()) { - return new MetaDataMutationResult(MutationCode.UNALLOWED_TABLE_MUTATION, - EnvironmentEdgeManager.currentTimeMillis(), null); - } else { - mutationsForAddingColumnsToViews = new ArrayList<>(childViewsResult.getResults().size() * tableMetaData.size()); - 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) - return mutationResult; - } - } + if (!childViewsResult.allViewsInSingleRegion() + || table.getBaseColumnCount() == 0 + || !request.hasClientVersion() + || switchTenancy(table, tableMetaData)) { + return new MetaDataMutationResult(MutationCode.UNALLOWED_TABLE_MUTATION, + EnvironmentEdgeManager.currentTimeMillis(), null); + } else { + mutationsForAddingColumnsToViews = new ArrayList<>(childViewsResult.getResults().size() * tableMetaData.size()); + 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) + return mutationResult; + } + } } for (Mutation m : tableMetaData) { byte[] key = m.getRow(); @@ -2287,7 +2292,7 @@ public class MetaDataEndpointImpl extends MetaDataProtocol implements Coprocesso } return new MetaDataMutationResult( MutationCode.COLUMN_ALREADY_EXISTS, EnvironmentEdgeManager - .currentTimeMillis(), table); + .currentTimeMillis(), table); } catch (ColumnFamilyNotFoundException e) { continue; } catch (ColumnNotFoundException e) { @@ -2301,7 +2306,7 @@ public class MetaDataEndpointImpl extends MetaDataProtocol implements Coprocesso // does not handle this. return new MetaDataMutationResult( MutationCode.UNALLOWED_TABLE_MUTATION, EnvironmentEdgeManager - .currentTimeMillis(), null); + .currentTimeMillis(), null); } // Add all indexes to invalidate list, as they will all be // adding the same PK column. No need to lock them, as we @@ -2327,6 +2332,21 @@ public class MetaDataEndpointImpl extends MetaDataProtocol implements Coprocesso tableMetaData.addAll(mutationsForAddingColumnsToViews); return null; } + + private boolean switchTenancy(PTable table, List<Mutation> tableMetaData) { + for (Mutation m : tableMetaData) { + if (m instanceof Put) { + Put p = (Put)m; + List<Cell> cells = p.get(TABLE_FAMILY_BYTES, MULTI_TENANT_BYTES); + if (cells != null && cells.size() > 0) { + Cell cell = cells.get(0); + boolean isMutlitenantProp = (boolean)PBoolean.INSTANCE.toObject(cell.getValueArray(), cell.getValueOffset(), cell.getValueLength()); + return table.isMultiTenant() != isMutlitenantProp; + } + } + } + return false; + } }); if (result != null) { done.run(MetaDataMutationResult.toProto(result));