PHOENIX-2055 Allow view with views to add column
Project: http://git-wip-us.apache.org/repos/asf/phoenix/repo Commit: http://git-wip-us.apache.org/repos/asf/phoenix/commit/97f0d626 Tree: http://git-wip-us.apache.org/repos/asf/phoenix/tree/97f0d626 Diff: http://git-wip-us.apache.org/repos/asf/phoenix/diff/97f0d626 Branch: refs/heads/4.x-HBase-0.98 Commit: 97f0d626079321fff78d1b117dbc3cfc16d7e2ee Parents: b95907e Author: Samarth <samarth.j...@salesforce.com> Authored: Tue Jun 23 15:33:17 2015 -0700 Committer: Samarth <samarth.j...@salesforce.com> Committed: Tue Jun 23 15:33:17 2015 -0700 ---------------------------------------------------------------------- .../apache/phoenix/end2end/AlterTableIT.java | 16 +++--- .../org/apache/phoenix/end2end/UpgradeIT.java | 12 ++--- .../coprocessor/MetaDataEndpointImpl.java | 51 ++++++++++++-------- .../apache/phoenix/query/QueryConstants.java | 2 +- .../org/apache/phoenix/util/UpgradeUtil.java | 6 +-- 5 files changed, 50 insertions(+), 37 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/phoenix/blob/97f0d626/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 1302c60..3a9517d 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 @@ -2127,8 +2127,8 @@ public class AlterTableIT extends BaseOwnClusterHBaseManagedTimeIT { } @Test - public void testAlteringViewThatHasChildViewsNotAllowed() throws Exception { - String baseTable = "testAlteringViewThatHasChildViewsNotAllowed"; + public void testAlteringViewThatHasChildViews() throws Exception { + String baseTable = "testAlteringViewThatHasChildViews"; String childView = "childView"; String grandChildView = "grandChildView"; try (Connection conn = DriverManager.getConnection(getUrl())) { @@ -2165,13 +2165,17 @@ public class AlterTableIT extends BaseOwnClusterHBaseManagedTimeIT { assertEquals(SQLExceptionCode.CANNOT_MUTATE_TABLE.getErrorCode(), e.getErrorCode()); } - // Adding column to view that has child views should fail + // Adding column to view that has child views is allowed String addColumnToChildView = "ALTER VIEW " + childView + " ADD V5 VARCHAR"; + conn.createStatement().execute(addColumnToChildView); + // V5 column should be visible now for childView + conn.createStatement().execute("SELECT V5 FROM " + childView); + + // However, column V5 shouldn't have propagated to grandChildView. Not till PHOENIX-2054 is fixed. try { - conn.createStatement().execute(addColumnToChildView); - fail("Adding columns to a view that has child views on it is not allowed"); + conn.createStatement().execute("SELECT V5 FROM " + grandChildView); } catch (SQLException e) { - assertEquals(SQLExceptionCode.CANNOT_MUTATE_TABLE.getErrorCode(), e.getErrorCode()); + assertEquals(SQLExceptionCode.COLUMN_NOT_FOUND.getErrorCode(), e.getErrorCode()); } // dropping column from the grand child view, however, should work. http://git-wip-us.apache.org/repos/asf/phoenix/blob/97f0d626/phoenix-core/src/it/java/org/apache/phoenix/end2end/UpgradeIT.java ---------------------------------------------------------------------- diff --git a/phoenix-core/src/it/java/org/apache/phoenix/end2end/UpgradeIT.java b/phoenix-core/src/it/java/org/apache/phoenix/end2end/UpgradeIT.java index 886e567..094816c 100644 --- a/phoenix-core/src/it/java/org/apache/phoenix/end2end/UpgradeIT.java +++ b/phoenix-core/src/it/java/org/apache/phoenix/end2end/UpgradeIT.java @@ -20,7 +20,7 @@ package org.apache.phoenix.end2end; import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.base.Preconditions.checkNotNull; import static org.apache.phoenix.query.QueryConstants.BASE_TABLE_BASE_COLUMN_COUNT; -import static org.apache.phoenix.query.QueryConstants.DIVORCED_VIEW_BASE_COLUMN_COUNT; +import static org.apache.phoenix.query.QueryConstants.DIVERGED_VIEW_BASE_COLUMN_COUNT; import static org.apache.phoenix.util.UpgradeUtil.SELECT_BASE_COLUMN_COUNT_FROM_HEADER_ROW; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; @@ -187,14 +187,14 @@ public class UpgradeIT extends BaseHBaseManagedTimeIT { for (int i = 1; i <=2 ; i++) { String tenantId = "tenant" + i; checkBaseColumnCount(tenantId, null, "TENANT_VIEW1", 4); - checkBaseColumnCount(tenantId, null, "TENANT_VIEW2", DIVORCED_VIEW_BASE_COLUMN_COUNT); - checkBaseColumnCount(tenantId, null, "TENANT_VIEW3", DIVORCED_VIEW_BASE_COLUMN_COUNT); + checkBaseColumnCount(tenantId, null, "TENANT_VIEW2", DIVERGED_VIEW_BASE_COLUMN_COUNT); + checkBaseColumnCount(tenantId, null, "TENANT_VIEW3", DIVERGED_VIEW_BASE_COLUMN_COUNT); } // Verify base column count for global views checkBaseColumnCount(null, null, "GLOBAL_VIEW1", 4); - checkBaseColumnCount(null, null, "GLOBAL_VIEW2", DIVORCED_VIEW_BASE_COLUMN_COUNT); - checkBaseColumnCount(null, null, "GLOBAL_VIEW3", DIVORCED_VIEW_BASE_COLUMN_COUNT); + checkBaseColumnCount(null, null, "GLOBAL_VIEW2", DIVERGED_VIEW_BASE_COLUMN_COUNT); + checkBaseColumnCount(null, null, "GLOBAL_VIEW3", DIVERGED_VIEW_BASE_COLUMN_COUNT); } @@ -243,7 +243,7 @@ public class UpgradeIT extends BaseHBaseManagedTimeIT { conn3.createStatement().execute( "ALTER VIEW " + fullViewName + " DROP COLUMN CF2.V2"); } - expectedBaseColumnCount = DIVORCED_VIEW_BASE_COLUMN_COUNT; + expectedBaseColumnCount = DIVERGED_VIEW_BASE_COLUMN_COUNT; } else { expectedBaseColumnCount = 6; } http://git-wip-us.apache.org/repos/asf/phoenix/blob/97f0d626/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 af0c7de..e13c635 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 @@ -61,7 +61,7 @@ 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.query.QueryConstants.DIVORCED_VIEW_BASE_COLUMN_COUNT; +import static org.apache.phoenix.query.QueryConstants.DIVERGED_VIEW_BASE_COLUMN_COUNT; import static org.apache.phoenix.query.QueryConstants.SEPARATOR_BYTE_ARRAY; import static org.apache.phoenix.schema.PTableType.INDEX; import static org.apache.phoenix.util.ByteUtil.EMPTY_BYTE_ARRAY; @@ -1584,7 +1584,7 @@ public class MetaDataEndpointImpl extends MetaDataProtocol implements Coprocesso // lock the rows corresponding to views so that no other thread can modify the view meta-data RowLock viewRowLock = acquireLock(region, viewKey, locks); PTable view = doGetTable(viewKey, clientTimeStamp, viewRowLock); - if (view.getBaseColumnCount() == QueryConstants.DIVORCED_VIEW_BASE_COLUMN_COUNT) { + if (view.getBaseColumnCount() == QueryConstants.DIVERGED_VIEW_BASE_COLUMN_COUNT) { // if a view has divorced itself from the base table, we don't allow schema changes // to be propagated to it. return; @@ -1790,21 +1790,29 @@ public class MetaDataEndpointImpl extends MetaDataProtocol implements Coprocesso byte[] schemaName = rowKeyMetaData[SCHEMA_NAME_INDEX]; byte[] tableName = rowKeyMetaData[TABLE_NAME_INDEX]; PTableType type = table.getType(); - TableViewFinderResult childViewsResult = findChildViews(region, tenantId, table, - (type == PTableType.VIEW ? PARENT_TABLE_BYTES : PHYSICAL_TABLE_BYTES)); List<Mutation> mutationsForAddingColumnsToViews = Collections.emptyList(); - if (childViewsResult.hasViews()) { - /* - * Adding a column is not allowed if: 1) Meta-data for child view/s spans over more than one - * region. 2) Adding column to a views that has child view/s. - */ - if (!childViewsResult.allViewsInSingleRegion() || type == PTableType.VIEW) { - return new MetaDataMutationResult(MutationCode.UNALLOWED_TABLE_MUTATION, - EnvironmentEdgeManager.currentTimeMillis(), null); - } else { - mutationsForAddingColumnsToViews = new ArrayList<>(childViewsResult.getResults().size() * tableMetaData.size()); - addRowsToChildViews(tableMetaData, mutationsForAddingColumnsToViews, schemaName, tableName, invalidateList, clientTimeStamp, - childViewsResult, region, locks); + /* + * If adding a column to a view, we don't want to propagate those meta-data changes to the child + * view hierarchy. This is because our check of finding child views is expensive and we want making + * meta-data changes to views to be light-weight. The side-effect of this change is that a child + * won't have it's parent views columns i.e. it would have diverged itself from the parent view. See + * https://issues.apache.org/jira/browse/PHOENIX-2051 for a proper way to fix the performance issue + * and https://issues.apache.org/jira/browse/PHOENIX-2054 for enabling meta-data changes to a view + * to be propagated to its view hierarchy. + */ + if (type == PTableType.TABLE) { + TableViewFinderResult childViewsResult = findChildViews(region, tenantId, table, PHYSICAL_TABLE_BYTES); + if (childViewsResult.hasViews()) { + // Adding a column is not allowed if the meta-data for child view/s spans over + // more than one region (since the changes cannot be done in a transactional fashion) + if (!childViewsResult.allViewsInSingleRegion()) { + return new MetaDataMutationResult(MutationCode.UNALLOWED_TABLE_MUTATION, + EnvironmentEdgeManager.currentTimeMillis(), null); + } else { + mutationsForAddingColumnsToViews = new ArrayList<>(childViewsResult.getResults().size() * tableMetaData.size()); + addRowsToChildViews(tableMetaData, mutationsForAddingColumnsToViews, schemaName, tableName, invalidateList, clientTimeStamp, + childViewsResult, region, locks); + } } } for (Mutation m : tableMetaData) { @@ -2037,18 +2045,19 @@ public class MetaDataEndpointImpl extends MetaDataProtocol implements Coprocesso continue; } if (table.getType() == PTableType.VIEW) { - if (table.getBaseColumnCount() != DIVORCED_VIEW_BASE_COLUMN_COUNT + if (table.getBaseColumnCount() != DIVERGED_VIEW_BASE_COLUMN_COUNT && columnToDelete.getPosition() < table.getBaseColumnCount()) { /* * If the column being dropped is inherited from the base table, then the - * view is about to divorce itself from the base table. Divorce here means - * that any further meta-data changes made to the base table will not be - * propagated to the hierarchy of views on the base table. + * view is about to diverge itself from the base table. The consequence of + * this divergence is that that any further meta-data changes made to the + * base table will not be propagated to the hierarchy of views where this + * view is the root. */ byte[] viewKey = SchemaUtil.getTableKey(tenantId, schemaName, tableName); Put updateBaseColumnCountPut = new Put(viewKey); byte[] baseColumnCountPtr = new byte[PInteger.INSTANCE.getByteSize()]; - PInteger.INSTANCE.getCodec().encodeInt(DIVORCED_VIEW_BASE_COLUMN_COUNT, + PInteger.INSTANCE.getCodec().encodeInt(DIVERGED_VIEW_BASE_COLUMN_COUNT, baseColumnCountPtr, 0); updateBaseColumnCountPut.add(PhoenixDatabaseMetaData.TABLE_FAMILY_BYTES, PhoenixDatabaseMetaData.BASE_COLUMN_COUNT_BYTES, clientTimeStamp, http://git-wip-us.apache.org/repos/asf/phoenix/blob/97f0d626/phoenix-core/src/main/java/org/apache/phoenix/query/QueryConstants.java ---------------------------------------------------------------------- diff --git a/phoenix-core/src/main/java/org/apache/phoenix/query/QueryConstants.java b/phoenix-core/src/main/java/org/apache/phoenix/query/QueryConstants.java index f82c594..d095049 100644 --- a/phoenix-core/src/main/java/org/apache/phoenix/query/QueryConstants.java +++ b/phoenix-core/src/main/java/org/apache/phoenix/query/QueryConstants.java @@ -185,7 +185,7 @@ public interface QueryConstants { public static final BigDecimal BD_MILLIS_IN_DAY = BigDecimal.valueOf(QueryConstants.MILLIS_IN_DAY); public static final int MAX_ALLOWED_NANOS = 999999999; public static final int NANOS_IN_SECOND = BigDecimal.valueOf(Math.pow(10, 9)).intValue(); - public static final int DIVORCED_VIEW_BASE_COLUMN_COUNT = -100; + public static final int DIVERGED_VIEW_BASE_COLUMN_COUNT = -100; public static final int BASE_TABLE_BASE_COLUMN_COUNT = -1; public static final String CREATE_TABLE_METADATA = // Do not use IF NOT EXISTS as we sometimes catch the TableAlreadyExists http://git-wip-us.apache.org/repos/asf/phoenix/blob/97f0d626/phoenix-core/src/main/java/org/apache/phoenix/util/UpgradeUtil.java ---------------------------------------------------------------------- diff --git a/phoenix-core/src/main/java/org/apache/phoenix/util/UpgradeUtil.java b/phoenix-core/src/main/java/org/apache/phoenix/util/UpgradeUtil.java index dff6598..8d574ce 100644 --- a/phoenix-core/src/main/java/org/apache/phoenix/util/UpgradeUtil.java +++ b/phoenix-core/src/main/java/org/apache/phoenix/util/UpgradeUtil.java @@ -32,7 +32,7 @@ import static org.apache.phoenix.jdbc.PhoenixDatabaseMetaData.TABLE_NAME; import static org.apache.phoenix.jdbc.PhoenixDatabaseMetaData.TABLE_SCHEM; import static org.apache.phoenix.jdbc.PhoenixDatabaseMetaData.TENANT_ID; import static org.apache.phoenix.query.QueryConstants.BASE_TABLE_BASE_COLUMN_COUNT; -import static org.apache.phoenix.query.QueryConstants.DIVORCED_VIEW_BASE_COLUMN_COUNT; +import static org.apache.phoenix.query.QueryConstants.DIVERGED_VIEW_BASE_COLUMN_COUNT; import java.io.IOException; import java.sql.PreparedStatement; @@ -595,7 +595,7 @@ public class UpgradeUtil { // We are about to iterate through columns of a different view. Check whether base column count was upserted. // If it wasn't then it is likely the case that a column inherited from the base table was dropped from view. if (currentViewName != null && !baseColumnCountUpserted && numBaseTableColsMatched < numColsInBaseTable) { - upsertBaseColumnCountInHeaderRow(metaConnection, currentTenantId, currentViewSchema, currentViewName, DIVORCED_VIEW_BASE_COLUMN_COUNT); + upsertBaseColumnCountInHeaderRow(metaConnection, currentTenantId, currentViewSchema, currentViewName, DIVERGED_VIEW_BASE_COLUMN_COUNT); } // reset the values as we are now going to iterate over columns of a new view. numBaseTableColsMatched = 0; @@ -641,7 +641,7 @@ public class UpgradeUtil { } } else { // special value to denote that the view has divorced itself from the base physical table. - upsertBaseColumnCountInHeaderRow(metaConnection, viewTenantId, viewSchema, viewName, DIVORCED_VIEW_BASE_COLUMN_COUNT); + upsertBaseColumnCountInHeaderRow(metaConnection, viewTenantId, viewSchema, viewName, DIVERGED_VIEW_BASE_COLUMN_COUNT); baseColumnCountUpserted = true; // ignore rest of the rows for the view. ignore = true;
