You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@phoenix.apache.org by ya...@apache.org on 2020/10/20 16:33:28 UTC
[phoenix] branch 4.x updated: PHOENIX-6124 : Conditionally block
adding/dropping a column on a parent view
This is an automated email from the ASF dual-hosted git repository.
yanxinyi pushed a commit to branch 4.x
in repository https://gitbox.apache.org/repos/asf/phoenix.git
The following commit(s) were added to refs/heads/4.x by this push:
new 4c88a81 PHOENIX-6124 : Conditionally block adding/dropping a column on a parent view
4c88a81 is described below
commit 4c88a81d0cba7614f27fc5fefa1ad13be2effec5
Author: Viraj Jasani <vj...@apache.org>
AuthorDate: Mon Oct 19 20:31:30 2020 +0530
PHOENIX-6124 : Conditionally block adding/dropping a column on a parent view
Signed-off-by: Xinyi Yan <ya...@apache.org>
---
.../AlterParentTableWithSysCatRollbackIT.java | 142 +++++++++++++++++++++
.../phoenix/coprocessor/MetaDataEndpointImpl.java | 119 ++++++++++++-----
2 files changed, 232 insertions(+), 29 deletions(-)
diff --git a/phoenix-core/src/it/java/org/apache/phoenix/end2end/AlterParentTableWithSysCatRollbackIT.java b/phoenix-core/src/it/java/org/apache/phoenix/end2end/AlterParentTableWithSysCatRollbackIT.java
new file mode 100644
index 0000000..99a2124
--- /dev/null
+++ b/phoenix-core/src/it/java/org/apache/phoenix/end2end/AlterParentTableWithSysCatRollbackIT.java
@@ -0,0 +1,142 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.phoenix.end2end;
+
+import com.google.common.collect.Maps;
+import org.apache.phoenix.exception.SQLExceptionCode;
+import org.apache.phoenix.query.BaseTest;
+import org.apache.phoenix.query.QueryServices;
+import org.apache.phoenix.util.ReadOnlyProps;
+import org.apache.phoenix.util.SchemaUtil;
+import org.junit.BeforeClass;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
+import java.sql.Connection;
+import java.sql.DriverManager;
+import java.sql.SQLException;
+import java.util.Collections;
+import java.util.Map;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.fail;
+
+@Category(NeedsOwnMiniClusterTest.class)
+public class AlterParentTableWithSysCatRollbackIT extends BaseTest {
+
+ private String getJdbcUrl() {
+ return "jdbc:phoenix:localhost:" + getUtility().getZkCluster().getClientPort()
+ + ":/hbase";
+ }
+
+ @BeforeClass
+ public static synchronized void doSetup() throws Exception {
+ Map<String, String> serverProps = Maps.newHashMapWithExpectedSize(1);
+ serverProps.put(QueryServices.ALLOW_SPLITTABLE_SYSTEM_CATALOG_ROLLBACK, "true");
+ setUpTestDriver(new ReadOnlyProps(serverProps.entrySet().iterator()),
+ new ReadOnlyProps(Collections.<String, String>emptyMap()));
+ }
+
+ @Test
+ public void testAddColumnOnParentTableView() throws Exception {
+ try (Connection conn = DriverManager.getConnection(getJdbcUrl())) {
+ String parentTableName = SchemaUtil.getTableName(generateUniqueName(),
+ generateUniqueName());
+ String parentViewName = SchemaUtil.getTableName(generateUniqueName(),
+ generateUniqueName());
+ String childViewName = SchemaUtil.getTableName(generateUniqueName(),
+ generateUniqueName());
+ // create parent table
+ String ddl = "CREATE TABLE " + parentTableName
+ + " (col1 INTEGER NOT NULL, col2 INTEGER " + "CONSTRAINT pk PRIMARY KEY (col1))";
+ conn.createStatement().execute(ddl);
+
+ // create view from table
+ ddl = "CREATE VIEW " + parentViewName + " AS SELECT * FROM " + parentTableName;
+ conn.createStatement().execute(ddl);
+ try {
+ ddl = "ALTER TABLE " + parentTableName + " ADD col4 INTEGER";
+ conn.createStatement().execute(ddl);
+ fail("ALTER TABLE should not be allowed on parent table");
+ } catch (SQLException e) {
+ assertEquals(SQLExceptionCode.CANNOT_MUTATE_TABLE.getErrorCode(), e.getErrorCode());
+ }
+
+ // create child view from above view
+ ddl = "CREATE VIEW " + childViewName + "(col3 INTEGER) AS SELECT * FROM "
+ + parentViewName;
+ conn.createStatement().execute(ddl);
+ try {
+ ddl = "ALTER VIEW " + parentViewName + " ADD col4 INTEGER";
+ conn.createStatement().execute(ddl);
+ fail("ALTER VIEW should not be allowed on parent view");
+ } catch (SQLException e) {
+ assertEquals(SQLExceptionCode.CANNOT_MUTATE_TABLE.getErrorCode(), e.getErrorCode());
+ }
+
+ // alter child view with add column should be allowed
+ ddl = "ALTER VIEW " + childViewName + " ADD col4 INTEGER";
+ conn.createStatement().execute(ddl);
+ }
+ }
+
+ @Test
+ public void testDropColumnOnParentTableView() throws Exception {
+ try (Connection conn = DriverManager.getConnection(getJdbcUrl())) {
+ String parentTableName = SchemaUtil.getTableName(generateUniqueName(),
+ generateUniqueName());
+ String parentViewName = SchemaUtil.getTableName(generateUniqueName(),
+ generateUniqueName());
+ String childViewName = SchemaUtil.getTableName(generateUniqueName(),
+ generateUniqueName());
+ // create parent table
+ String ddl = "CREATE TABLE " + parentTableName
+ + " (col1 INTEGER NOT NULL, col2 INTEGER, col3 VARCHAR "
+ + "CONSTRAINT pk PRIMARY KEY (col1))";
+ conn.createStatement().execute(ddl);
+
+ // create view from table
+ ddl = "CREATE VIEW " + parentViewName + " AS SELECT * FROM " + parentTableName;
+ conn.createStatement().execute(ddl);
+ try {
+ ddl = "ALTER TABLE " + parentTableName + " DROP COLUMN col2";
+ conn.createStatement().execute(ddl);
+ fail("ALTER TABLE DROP COLUMN should not be allowed on parent table");
+ } catch (SQLException e) {
+ assertEquals(SQLExceptionCode.CANNOT_MUTATE_TABLE.getErrorCode(), e.getErrorCode());
+ }
+
+ // create child view from above view
+ ddl = "CREATE VIEW " + childViewName + "(col5 INTEGER) AS SELECT * FROM "
+ + parentViewName;
+ conn.createStatement().execute(ddl);
+ try {
+ ddl = "ALTER VIEW " + parentViewName + " DROP COLUMN col2";
+ conn.createStatement().execute(ddl);
+ fail("ALTER VIEW DROP COLUMN should not be allowed on parent view");
+ } catch (SQLException e) {
+ assertEquals(SQLExceptionCode.CANNOT_MUTATE_TABLE.getErrorCode(), e.getErrorCode());
+ }
+
+ // alter child view with drop column should be allowed
+ ddl = "ALTER VIEW " + childViewName + " DROP COLUMN col2";
+ conn.createStatement().execute(ddl);
+ }
+ }
+}
\ No newline at end of file
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 236b3f5..b3fa588 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
@@ -95,6 +95,7 @@ import java.util.HashSet;
import java.util.Iterator;
import java.util.List;
import java.util.NavigableMap;
+import java.util.Optional;
import java.util.Properties;
import java.util.Set;
@@ -2562,9 +2563,87 @@ public class MetaDataEndpointImpl extends MetaDataProtocol implements Coprocesso
EnvironmentEdgeManager.currentTimeMillis(), table, tableNamesToDelete, sharedTablesToDelete);
}
- private MetaDataMutationResult
- mutateColumn(List<Mutation> tableMetadata, ColumnMutator mutator, int clientVersion, PTable parentTable)
- throws IOException {
+ /**
+ * Validate if mutation is allowed on parent table/view
+ * based on their child views.
+ * If this method returns MetaDataMutationResult, mutation is not allowed,
+ * and returned object will contain returnCode
+ * (MutationCode) to indicate the underlying
+ * problem (validation failure code).
+ *
+ * @param expectedType expected type of PTable
+ * @param clientTimeStamp client timestamp, e.g check
+ * {@link MetaDataUtil#getClientTimeStamp(List)}
+ * @param tenantId tenant Id
+ * @param schemaName schema name
+ * @param tableName table name
+ * @param childViews child views of table or parent view.
+ * Usually this is an empty list
+ * passed to this method, and this method will add
+ * child views retrieved using
+ * {@link #findAllChildViews(long, int, byte[], byte[], byte[])}
+ * @param clientVersion client version, used to determine if
+ * mutation is allowed.
+ * @return Optional.empty() if mutation is allowed on parent
+ * table/view. If not allowed, returned Optional object
+ * will contain metaDataMutationResult with MutationCode.
+ * @throws IOException if something goes wrong while retrieving
+ * child views using
+ * {@link #findAllChildViews(long, int, byte[], byte[], byte[])}
+ * @throws SQLException if something goes wrong while retrieving
+ * child views using
+ * {@link #findAllChildViews(long, int, byte[], byte[], byte[])}
+ */
+ private Optional<MetaDataMutationResult> validateIfMutationAllowedOnParent(
+ final PTableType expectedType, final long clientTimeStamp,
+ final byte[] tenantId, final byte[] schemaName,
+ final byte[] tableName, final List<PTable> childViews,
+ final int clientVersion) throws IOException, SQLException {
+ boolean isMutationAllowed = true;
+ if (expectedType == PTableType.TABLE ||
+ expectedType == PTableType.VIEW) {
+ childViews.addAll(findAllChildViews(clientTimeStamp, clientVersion,
+ tenantId, schemaName, tableName));
+ if (!childViews.isEmpty()) {
+ // From 4.15 onwards we allow SYSTEM.CATALOG to split and no
+ // longer propagate parent
+ // metadata changes to child views.
+ // If the client is on a version older than 4.15 we have to
+ // block adding a column to a parent table/view as we no
+ // longer lock the parent table on the server side
+ // while creating a child view to prevent conflicting changes.
+ // This is handled on the client side from 4.15 onwards.
+ // Also if
+ // QueryServices.ALLOW_SPLITTABLE_SYSTEM_CATALOG_ROLLBACK is
+ // true, we block adding a column to a parent table/view so
+ // that we can rollback the upgrade if required.
+ if (clientVersion < MIN_SPLITTABLE_SYSTEM_CATALOG) {
+ isMutationAllowed = false;
+ LOGGER.error("Unable to add or drop a column as the " +
+ "client is older than {}",
+ MIN_SPLITTABLE_SYSTEM_CATALOG_VERSION);
+ } else if (allowSplittableSystemCatalogRollback) {
+ isMutationAllowed = false;
+ LOGGER.error("Unable to add or drop a column as the {} " +
+ "config is set to true",
+ QueryServices.ALLOW_SPLITTABLE_SYSTEM_CATALOG_ROLLBACK);
+ }
+ }
+ }
+ if (!isMutationAllowed) {
+ MetaDataMutationResult metaDataMutationResult =
+ new MetaDataMutationResult(
+ MetaDataProtocol.MutationCode.UNALLOWED_TABLE_MUTATION,
+ EnvironmentEdgeManager.currentTimeMillis(), null);
+ return Optional.of(metaDataMutationResult);
+ }
+ return Optional.empty();
+ }
+
+ private MetaDataMutationResult mutateColumn(
+ final List<Mutation> tableMetadata,
+ final ColumnMutator mutator, final int clientVersion,
+ final PTable parentTable) throws IOException {
byte[][] rowKeyMetaData = new byte[5][];
MetaDataUtil.getTenantIdAndSchemaAndTableName(tableMetadata, rowKeyMetaData);
byte[] tenantId = rowKeyMetaData[PhoenixDatabaseMetaData.TENANT_ID_INDEX];
@@ -2588,32 +2667,14 @@ public class MetaDataEndpointImpl extends MetaDataProtocol implements Coprocesso
List<RowLock> locks = Lists.newArrayList();
try {
- if (expectedType == PTableType.TABLE) {
- childViews = findAllChildViews(clientTimeStamp, clientVersion, tenantId,
- schemaName, tableName);
-
- if (!childViews.isEmpty()) {
- // From 4.15 onwards we allow SYSTEM.CATALOG to split and no longer propagate parent
- // metadata changes to child views.
- // If the client is on a version older than 4.15 we have to block adding a column to a
- // parent able as we no longer lock the parent table on the server side while creating a
- // child view to prevent conflicting changes. This is handled on the client side from
- // 4.15 onwards.
- // Also if QueryServices.ALLOW_SPLITTABLE_SYSTEM_CATALOG_ROLLBACK is true, we block adding
- // a column to a parent table so that we can rollback the upgrade if required.
- if (clientVersion < MIN_SPLITTABLE_SYSTEM_CATALOG) {
- LOGGER.error("Unable to add or drop a column as the client is older "
- + "than " + MIN_SPLITTABLE_SYSTEM_CATALOG_VERSION);
- return new MetaDataProtocol.MetaDataMutationResult(MetaDataProtocol.MutationCode.UNALLOWED_TABLE_MUTATION,
- EnvironmentEdgeManager.currentTimeMillis(), null);
- } else if (allowSplittableSystemCatalogRollback) {
- LOGGER.error("Unable to add or drop a column as the "
- + QueryServices.ALLOW_SPLITTABLE_SYSTEM_CATALOG_ROLLBACK
- + " config is set to true");
- return new MetaDataProtocol.MetaDataMutationResult(MetaDataProtocol.MutationCode.UNALLOWED_TABLE_MUTATION,
- EnvironmentEdgeManager.currentTimeMillis(), null);
- }
- }
+ Optional<MetaDataMutationResult> mutationResult =
+ validateIfMutationAllowedOnParent(expectedType,
+ clientTimeStamp, tenantId, schemaName, tableName,
+ childViews, clientVersion);
+ // only if mutation is allowed, we should get Optional.empty()
+ // here
+ if (mutationResult.isPresent()) {
+ return mutationResult.get();
}
acquireLock(region, key, locks);