Github user ChinmaySKulkarni commented on a diff in the pull request:
https://github.com/apache/phoenix/pull/346#discussion_r220352602
--- 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 --
It's not necessary. I just wanted to make sure the upgrade path works fine
for multiple base tables
---
