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?


---

Reply via email to