PHOENIX-2050 Avoid checking for child views unless operating on table
Project: http://git-wip-us.apache.org/repos/asf/phoenix/repo Commit: http://git-wip-us.apache.org/repos/asf/phoenix/commit/a8a9d01d Tree: http://git-wip-us.apache.org/repos/asf/phoenix/tree/a8a9d01d Diff: http://git-wip-us.apache.org/repos/asf/phoenix/diff/a8a9d01d Branch: refs/heads/calcite Commit: a8a9d01d1eaafc33ea73913bec16254ac6a55be3 Parents: fb8c941 Author: James Taylor <jtay...@salesforce.com> Authored: Mon Jun 29 21:36:19 2015 -0700 Committer: James Taylor <jtay...@salesforce.com> Committed: Tue Jun 30 17:31:24 2015 -0700 ---------------------------------------------------------------------- .../apache/phoenix/end2end/AlterTableIT.java | 18 +-- .../coprocessor/MetaDataEndpointImpl.java | 141 ++++++++++--------- 2 files changed, 81 insertions(+), 78 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/phoenix/blob/a8a9d01d/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 946aaab..cd46927 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 @@ -2149,23 +2149,13 @@ public class AlterTableIT extends BaseOwnClusterHBaseManagedTimeIT { "CREATE VIEW " + grandChildView + " AS SELECT * FROM " + childView; conn.createStatement().execute(grandChildViewDDL); - // dropping base table column from child view should fail + // dropping base table column from child view should succeed String dropColumnFromChildView = "ALTER VIEW " + childView + " DROP COLUMN V2"; - try { - conn.createStatement().execute(dropColumnFromChildView); - fail("Dropping columns from a view that has child views on it is not allowed"); - } catch (SQLException e) { - assertEquals(CANNOT_MUTATE_TABLE.getErrorCode(), e.getErrorCode()); - } + conn.createStatement().execute(dropColumnFromChildView); - // dropping view specific column from child view should fail + // dropping view specific column from child view should succeed dropColumnFromChildView = "ALTER VIEW " + childView + " DROP COLUMN CHILD_VIEW_COL"; - try { - conn.createStatement().execute(dropColumnFromChildView); - fail("Dropping columns from a view that has child views on it is not allowed"); - } catch (SQLException e) { - assertEquals(SQLExceptionCode.CANNOT_MUTATE_TABLE.getErrorCode(), e.getErrorCode()); - } + conn.createStatement().execute(dropColumnFromChildView); // Adding column to view that has child views is allowed String addColumnToChildView = "ALTER VIEW " + childView + " ADD V5 VARCHAR"; http://git-wip-us.apache.org/repos/asf/phoenix/blob/a8a9d01d/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 0ddd58d..cc486d5 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 @@ -1366,69 +1366,76 @@ public class MetaDataEndpointImpl extends MetaDataProtocol implements Coprocesso List<byte[]> indexNames = Lists.newArrayList(); List<Cell> results = Lists.newArrayList(); try (RegionScanner scanner = region.getScanner(scan);) { - scanner.next(results); - if (results.isEmpty()) { // Should not be possible - return new MetaDataMutationResult(MutationCode.TABLE_NOT_FOUND, EnvironmentEdgeManager.currentTimeMillis(), null); - } + scanner.next(results); + if (results.isEmpty()) { // Should not be possible + return new MetaDataMutationResult(MutationCode.TABLE_NOT_FOUND, + EnvironmentEdgeManager.currentTimeMillis(), null); + } - // Handle any child views that exist - TableViewFinderResult tableViewFinderResult = findChildViews(region, tenantId, table, PHYSICAL_TABLE_BYTES); - if (tableViewFinderResult.hasViews()) { - if (isCascade) { - if (tableViewFinderResult.allViewsInMultipleRegions()) { - // We don't yet support deleting a table with views where SYSTEM.CATALOG has split and the - // view metadata spans multiple regions - return new MetaDataMutationResult(MutationCode.UNALLOWED_TABLE_MUTATION, EnvironmentEdgeManager.currentTimeMillis(), null); - } else if (tableViewFinderResult.allViewsInSingleRegion()) { - // Recursively delete views - safe as all the views as all in the same region - for (Result viewResult : tableViewFinderResult.getResults()) { - byte[][] rowKeyMetaData = new byte[3][]; - getVarChars(viewResult.getRow(), 3, rowKeyMetaData); - byte[] viewTenantId = rowKeyMetaData[PhoenixDatabaseMetaData.TENANT_ID_INDEX]; - byte[] viewSchemaName = rowKeyMetaData[PhoenixDatabaseMetaData.SCHEMA_NAME_INDEX]; - byte[] viewName = rowKeyMetaData[PhoenixDatabaseMetaData.TABLE_NAME_INDEX]; - byte[] viewKey = SchemaUtil.getTableKey(viewTenantId, viewSchemaName, viewName); - Delete delete = new Delete(viewKey, clientTimeStamp); - rowsToDelete.add(delete); - acquireLock(region, viewKey, locks); - MetaDataMutationResult result = - doDropTable(viewKey, viewTenantId, viewSchemaName, viewName, null, PTableType.VIEW, - rowsToDelete, invalidateList, locks, tableNamesToDelete, false); - if (result.getMutationCode() != MutationCode.TABLE_ALREADY_EXISTS) { - return result; - } + // Only tables may have views, so prevent the running of this potentially + // expensive full table scan over the SYSTEM.CATALOG table unless it's needed. + if (tableType == PTableType.TABLE || tableType == PTableType.SYSTEM) { + // Handle any child views that exist + TableViewFinderResult tableViewFinderResult = findChildViews(region, tenantId, table, + PHYSICAL_TABLE_BYTES); + if (tableViewFinderResult.hasViews()) { + if (isCascade) { + if (tableViewFinderResult.allViewsInMultipleRegions()) { + // We don't yet support deleting a table with views where SYSTEM.CATALOG has split and the + // view metadata spans multiple regions + return new MetaDataMutationResult(MutationCode.UNALLOWED_TABLE_MUTATION, + EnvironmentEdgeManager.currentTimeMillis(), null); + } else if (tableViewFinderResult.allViewsInSingleRegion()) { + // Recursively delete views - safe as all the views as all in the same region + for (Result viewResult : tableViewFinderResult.getResults()) { + byte[][] rowKeyMetaData = new byte[3][]; + getVarChars(viewResult.getRow(), 3, rowKeyMetaData); + byte[] viewTenantId = rowKeyMetaData[PhoenixDatabaseMetaData.TENANT_ID_INDEX]; + byte[] viewSchemaName = rowKeyMetaData[PhoenixDatabaseMetaData.SCHEMA_NAME_INDEX]; + byte[] viewName = rowKeyMetaData[PhoenixDatabaseMetaData.TABLE_NAME_INDEX]; + byte[] viewKey = SchemaUtil.getTableKey(viewTenantId, viewSchemaName, viewName); + Delete delete = new Delete(viewKey, clientTimeStamp); + rowsToDelete.add(delete); + acquireLock(region, viewKey, locks); + MetaDataMutationResult result = doDropTable(viewKey, viewTenantId, viewSchemaName, + viewName, null, PTableType.VIEW, rowsToDelete, invalidateList, locks, + tableNamesToDelete, false); + if (result.getMutationCode() != MutationCode.TABLE_ALREADY_EXISTS) { return result; } + } + } + } else { + // DROP without CASCADE on tables with child views is not permitted + return new MetaDataMutationResult(MutationCode.UNALLOWED_TABLE_MUTATION, + EnvironmentEdgeManager.currentTimeMillis(), null); + } } - } - } else { - // DROP without CASCADE on tables with child views is not permitted - return new MetaDataMutationResult(MutationCode.UNALLOWED_TABLE_MUTATION, EnvironmentEdgeManager.currentTimeMillis(), null); } - } - if (tableType != PTableType.VIEW) { // Add to list of HTables to delete, unless it's a view - tableNamesToDelete.add(table.getName().getBytes()); - } - invalidateList.add(cacheKey); - byte[][] rowKeyMetaData = new byte[5][]; - do { - Cell kv = results.get(LINK_TYPE_INDEX); - int nColumns = getVarChars(kv.getRowArray(), kv.getRowOffset(), kv.getRowLength(), 0, rowKeyMetaData); - if (nColumns == 5 - && rowKeyMetaData[PhoenixDatabaseMetaData.COLUMN_NAME_INDEX].length == 0 - && rowKeyMetaData[PhoenixDatabaseMetaData.INDEX_NAME_INDEX].length > 0 - && Bytes.compareTo(kv.getQualifierArray(), kv.getQualifierOffset(), kv.getQualifierLength(), LINK_TYPE_BYTES, 0, LINK_TYPE_BYTES.length) == 0 - && LinkType.fromSerializedValue(kv.getValueArray()[kv.getValueOffset()]) == LinkType.INDEX_TABLE) { - indexNames.add(rowKeyMetaData[PhoenixDatabaseMetaData.INDEX_NAME_INDEX]); + if (tableType != PTableType.VIEW) { // Add to list of HTables to delete, unless it's a view + tableNamesToDelete.add(table.getName().getBytes()); } - // FIXME: Remove when unintentionally deprecated method is fixed (HBASE-7870). - // FIXME: the version of the Delete constructor without the lock args was introduced - // in 0.94.4, thus if we try to use it here we can no longer use the 0.94.2 version - // of the client. - Delete delete = new Delete(kv.getRowArray(), kv.getRowOffset(), kv.getRowLength(), clientTimeStamp); - rowsToDelete.add(delete); - results.clear(); - scanner.next(results); - } while (!results.isEmpty()); + invalidateList.add(cacheKey); + byte[][] rowKeyMetaData = new byte[5][]; + do { + Cell kv = results.get(LINK_TYPE_INDEX); + int nColumns = getVarChars(kv.getRowArray(), kv.getRowOffset(), kv.getRowLength(), 0, rowKeyMetaData); + if (nColumns == 5 + && rowKeyMetaData[PhoenixDatabaseMetaData.COLUMN_NAME_INDEX].length == 0 + && rowKeyMetaData[PhoenixDatabaseMetaData.INDEX_NAME_INDEX].length > 0 + && Bytes.compareTo(kv.getQualifierArray(), kv.getQualifierOffset(), kv.getQualifierLength(), + LINK_TYPE_BYTES, 0, LINK_TYPE_BYTES.length) == 0 + && LinkType.fromSerializedValue(kv.getValueArray()[kv.getValueOffset()]) == LinkType.INDEX_TABLE) { + indexNames.add(rowKeyMetaData[PhoenixDatabaseMetaData.INDEX_NAME_INDEX]); + } + // FIXME: Remove when unintentionally deprecated method is fixed (HBASE-7870). + // FIXME: the version of the Delete constructor without the lock args was introduced + // in 0.94.4, thus if we try to use it here we can no longer use the 0.94.2 version + // of the client. + Delete delete = new Delete(kv.getRowArray(), kv.getRowOffset(), kv.getRowLength(), clientTimeStamp); + rowsToDelete.add(delete); + results.clear(); + scanner.next(results); + } while (!results.isEmpty()); } // Recursively delete indexes @@ -1804,7 +1811,7 @@ public class MetaDataEndpointImpl extends MetaDataProtocol implements Coprocesso * 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) { + if (type == PTableType.TABLE || type == PTableType.SYSTEM) { 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 @@ -2017,11 +2024,17 @@ public class MetaDataEndpointImpl extends MetaDataProtocol implements Coprocesso List<Mutation> additionalTableMetaData = Lists.newArrayList(); PTableType type = table.getType(); - TableViewFinderResult childViewsResult = findChildViews(region, tenantId, table, - (type == PTableType.VIEW ? PARENT_TABLE_BYTES : PHYSICAL_TABLE_BYTES)); - if (childViewsResult.hasViews()) { - return new MetaDataMutationResult(MutationCode.UNALLOWED_TABLE_MUTATION, EnvironmentEdgeManager - .currentTimeMillis(), null); + // Only tables may have views, so prevent the running of this potentially + // expensive full table scan over the SYSTEM.CATALOG table unless it's needed. + // In the case of a view, we allow a column to be dropped without checking for + // child views, but in the future we'll allow it and propagate it as necessary. + if (type == PTableType.TABLE || type == PTableType.SYSTEM) { + TableViewFinderResult childViewsResult = + findChildViews(region, tenantId, table, PHYSICAL_TABLE_BYTES); + if (childViewsResult.hasViews()) { + return new MetaDataMutationResult(MutationCode.UNALLOWED_TABLE_MUTATION, EnvironmentEdgeManager + .currentTimeMillis(), null); + } } for (Mutation m : tableMetaData) {