Github user twdsilva commented on a diff in the pull request: https://github.com/apache/phoenix/pull/346#discussion_r220293666 --- Diff: phoenix-core/src/it/java/org/apache/phoenix/end2end/PropertiesInSyncIT.java --- @@ -0,0 +1,462 @@ +/* + * 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 org.apache.hadoop.hbase.KeepDeletedCells; +import org.apache.hadoop.hbase.TableName; +import org.apache.hadoop.hbase.client.Admin; +import org.apache.hadoop.hbase.client.ColumnFamilyDescriptor; +import org.apache.hadoop.hbase.client.ColumnFamilyDescriptorBuilder; +import org.apache.hadoop.hbase.client.TableDescriptor; +import org.apache.hadoop.hbase.client.TableDescriptorBuilder; +import org.apache.hadoop.hbase.util.Bytes; +import org.apache.phoenix.exception.SQLExceptionCode; +import org.apache.phoenix.jdbc.PhoenixConnection; +import org.apache.phoenix.schema.PTable; +import org.apache.phoenix.util.MetaDataUtil; +import org.apache.phoenix.util.PhoenixRuntime; +import org.apache.phoenix.util.SchemaUtil; +import org.junit.Test; + +import java.sql.Connection; +import java.sql.DriverManager; +import java.sql.SQLException; +import java.util.HashSet; +import java.util.Properties; +import java.util.Set; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.fail; +import static org.apache.phoenix.query.QueryConstants.DEFAULT_COLUMN_FAMILY_BYTES; +import static org.apache.phoenix.util.MetaDataUtil.PROPERTIES_TO_KEEP_IN_SYNC_AMONG_COL_FAMS_AND_INDEXES; +import static org.apache.phoenix.util.MetaDataUtil.VIEW_INDEX_TABLE_PREFIX; +import static org.apache.phoenix.util.UpgradeUtil.syncTableAndIndexProperties; + +/** + * Test properties that need to be kept in sync amongst all column families and indexes of a table + */ +public class PropertiesInSyncIT extends ParallelStatsDisabledIT { + private static final String COL_FAM1 = "CF1"; + private static final String COL_FAM2 = "CF2"; + private static final String NEW_CF = "NEW_CF"; + private static final String DUMMY_PROP_VALUE = "dummy"; + private static final int INITIAL_TTL_VALUE = 700; + private static final KeepDeletedCells INITIAL_KEEP_DELETED_CELLS_VALUE = KeepDeletedCells.TRUE; + private static final int INITIAL_REPLICATION_SCOPE_VALUE = 1; + private static final int MODIFIED_TTL_VALUE = INITIAL_TTL_VALUE + 300; + private static final KeepDeletedCells MODIFIED_KEEP_DELETED_CELLS_VALUE = + (INITIAL_KEEP_DELETED_CELLS_VALUE == KeepDeletedCells.TRUE) ? KeepDeletedCells.FALSE: KeepDeletedCells.TRUE; + private static final int MODIFIED_REPLICATION_SCOPE_VALUE = (INITIAL_REPLICATION_SCOPE_VALUE == 1) ? 0 : 1; + + + // Test that we disallow specifying synced properties to be set per column family when creating a table + @Test + public void testDisallowSyncedPropsToBeSetColFamSpecificCreateTable() throws Exception { + Connection conn = DriverManager.getConnection(getUrl(), new Properties()); + String tableName = generateUniqueName(); + for (String propName: PROPERTIES_TO_KEEP_IN_SYNC_AMONG_COL_FAMS_AND_INDEXES) { + try { + conn.createStatement().execute("create table " + tableName + + " (id INTEGER not null primary key, " + + COL_FAM1 + ".name varchar(10), " + COL_FAM2 + ".flag boolean) " + + COL_FAM1 + "." + propName + "=" + DUMMY_PROP_VALUE); + fail("Should fail with SQLException when setting synced property for a specific column family"); + } catch (SQLException sqlE) { + assertEquals("Should fail to set synced property for a specific column family", + SQLExceptionCode.COLUMN_FAMILY_NOT_ALLOWED_FOR_PROPERTY.getErrorCode(), sqlE.getErrorCode()); + } + } + conn.close(); + } + + // Test that all column families have the same value of synced properties when creating a table + @Test + public void testSyncedPropsAllColFamsCreateTable() throws Exception { + Connection conn = DriverManager.getConnection(getUrl(), new Properties()); + String tableName = createBaseTableWithProps(conn); + verifyHBaseColumnFamilyProperties(tableName, conn, false, false); + conn.close(); + } + + // Test that we disallow specifying synced properties to be set when creating an index on a physical table or a view + @Test + public void testDisallowSyncedPropsToBeSetCreateIndex() throws Exception { + Connection conn = DriverManager.getConnection(getUrl(), new Properties()); + String tableName = createBaseTableWithProps(conn); + String localIndexName = tableName + "_LOCAL_IDX"; + String globalIndexName = tableName + "_GLOBAL_IDX"; + String viewName = "VIEW_" + tableName; + conn.createStatement().execute("create view " + viewName + + " (new_col SMALLINT) as select * from " + tableName + " where id > 1"); + for (String propName: PROPERTIES_TO_KEEP_IN_SYNC_AMONG_COL_FAMS_AND_INDEXES) { + try { + conn.createStatement().execute("create local index " + localIndexName + + " on " + tableName + "(name) " + + propName + "=" + DUMMY_PROP_VALUE); + fail("Should fail with SQLException when setting synced property for a local index"); + } catch (SQLException sqlE) { + assertEquals("Should fail to set synced property for a local index", + SQLExceptionCode.CANNOT_SET_OR_ALTER_PROPERTY_FOR_INDEX.getErrorCode(), sqlE.getErrorCode()); + } + try { + conn.createStatement().execute("create index " + globalIndexName + + " on " + tableName + "(flag) " + + propName + "=" + DUMMY_PROP_VALUE); + fail("Should fail with SQLException when setting synced property for a global index"); + } catch (SQLException sqlE) { + assertEquals("Should fail to set synced property for a global index", + SQLExceptionCode.CANNOT_SET_OR_ALTER_PROPERTY_FOR_INDEX.getErrorCode(), sqlE.getErrorCode()); + } + try { + conn.createStatement().execute("create index view_index" + + " on " + viewName + " (flag)" + propName + "=" + DUMMY_PROP_VALUE); + fail("Should fail with SQLException when setting synced property for a view index"); + } catch (SQLException sqlE) { + assertEquals("Should fail to set synced property for a view index", + SQLExceptionCode.CANNOT_SET_OR_ALTER_PROPERTY_FOR_INDEX.getErrorCode(), sqlE.getErrorCode()); + } + } + conn.close(); + } + + // Test that indexes have the same value of synced properties as their base table + @Test + public void testSyncedPropsBaseTableCreateIndex() throws Exception { + Connection conn = DriverManager.getConnection(getUrl(), new Properties()); + String tableName = createBaseTableWithProps(conn); + createIndexTable(conn, tableName, PTable.IndexType.LOCAL); + String globalIndexName = createIndexTable(conn, tableName, PTable.IndexType.GLOBAL); + + // We pass the base table as the physical HBase table since our check includes checking the local index column family too + verifyHBaseColumnFamilyProperties(tableName, conn, false, false); + verifyHBaseColumnFamilyProperties(globalIndexName, conn, false, false); + conn.close(); + } + + // Test that the physical view index table has the same value of synced properties as its base table + @Test + public void testSyncedPropsBaseTableCreateViewIndex() throws Exception { + Connection conn = DriverManager.getConnection(getUrl(), new Properties()); + String tableName = createBaseTableWithProps(conn); + String viewIndexName = createIndexTable(conn, tableName, null); + + verifyHBaseColumnFamilyProperties(tableName, conn, false, false); + verifyHBaseColumnFamilyProperties(viewIndexName, conn, false, false); + conn.close(); + } + + // Test that we disallow specifying synced properties to be set per column family when altering a table + @Test + public void testDisallowSyncedPropsToBeSetColFamSpecificAlterTable() throws Exception { + Connection conn = DriverManager.getConnection(getUrl(), new Properties()); + String tableName = createBaseTableWithProps(conn); + StringBuilder alterAllSyncedPropsString = new StringBuilder(); + String modPropString = COL_FAM1 + ".%s=" + DUMMY_PROP_VALUE + ","; + for (String propName: PROPERTIES_TO_KEEP_IN_SYNC_AMONG_COL_FAMS_AND_INDEXES) { + try { + conn.createStatement().execute("alter table " + tableName + + " set " + COL_FAM1 + "." + propName + "=" + DUMMY_PROP_VALUE); + fail("Should fail with SQLException when altering synced property for a specific column family"); + } catch (SQLException sqlE) { + assertEquals("Should fail to alter synced property for a specific column family", + SQLExceptionCode.COLUMN_FAMILY_NOT_ALLOWED_FOR_PROPERTY.getErrorCode(), sqlE.getErrorCode()); + } + alterAllSyncedPropsString.append(String.format(modPropString, propName)); + } + + // Test the same when we try to set all of these properties at once + try { + conn.createStatement().execute("alter table " + tableName + " set " + + alterAllSyncedPropsString.substring(0, alterAllSyncedPropsString.length() - 1)); + fail("Should fail with SQLException when altering synced properties for a specific column family"); + } catch (SQLException sqlE) { + assertEquals("Should fail to alter synced properties for a specific column family", + SQLExceptionCode.COLUMN_FAMILY_NOT_ALLOWED_FOR_PROPERTY.getErrorCode(), sqlE.getErrorCode()); + } + conn.close(); + } + + // Test that any alteration of the synced properties gets propagated to all indexes and the physical view index table + @Test + public void testAlterSyncedPropsPropagateToAllIndexesAndViewIndex() throws Exception { + Connection conn = DriverManager.getConnection(getUrl(), new Properties()); + String tableName = createBaseTableWithProps(conn); + Set<String> tablesToCheck = new HashSet<>(); + tablesToCheck.add(tableName); + for (int i=0; i<2; i++) { + tablesToCheck.add(createIndexTable(conn, tableName, PTable.IndexType.LOCAL)); + tablesToCheck.add(createIndexTable(conn, tableName, PTable.IndexType.GLOBAL)); + } + // Create a view and view index + tablesToCheck.add(createIndexTable(conn, tableName, null)); + + // Now alter the base table's properties. This should get propagated to all index tables + conn.createStatement().execute("alter table " + tableName + " set TTL=" + MODIFIED_TTL_VALUE + + ",KEEP_DELETED_CELLS=" + MODIFIED_KEEP_DELETED_CELLS_VALUE + + ",REPLICATION_SCOPE=" + MODIFIED_REPLICATION_SCOPE_VALUE); + + for (String table: tablesToCheck) { + verifyHBaseColumnFamilyProperties(table, conn, true, false); + } + + // Any indexes created henceforth should have the modified properties + String newGlobalIndex = createIndexTable(conn, tableName, PTable.IndexType.GLOBAL); + String newViewIndex = createIndexTable(conn, tableName, null); + verifyHBaseColumnFamilyProperties(newGlobalIndex, conn, true, false); + verifyHBaseColumnFamilyProperties(newViewIndex, conn, true, false); + conn.close(); + } + + // Test that any if we add a column family to a base table, it gets the synced properties + @Test + public void testAlterTableAddColumnFamilyGetsSyncedProps() throws Exception { + Connection conn = DriverManager.getConnection(getUrl(), new Properties()); + String tableName = createBaseTableWithProps(conn); + + // Test that we are not allowed to set any property to be kept in sync, specific to the new column family to be added + for (String propName: PROPERTIES_TO_KEEP_IN_SYNC_AMONG_COL_FAMS_AND_INDEXES) { + try { + conn.createStatement().execute( + "alter table " + tableName + " add " + NEW_CF + ".new_column varchar(2) " + + NEW_CF + "." + propName + "=" + DUMMY_PROP_VALUE); + fail("Should fail with SQLException when altering synced property for a specific column family when adding a column"); + } catch (SQLException sqlE) { + assertEquals("Should fail to alter synced property for a specific column family when adding a column", + SQLExceptionCode.COLUMN_FAMILY_NOT_ALLOWED_FOR_PROPERTY.getErrorCode(), sqlE.getErrorCode()); + } + } + + // Test that when we add a new column (belonging to a new column family) and set any property that should be + // in sync, then the property is modified for all existing column families of the base table and its indexes + Set<String> tablesToCheck = new HashSet<>(); + tablesToCheck.add(tableName); + for (int i=0; i<2; i++) { + tablesToCheck.add(createIndexTable(conn, tableName, PTable.IndexType.LOCAL)); + tablesToCheck.add(createIndexTable(conn, tableName, PTable.IndexType.GLOBAL)); + } + // Create a view and view index + tablesToCheck.add(createIndexTable(conn, tableName, null)); + + // Now add a new column family while simultaneously modifying properties to be kept in sync, + // as well as a property which does not need to be kept in sync. Properties to be kept in sync + // should get propagated to all index tables and already existing column families + conn.createStatement().execute( + "alter table " + tableName + " add " + NEW_CF + ".new_column varchar(2) " + + "KEEP_DELETED_CELLS=" + MODIFIED_KEEP_DELETED_CELLS_VALUE + + ",REPLICATION_SCOPE=" + MODIFIED_REPLICATION_SCOPE_VALUE + + ",BLOCKSIZE=300000"); + + for (String table: tablesToCheck) { + verifyHBaseColumnFamilyProperties(table, conn, true, true); + } + try (Admin admin = conn.unwrap(PhoenixConnection.class).getQueryServices().getAdmin()) { + ColumnFamilyDescriptor[] columnFamilies = admin.getDescriptor(TableName.valueOf(tableName)) + .getColumnFamilies(); + for (ColumnFamilyDescriptor cfd: columnFamilies) { + if (cfd.getNameAsString().equals(NEW_CF)) { + assertEquals("Newly added column fmaily should have updated property", + 300000, cfd.getBlocksize()); + } else { + assertEquals("Existing column families should have default value for property", + ColumnFamilyDescriptorBuilder.DEFAULT_BLOCKSIZE, cfd.getBlocksize()); + } + } + } + conn.close(); + } + + // Test that we disallow altering a synced property for a global index table + @Test + public void testDisallowAlterGlobalIndexTable() throws Exception { + Connection conn = DriverManager.getConnection(getUrl(), new Properties()); + String tableName = createBaseTableWithProps(conn); + String globalIndexName = createIndexTable(conn, tableName, PTable.IndexType.GLOBAL); + for (String propName: PROPERTIES_TO_KEEP_IN_SYNC_AMONG_COL_FAMS_AND_INDEXES) { + try { + conn.createStatement().execute("alter table " + globalIndexName + " set " + + propName + "=" + DUMMY_PROP_VALUE); + fail("Should fail with SQLException when altering synced property for a global index"); + } catch (SQLException sqlE) { + assertEquals("Should fail to alter synced property for a global index", + SQLExceptionCode.CANNOT_SET_OR_ALTER_PROPERTY_FOR_INDEX.getErrorCode(), sqlE.getErrorCode()); + } + } + conn.close(); + } + + // Test the upgrade code path for old client to new phoenix server cases in which the client + // may have tables which have column families and indexes whose properties are out of sync + @Test + public void testOldClientSyncPropsUpgradePath() throws Exception { + Connection conn = DriverManager.getConnection(getUrl(), new Properties()); + + String baseTableName = createBaseTableWithProps(conn); --- End diff -- Do you need to test for both baseTableName and baseTableName1?
---