You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@phoenix.apache.org by GitBox <gi...@apache.org> on 2020/07/29 08:00:59 UTC

[GitHub] [phoenix] Qinrui98 opened a new pull request #848: Phoenix-5947 Add tests and extensions to Schema Extraction Tool utility

Qinrui98 opened a new pull request #848:
URL: https://github.com/apache/phoenix/pull/848


   1. Added support for extracting ddl for salted tables, array column type and non-default column families
   2. Added tests for extracting table, view and index ddls


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [phoenix] swaroopak merged pull request #848: Phoenix-5947 Add tests and extensions to Schema Extraction Tool utility

Posted by GitBox <gi...@apache.org>.
swaroopak merged pull request #848:
URL: https://github.com/apache/phoenix/pull/848


   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [phoenix] swaroopak commented on a change in pull request #848: Phoenix-5947 Add tests and extensions to Schema Extraction Tool utility

Posted by GitBox <gi...@apache.org>.
swaroopak commented on a change in pull request #848:
URL: https://github.com/apache/phoenix/pull/848#discussion_r464711039



##########
File path: phoenix-tools/src/it/java/org/apache/phoenix/schema/SchemaExtractionToolIT.java
##########
@@ -58,156 +130,251 @@ public void testCreateTableStatement_tenant() throws Exception {
         String viewName = generateUniqueName();
         String schemaName = generateUniqueName();
         String tenantId = "abc";
-        Properties props = PropertiesUtil.deepCopy(TEST_PROPERTIES);
         String pTableFullName = SchemaUtil.getQualifiedTableName(schemaName, tableName);
-        String properties = "TTL=2592000,IMMUTABLE_ROWS=true,DISABLE_WAL=true";
-        SchemaExtractionTool set;
+        String createTableStmt = "CREATE TABLE "+pTableFullName + "(k BIGINT NOT NULL PRIMARY KEY, "
+                + "v1 VARCHAR, v2 VARCHAR)";
         String viewFullName = SchemaUtil.getQualifiedTableName(schemaName, viewName);
-        String createView = "CREATE VIEW "+viewFullName + "(id1 BIGINT, id2 BIGINT NOT NULL, "
+        String createViewStmt = "CREATE VIEW "+viewFullName + "(id1 BIGINT, id2 BIGINT NOT NULL, "
                 + "id3 VARCHAR NOT NULL CONSTRAINT PKVIEW PRIMARY KEY (id2, id3 DESC)) "
                 + "AS SELECT * FROM "+pTableFullName;
 
-        try (Connection conn = DriverManager.getConnection(getUrl(), props)) {
-
-            conn.createStatement().execute("CREATE TABLE "+pTableFullName + "(k BIGINT NOT NULL PRIMARY KEY, "
-                    + "v1 VARCHAR, v2 VARCHAR)"
-                    + properties);
-            set = new SchemaExtractionTool();
-            set.setConf(conn.unwrap(PhoenixConnection.class).getQueryServices().getConfiguration());
-            conn.commit();
-        }
-        try (Connection conn = getTenantConnection(getUrl(), tenantId)) {
-            conn.createStatement().execute(createView);
-            conn.commit();
-        }
-        String [] args = {"-tb", viewName, "-s", schemaName, "-t", tenantId};
-        set.run(args);
-        Assert.assertEquals(createView.toUpperCase(), set.getOutput().toUpperCase());
+        List<String> queries1 = new ArrayList<String>(){};
+        queries1.add(createTableStmt);
+        runSchemaExtractionTool(schemaName, tableName, null, queries1);
+        List<String> queries2 = new ArrayList<String>();
+        queries2.add(createViewStmt);
+        String result2 = runSchemaExtractionTool(schemaName, viewName, tenantId, queries2);
+        Assert.assertEquals(createViewStmt.toUpperCase(), result2.toUpperCase());
     }
 
-    private Connection getTenantConnection(String url, String tenantId) throws SQLException {
-        Properties props = new Properties();
-        props.setProperty(PhoenixRuntime.TENANT_ID_ATTRIB, tenantId);
-        return DriverManager.getConnection(url, props);
+    @Test
+    public void testSaltedTableStatement() throws Exception {
+        String tableName = generateUniqueName();
+        String schemaName = generateUniqueName();
+        String pTableFullName = SchemaUtil.getQualifiedTableName(schemaName, tableName);
+        String query = "create table " + pTableFullName +
+                "(a_integer integer not null CONSTRAINT pk PRIMARY KEY (a_integer)) SALT_BUCKETS=16";
+        List<String> queries = new ArrayList<String>(){};
+        queries.add(query);
+        String result = runSchemaExtractionTool(schemaName, tableName, null, queries);
+        Assert.assertTrue(getProperties(result).contains("SALT_BUCKETS=16"));
     }
 
     @Test
-    public void testCreateIndexStatement() throws Exception {
+    public void testCreateTableWithPKConstraint() throws Exception {
         String tableName = generateUniqueName();
         String schemaName = generateUniqueName();
-        String indexName = generateUniqueName();
-        String indexName1 = generateUniqueName();
-        String indexName2 = generateUniqueName();
-        Properties props = PropertiesUtil.deepCopy(TEST_PROPERTIES);
-        String properties = "TTL=2592000,IMMUTABLE_ROWS=true,DISABLE_WAL=true";
-
-        try (Connection conn = DriverManager.getConnection(getUrl(), props)) {
-
-            String pTableFullName = SchemaUtil.getQualifiedTableName(schemaName, tableName);
-            conn.createStatement().execute("CREATE TABLE "+pTableFullName + "(k VARCHAR NOT NULL PRIMARY KEY, v1 VARCHAR, v2 VARCHAR)"
-                    + properties);
-
-            String createIndexStatement = "CREATE INDEX "+indexName + " ON "+pTableFullName+"(v1 DESC) INCLUDE (v2)";
-
-            String createIndexStatement1 = "CREATE INDEX "+indexName1 + " ON "+pTableFullName+"(v2 DESC) INCLUDE (v1)";
-
-            String createIndexStatement2 = "CREATE INDEX "+indexName2 + " ON "+pTableFullName+"(k)";
-
-            conn.createStatement().execute(createIndexStatement);
-            conn.createStatement().execute(createIndexStatement1);
-            conn.createStatement().execute(createIndexStatement2);
-            conn.commit();
-            SchemaExtractionTool set = new SchemaExtractionTool();
-            set.setConf(conn.unwrap(PhoenixConnection.class).getQueryServices().getConfiguration());
-
-            String [] args = {"-tb", indexName, "-s", schemaName};
-            set.run(args);
-            Assert.assertEquals(createIndexStatement.toUpperCase(), set.getOutput().toUpperCase());
-
-            String [] args1 = {"-tb", indexName1, "-s", schemaName};
-            set.run(args1);
-            Assert.assertEquals(createIndexStatement1.toUpperCase(), set.getOutput().toUpperCase());
-
-            String [] args2 = {"-tb", indexName2, "-s", schemaName};
-            set.run(args2);
-            Assert.assertEquals(createIndexStatement2.toUpperCase(), set.getOutput().toUpperCase());
-        }
+        String pTableFullName = SchemaUtil.getQualifiedTableName(schemaName, tableName);
+        String query = "create table " + pTableFullName +
+                "(a_char CHAR(15) NOT NULL, " +
+                "b_char CHAR(15) NOT NULL, " +
+                "c_bigint BIGINT NOT NULL CONSTRAINT PK PRIMARY KEY (a_char, b_char, c_bigint)) IMMUTABLE_ROWS=TRUE";
+        List<String> queries = new ArrayList<String>(){};
+        queries.add(query);
+        String result = runSchemaExtractionTool(schemaName, tableName, null, queries);
+        Assert.assertEquals(query.toUpperCase(), result.toUpperCase());
     }
 
     @Test
-    public void testCreateViewStatement() throws Exception {
+    public void testCreateTableWithArrayColumn() throws Exception {
         String tableName = generateUniqueName();
         String schemaName = generateUniqueName();
-        String viewName = generateUniqueName();
-        Properties props = PropertiesUtil.deepCopy(TEST_PROPERTIES);
-        String properties = "TTL=2592000,IMMUTABLE_ROWS=true,DISABLE_WAL=true";
+        String pTableFullName = SchemaUtil.getQualifiedTableName(schemaName, tableName);
+        String query = "create table " + pTableFullName +
+                "(a_char CHAR(15) NOT NULL, " +
+                "b_char CHAR(10) NOT NULL, " +
+                "c_var_array VARCHAR ARRAY, " +
+                "d_char_array CHAR(15) ARRAY[3] CONSTRAINT PK PRIMARY KEY (a_char, b_char)) " +
+                "TTL=2592000, IMMUTABLE_STORAGE_SCHEME=ONE_CELL_PER_COLUMN, REPLICATION_SCOPE=1";
+        List<String> queries = new ArrayList<String>(){};
+        queries.add(query);
+        String result = runSchemaExtractionTool(schemaName, tableName, null, queries);
+        Assert.assertEquals(query.toUpperCase(), result.toUpperCase());
+    }
 
-        try (Connection conn = DriverManager.getConnection(getUrl(), props)) {
+    @Test
+    public void testCreateTableWithNonDefaultColumnFamily() throws Exception {
+        String tableName = generateUniqueName();
+        String schemaName = generateUniqueName();
+        String pTableFullName = SchemaUtil.getQualifiedTableName(schemaName, tableName);
+        String query = "create table " + pTableFullName +
+                "(a_char CHAR(15) NOT NULL, " +
+                "b_char CHAR(10) NOT NULL, " +
+                "\"av\".\"_\" CHAR(1), " +
+                "\"bv\".\"_\" CHAR(1), " +
+                "\"cv\".\"_\" CHAR(1), " +
+                "\"dv\".\"_\" CHAR(1) CONSTRAINT PK PRIMARY KEY (a_char, b_char)) " +
+                "TTL=1209600, IMMUTABLE_ROWS=true, IMMUTABLE_STORAGE_SCHEME=ONE_CELL_PER_COLUMN, SALT_BUCKETS=16, MULTI_TENANT=true";
+        List<String> queries = new ArrayList<String>(){};
+        queries.add(query);
+        String result = runSchemaExtractionTool(schemaName, tableName, null, queries);
+        Assert.assertEquals(query.toUpperCase(), result.toUpperCase());
+    }
 
-            String pTableFullName = SchemaUtil.getQualifiedTableName(schemaName, tableName);
-            conn.createStatement().execute("CREATE TABLE "+pTableFullName + "(k BIGINT NOT NULL PRIMARY KEY, "
-                    + "v1 VARCHAR, v2 VARCHAR)"
-                    + properties);
-            String viewFullName = SchemaUtil.getQualifiedTableName(schemaName, viewName);
-            String viewFullName1 = SchemaUtil.getQualifiedTableName(schemaName, viewName+"1");
+    @Test
+    public void testCreateTableWithUniversalCFProperties() throws Exception {

Review comment:
       I feel this and testCreateTableWithNonDefaultColumnFamily can be clubbed into one test?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [phoenix] swaroopak commented on a change in pull request #848: Phoenix-5947 Add tests and extensions to Schema Extraction Tool utility

Posted by GitBox <gi...@apache.org>.
swaroopak commented on a change in pull request #848:
URL: https://github.com/apache/phoenix/pull/848#discussion_r464711255



##########
File path: phoenix-tools/src/it/java/org/apache/phoenix/schema/SchemaExtractionToolIT.java
##########
@@ -58,156 +130,251 @@ public void testCreateTableStatement_tenant() throws Exception {
         String viewName = generateUniqueName();
         String schemaName = generateUniqueName();
         String tenantId = "abc";
-        Properties props = PropertiesUtil.deepCopy(TEST_PROPERTIES);
         String pTableFullName = SchemaUtil.getQualifiedTableName(schemaName, tableName);
-        String properties = "TTL=2592000,IMMUTABLE_ROWS=true,DISABLE_WAL=true";
-        SchemaExtractionTool set;
+        String createTableStmt = "CREATE TABLE "+pTableFullName + "(k BIGINT NOT NULL PRIMARY KEY, "
+                + "v1 VARCHAR, v2 VARCHAR)";
         String viewFullName = SchemaUtil.getQualifiedTableName(schemaName, viewName);
-        String createView = "CREATE VIEW "+viewFullName + "(id1 BIGINT, id2 BIGINT NOT NULL, "
+        String createViewStmt = "CREATE VIEW "+viewFullName + "(id1 BIGINT, id2 BIGINT NOT NULL, "
                 + "id3 VARCHAR NOT NULL CONSTRAINT PKVIEW PRIMARY KEY (id2, id3 DESC)) "
                 + "AS SELECT * FROM "+pTableFullName;
 
-        try (Connection conn = DriverManager.getConnection(getUrl(), props)) {
-
-            conn.createStatement().execute("CREATE TABLE "+pTableFullName + "(k BIGINT NOT NULL PRIMARY KEY, "
-                    + "v1 VARCHAR, v2 VARCHAR)"
-                    + properties);
-            set = new SchemaExtractionTool();
-            set.setConf(conn.unwrap(PhoenixConnection.class).getQueryServices().getConfiguration());
-            conn.commit();
-        }
-        try (Connection conn = getTenantConnection(getUrl(), tenantId)) {
-            conn.createStatement().execute(createView);
-            conn.commit();
-        }
-        String [] args = {"-tb", viewName, "-s", schemaName, "-t", tenantId};
-        set.run(args);
-        Assert.assertEquals(createView.toUpperCase(), set.getOutput().toUpperCase());
+        List<String> queries1 = new ArrayList<String>(){};
+        queries1.add(createTableStmt);
+        runSchemaExtractionTool(schemaName, tableName, null, queries1);
+        List<String> queries2 = new ArrayList<String>();
+        queries2.add(createViewStmt);
+        String result2 = runSchemaExtractionTool(schemaName, viewName, tenantId, queries2);
+        Assert.assertEquals(createViewStmt.toUpperCase(), result2.toUpperCase());
     }
 
-    private Connection getTenantConnection(String url, String tenantId) throws SQLException {
-        Properties props = new Properties();
-        props.setProperty(PhoenixRuntime.TENANT_ID_ATTRIB, tenantId);
-        return DriverManager.getConnection(url, props);
+    @Test
+    public void testSaltedTableStatement() throws Exception {
+        String tableName = generateUniqueName();
+        String schemaName = generateUniqueName();
+        String pTableFullName = SchemaUtil.getQualifiedTableName(schemaName, tableName);
+        String query = "create table " + pTableFullName +
+                "(a_integer integer not null CONSTRAINT pk PRIMARY KEY (a_integer)) SALT_BUCKETS=16";
+        List<String> queries = new ArrayList<String>(){};
+        queries.add(query);
+        String result = runSchemaExtractionTool(schemaName, tableName, null, queries);
+        Assert.assertTrue(getProperties(result).contains("SALT_BUCKETS=16"));
     }
 
     @Test
-    public void testCreateIndexStatement() throws Exception {
+    public void testCreateTableWithPKConstraint() throws Exception {
         String tableName = generateUniqueName();
         String schemaName = generateUniqueName();
-        String indexName = generateUniqueName();
-        String indexName1 = generateUniqueName();
-        String indexName2 = generateUniqueName();
-        Properties props = PropertiesUtil.deepCopy(TEST_PROPERTIES);
-        String properties = "TTL=2592000,IMMUTABLE_ROWS=true,DISABLE_WAL=true";
-
-        try (Connection conn = DriverManager.getConnection(getUrl(), props)) {
-
-            String pTableFullName = SchemaUtil.getQualifiedTableName(schemaName, tableName);
-            conn.createStatement().execute("CREATE TABLE "+pTableFullName + "(k VARCHAR NOT NULL PRIMARY KEY, v1 VARCHAR, v2 VARCHAR)"
-                    + properties);
-
-            String createIndexStatement = "CREATE INDEX "+indexName + " ON "+pTableFullName+"(v1 DESC) INCLUDE (v2)";
-
-            String createIndexStatement1 = "CREATE INDEX "+indexName1 + " ON "+pTableFullName+"(v2 DESC) INCLUDE (v1)";
-
-            String createIndexStatement2 = "CREATE INDEX "+indexName2 + " ON "+pTableFullName+"(k)";
-
-            conn.createStatement().execute(createIndexStatement);
-            conn.createStatement().execute(createIndexStatement1);
-            conn.createStatement().execute(createIndexStatement2);
-            conn.commit();
-            SchemaExtractionTool set = new SchemaExtractionTool();
-            set.setConf(conn.unwrap(PhoenixConnection.class).getQueryServices().getConfiguration());
-
-            String [] args = {"-tb", indexName, "-s", schemaName};
-            set.run(args);
-            Assert.assertEquals(createIndexStatement.toUpperCase(), set.getOutput().toUpperCase());
-
-            String [] args1 = {"-tb", indexName1, "-s", schemaName};
-            set.run(args1);
-            Assert.assertEquals(createIndexStatement1.toUpperCase(), set.getOutput().toUpperCase());
-
-            String [] args2 = {"-tb", indexName2, "-s", schemaName};
-            set.run(args2);
-            Assert.assertEquals(createIndexStatement2.toUpperCase(), set.getOutput().toUpperCase());
-        }
+        String pTableFullName = SchemaUtil.getQualifiedTableName(schemaName, tableName);
+        String query = "create table " + pTableFullName +
+                "(a_char CHAR(15) NOT NULL, " +
+                "b_char CHAR(15) NOT NULL, " +
+                "c_bigint BIGINT NOT NULL CONSTRAINT PK PRIMARY KEY (a_char, b_char, c_bigint)) IMMUTABLE_ROWS=TRUE";
+        List<String> queries = new ArrayList<String>(){};
+        queries.add(query);
+        String result = runSchemaExtractionTool(schemaName, tableName, null, queries);
+        Assert.assertEquals(query.toUpperCase(), result.toUpperCase());
     }
 
     @Test
-    public void testCreateViewStatement() throws Exception {
+    public void testCreateTableWithArrayColumn() throws Exception {
         String tableName = generateUniqueName();
         String schemaName = generateUniqueName();
-        String viewName = generateUniqueName();
-        Properties props = PropertiesUtil.deepCopy(TEST_PROPERTIES);
-        String properties = "TTL=2592000,IMMUTABLE_ROWS=true,DISABLE_WAL=true";
+        String pTableFullName = SchemaUtil.getQualifiedTableName(schemaName, tableName);
+        String query = "create table " + pTableFullName +
+                "(a_char CHAR(15) NOT NULL, " +
+                "b_char CHAR(10) NOT NULL, " +
+                "c_var_array VARCHAR ARRAY, " +
+                "d_char_array CHAR(15) ARRAY[3] CONSTRAINT PK PRIMARY KEY (a_char, b_char)) " +
+                "TTL=2592000, IMMUTABLE_STORAGE_SCHEME=ONE_CELL_PER_COLUMN, REPLICATION_SCOPE=1";
+        List<String> queries = new ArrayList<String>(){};
+        queries.add(query);
+        String result = runSchemaExtractionTool(schemaName, tableName, null, queries);
+        Assert.assertEquals(query.toUpperCase(), result.toUpperCase());
+    }
 
-        try (Connection conn = DriverManager.getConnection(getUrl(), props)) {
+    @Test
+    public void testCreateTableWithNonDefaultColumnFamily() throws Exception {
+        String tableName = generateUniqueName();
+        String schemaName = generateUniqueName();
+        String pTableFullName = SchemaUtil.getQualifiedTableName(schemaName, tableName);
+        String query = "create table " + pTableFullName +
+                "(a_char CHAR(15) NOT NULL, " +
+                "b_char CHAR(10) NOT NULL, " +
+                "\"av\".\"_\" CHAR(1), " +
+                "\"bv\".\"_\" CHAR(1), " +
+                "\"cv\".\"_\" CHAR(1), " +
+                "\"dv\".\"_\" CHAR(1) CONSTRAINT PK PRIMARY KEY (a_char, b_char)) " +
+                "TTL=1209600, IMMUTABLE_ROWS=true, IMMUTABLE_STORAGE_SCHEME=ONE_CELL_PER_COLUMN, SALT_BUCKETS=16, MULTI_TENANT=true";
+        List<String> queries = new ArrayList<String>(){};
+        queries.add(query);
+        String result = runSchemaExtractionTool(schemaName, tableName, null, queries);
+        Assert.assertEquals(query.toUpperCase(), result.toUpperCase());
+    }
 
-            String pTableFullName = SchemaUtil.getQualifiedTableName(schemaName, tableName);
-            conn.createStatement().execute("CREATE TABLE "+pTableFullName + "(k BIGINT NOT NULL PRIMARY KEY, "
-                    + "v1 VARCHAR, v2 VARCHAR)"
-                    + properties);
-            String viewFullName = SchemaUtil.getQualifiedTableName(schemaName, viewName);
-            String viewFullName1 = SchemaUtil.getQualifiedTableName(schemaName, viewName+"1");
+    @Test
+    public void testCreateTableWithUniversalCFProperties() throws Exception {
+        String tableName = generateUniqueName();
+        String schemaName = generateUniqueName();
+        String properties = "KEEP_DELETED_CELLS=TRUE, TTL=1209600, IMMUTABLE_STORAGE_SCHEME=ONE_CELL_PER_COLUMN, REPLICATION_SCOPE=1";
+        String pTableFullName = SchemaUtil.getQualifiedTableName(schemaName, tableName);
+        String query = "create table " + pTableFullName +
+                "(a_char CHAR(15) NOT NULL, " +
+                "b_char CHAR(10) NOT NULL, " +
+                "\"av\".\"_\" CHAR(1), " +
+                "\"bv\".\"_\" CHAR(1), " +
+                "\"cv\".\"_\" CHAR(1), " +
+                "\"dv\".\"_\" CHAR(1) CONSTRAINT PK PRIMARY KEY (a_char, b_char)) " + properties;
+        List<String> queries = new ArrayList<String>(){};
+        queries.add(query);
+        String result = runSchemaExtractionTool(schemaName, tableName, null, queries);
+        Assert.assertEquals(query.toUpperCase(), result.toUpperCase());
+    }
 
+    @Test
+    public void testCreateTableWithDefaultCFProperties() throws Exception {

Review comment:
       even this can be clubbed with above one as there is only one extra property here. 




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [phoenix] Qinrui98 commented on a change in pull request #848: Phoenix-5947 Add tests and extensions to Schema Extraction Tool utility

Posted by GitBox <gi...@apache.org>.
Qinrui98 commented on a change in pull request #848:
URL: https://github.com/apache/phoenix/pull/848#discussion_r463158802



##########
File path: phoenix-tools/src/main/java/org/apache/phoenix/schema/SchemaExtractionProcessor.java
##########
@@ -250,12 +247,31 @@ private void setHTableProperties(HTableDescriptor htd) {
         }
     }
 
-    private void setHColumnFamilyProperties(HColumnDescriptor columnDescriptor) {
-        Map<ImmutableBytesWritable, ImmutableBytesWritable> propsMap = columnDescriptor.getValues();
-        for (Map.Entry<ImmutableBytesWritable, ImmutableBytesWritable> entry : propsMap.entrySet()) {
+    private void setHColumnFamilyProperties(HColumnDescriptor[] columnDescriptors) {
+        Map<ImmutableBytesWritable, ImmutableBytesWritable> propsMap = columnDescriptors[0].getValues();
+        for(Map.Entry<ImmutableBytesWritable, ImmutableBytesWritable> entry : propsMap.entrySet()){
             ImmutableBytesWritable key = entry.getKey();
-            ImmutableBytesWritable value = entry.getValue();
-            definedProps.put(Bytes.toString(key.get()), Bytes.toString(value.get()));
+            ImmutableBytesWritable defaultValue = entry.getValue();
+            Map<String, String> CFMap = new HashMap<String, String>();
+            Set<ImmutableBytesWritable> set = new HashSet<ImmutableBytesWritable>();
+            for(HColumnDescriptor columnDescriptor: columnDescriptors){
+                String columnFamilyName = Bytes.toString(columnDescriptor.getName());
+                ImmutableBytesWritable value = columnDescriptor.getValues().get(key);
+                // check if it is universal properties
+                if (SYNCED_DATA_TABLE_AND_INDEX_COL_FAM_PROPERTIES.contains(Bytes.toString(key.get()))){
+                    definedProps.put(Bytes.toString(key.get()), Bytes.toString(value.get()));
+                    continue;
+                }
+                CFMap.put(columnFamilyName, Bytes.toString(value.get()));
+                set.add(value);
+            }
+            if (set.size() > 1){

Review comment:
       Yes it's working with only 2 of them having VERSIONS=2. The size of the set will be 2 in this case. I'll add a test to cover this scenario. 
   




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [phoenix] Qinrui98 commented on a change in pull request #848: Phoenix-5947 Add tests and extensions to Schema Extraction Tool utility

Posted by GitBox <gi...@apache.org>.
Qinrui98 commented on a change in pull request #848:
URL: https://github.com/apache/phoenix/pull/848#discussion_r463285826



##########
File path: phoenix-tools/src/it/java/org/apache/phoenix/schema/SchemaExtractionToolIT.java
##########
@@ -210,4 +204,242 @@ public void testCreateViewIndexStatement() throws Exception {
             Assert.assertEquals(createIndexStatement.toUpperCase(), set.getOutput().toUpperCase());
         }
     }
+
+    @Test
+    public void testSaltedTableStatement() throws Exception {
+        String tableName = generateUniqueName();
+        String schemaName = generateUniqueName();
+        Properties props = PropertiesUtil.deepCopy(TEST_PROPERTIES);
+
+        try (Connection conn = DriverManager.getConnection(getUrl(), props)) {
+            String pTableFullName = SchemaUtil.getQualifiedTableName(schemaName, tableName);
+            String query = "create table " + pTableFullName +
+                    "(a_integer integer not null CONSTRAINT pk PRIMARY KEY (a_integer)) SALT_BUCKETS=16";
+            conn.createStatement().execute(query);
+            conn.commit();
+            String [] args = {"-tb", tableName, "-s", schemaName};
+            SchemaExtractionTool set = new SchemaExtractionTool();
+            set.setConf(conn.unwrap(PhoenixConnection.class).getQueryServices().getConfiguration());
+            set.run(args);
+            String actualProperties = set.getOutput().substring(set.getOutput().lastIndexOf(")")+1);
+            Assert.assertEquals(true, actualProperties.contains("SALT_BUCKETS=16"));
+        }
+    }
+
+    @Test
+    public void testCreateTableWithPKConstraint() throws Exception {
+        String tableName = generateUniqueName();
+        String schemaName = generateUniqueName();
+        Properties props = PropertiesUtil.deepCopy(TEST_PROPERTIES);

Review comment:
       Yes I'll extract the common part into a helper function and avoid duplicated code here. 




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [phoenix] gjacoby126 commented on a change in pull request #848: Phoenix-5947 Add tests and extensions to Schema Extraction Tool utility

Posted by GitBox <gi...@apache.org>.
gjacoby126 commented on a change in pull request #848:
URL: https://github.com/apache/phoenix/pull/848#discussion_r463259391



##########
File path: phoenix-tools/src/it/java/org/apache/phoenix/schema/SchemaExtractionToolIT.java
##########
@@ -210,4 +204,242 @@ public void testCreateViewIndexStatement() throws Exception {
             Assert.assertEquals(createIndexStatement.toUpperCase(), set.getOutput().toUpperCase());
         }
     }
+
+    @Test
+    public void testSaltedTableStatement() throws Exception {
+        String tableName = generateUniqueName();
+        String schemaName = generateUniqueName();
+        Properties props = PropertiesUtil.deepCopy(TEST_PROPERTIES);
+
+        try (Connection conn = DriverManager.getConnection(getUrl(), props)) {
+            String pTableFullName = SchemaUtil.getQualifiedTableName(schemaName, tableName);
+            String query = "create table " + pTableFullName +
+                    "(a_integer integer not null CONSTRAINT pk PRIMARY KEY (a_integer)) SALT_BUCKETS=16";
+            conn.createStatement().execute(query);
+            conn.commit();
+            String [] args = {"-tb", tableName, "-s", schemaName};
+            SchemaExtractionTool set = new SchemaExtractionTool();
+            set.setConf(conn.unwrap(PhoenixConnection.class).getQueryServices().getConfiguration());
+            set.run(args);
+            String actualProperties = set.getOutput().substring(set.getOutput().lastIndexOf(")")+1);
+            Assert.assertEquals(true, actualProperties.contains("SALT_BUCKETS=16"));
+        }
+    }
+
+    @Test
+    public void testCreateTableWithPKConstraint() throws Exception {
+        String tableName = generateUniqueName();
+        String schemaName = generateUniqueName();
+        Properties props = PropertiesUtil.deepCopy(TEST_PROPERTIES);

Review comment:
       Several of these tests are of the form: 
   1. Create a table according to some SQL and table name
   2. Run SchemaExtractionTool
   3. Assert that output is equal to the original SQL. 
   
   Can we extract that logic into a helper function and you just pass in different tablename and SQL in each test?

##########
File path: phoenix-tools/src/it/java/org/apache/phoenix/schema/SchemaExtractionToolIT.java
##########
@@ -210,4 +204,242 @@ public void testCreateViewIndexStatement() throws Exception {
             Assert.assertEquals(createIndexStatement.toUpperCase(), set.getOutput().toUpperCase());
         }
     }
+
+    @Test
+    public void testSaltedTableStatement() throws Exception {
+        String tableName = generateUniqueName();
+        String schemaName = generateUniqueName();
+        Properties props = PropertiesUtil.deepCopy(TEST_PROPERTIES);
+
+        try (Connection conn = DriverManager.getConnection(getUrl(), props)) {
+            String pTableFullName = SchemaUtil.getQualifiedTableName(schemaName, tableName);
+            String query = "create table " + pTableFullName +
+                    "(a_integer integer not null CONSTRAINT pk PRIMARY KEY (a_integer)) SALT_BUCKETS=16";
+            conn.createStatement().execute(query);
+            conn.commit();
+            String [] args = {"-tb", tableName, "-s", schemaName};
+            SchemaExtractionTool set = new SchemaExtractionTool();
+            set.setConf(conn.unwrap(PhoenixConnection.class).getQueryServices().getConfiguration());
+            set.run(args);
+            String actualProperties = set.getOutput().substring(set.getOutput().lastIndexOf(")")+1);
+            Assert.assertEquals(true, actualProperties.contains("SALT_BUCKETS=16"));
+        }
+    }
+
+    @Test
+    public void testCreateTableWithPKConstraint() throws Exception {
+        String tableName = generateUniqueName();
+        String schemaName = generateUniqueName();
+        Properties props = PropertiesUtil.deepCopy(TEST_PROPERTIES);
+
+        try (Connection conn = DriverManager.getConnection(getUrl(), props)) {
+            String pTableFullName = SchemaUtil.getQualifiedTableName(schemaName, tableName);
+            String query = "create table " + pTableFullName +
+                    "(a_char CHAR(15) NOT NULL, " +
+                    "b_char CHAR(15) NOT NULL, " +
+                    "c_bigint BIGINT NOT NULL CONSTRAINT PK PRIMARY KEY (a_char, b_char, c_bigint)) IMMUTABLE_ROWS=TRUE";
+            conn.createStatement().execute(query);
+            conn.commit();
+            String [] args = {"-tb", tableName, "-s", schemaName};
+            SchemaExtractionTool set = new SchemaExtractionTool();
+            set.setConf(conn.unwrap(PhoenixConnection.class).getQueryServices().getConfiguration());
+            set.run(args);
+
+            Assert.assertEquals(query.toUpperCase(), set.getOutput().toUpperCase());
+        }
+    }
+
+    @Test
+    public void testCreateTableWithArrayColumn() throws Exception {
+        String tableName = generateUniqueName();
+        String schemaName = generateUniqueName();
+        Properties props = PropertiesUtil.deepCopy(TEST_PROPERTIES);
+
+        try (Connection conn = DriverManager.getConnection(getUrl(), props)) {
+            String pTableFullName = SchemaUtil.getQualifiedTableName(schemaName, tableName);
+            String query = "create table " + pTableFullName +
+                    "(a_char CHAR(15) NOT NULL, " +
+                    "b_char CHAR(10) NOT NULL, " +
+                    "c_var_array VARCHAR ARRAY, " +
+                    "d_char_array CHAR(15) ARRAY[3] CONSTRAINT PK PRIMARY KEY (a_char, b_char)) " +
+                    "TTL=2592000, IMMUTABLE_STORAGE_SCHEME=ONE_CELL_PER_COLUMN, DISABLE_TABLE_SOR=true, REPLICATION_SCOPE=1";
+            conn.createStatement().execute(query);
+            conn.commit();
+            String[] args = {"-tb", tableName, "-s", schemaName};
+            SchemaExtractionTool set = new SchemaExtractionTool();
+            set.setConf(conn.unwrap(PhoenixConnection.class).getQueryServices().getConfiguration());
+            set.run(args);
+
+            Assert.assertEquals(query.toUpperCase(), set.getOutput().toUpperCase());
+        }
+    }
+
+    @Test
+    public void testCreateTableWithNonDefaultColumnFamily() throws Exception {
+        String tableName = generateUniqueName();
+        String schemaName = generateUniqueName();
+        Properties props = PropertiesUtil.deepCopy(TEST_PROPERTIES);
+
+        try (Connection conn = DriverManager.getConnection(getUrl(), props)) {
+            String pTableFullName = SchemaUtil.getQualifiedTableName(schemaName, tableName);
+            String query = "create table " + pTableFullName +
+                    "(a_char CHAR(15) NOT NULL, " +
+                    "b_char CHAR(10) NOT NULL, " +
+                    "\"av\".\"_\" CHAR(1), " +
+                    "\"bv\".\"_\" CHAR(1), " +
+                    "\"cv\".\"_\" CHAR(1), " +
+                    "\"dv\".\"_\" CHAR(1) CONSTRAINT PK PRIMARY KEY (a_char, b_char)) " +
+                    "TTL=1209600, IMMUTABLE_ROWS=true, IMMUTABLE_STORAGE_SCHEME=ONE_CELL_PER_COLUMN, SALT_BUCKETS=16, DISABLE_TABLE_SOR=true, MULTI_TENANT=true";
+            conn.createStatement().execute(query);
+            conn.commit();
+            String[] args = {"-tb", tableName, "-s", schemaName};
+            SchemaExtractionTool set = new SchemaExtractionTool();
+            set.setConf(conn.unwrap(PhoenixConnection.class).getQueryServices().getConfiguration());
+            set.run(args);
+
+            Assert.assertEquals(query.toUpperCase(), set.getOutput().toUpperCase());
+        }
+    }
+
+    @Test
+    public void testCreateTableWithUniversalCFProperties() throws Exception {
+        String tableName = generateUniqueName();
+        String schemaName = generateUniqueName();
+        Properties props = PropertiesUtil.deepCopy(TEST_PROPERTIES);
+        String properties = "KEEP_DELETED_CELLS=TRUE, TTL=1209600, IMMUTABLE_STORAGE_SCHEME=ONE_CELL_PER_COLUMN, REPLICATION_SCOPE=1";
+
+        try (Connection conn = DriverManager.getConnection(getUrl(), props)) {
+            String pTableFullName = SchemaUtil.getQualifiedTableName(schemaName, tableName);
+            String query = "create table " + pTableFullName +
+                    "(a_char CHAR(15) NOT NULL, " +
+                    "b_char CHAR(10) NOT NULL, " +
+                    "\"av\".\"_\" CHAR(1), " +
+                    "\"bv\".\"_\" CHAR(1), " +
+                    "\"cv\".\"_\" CHAR(1), " +
+                    "\"dv\".\"_\" CHAR(1) CONSTRAINT PK PRIMARY KEY (a_char, b_char)) " + properties;
+            conn.createStatement().execute(query);
+            conn.commit();
+            String[] args = {"-tb", tableName, "-s", schemaName};
+            SchemaExtractionTool set = new SchemaExtractionTool();
+            set.setConf(conn.unwrap(PhoenixConnection.class).getQueryServices().getConfiguration());
+            set.run(args);
+
+            Assert.assertEquals(query.toUpperCase(), set.getOutput().toUpperCase());
+        }
+    }
+
+    @Test
+    public void testCreateTableWithDefaultCFProperties() throws Exception {
+        String tableName = generateUniqueName();
+        String schemaName = generateUniqueName();
+        Properties props = PropertiesUtil.deepCopy(TEST_PROPERTIES);
+        String properties = "KEEP_DELETED_CELLS=TRUE, TTL=1209600, IMMUTABLE_STORAGE_SCHEME=ONE_CELL_PER_COLUMN, REPLICATION_SCOPE=1, DEFAULT_COLUMN_FAMILY=cv";
+
+        try (Connection conn = DriverManager.getConnection(getUrl(), props)) {
+            String pTableFullName = SchemaUtil.getQualifiedTableName(schemaName, tableName);
+            String query = "create table " + pTableFullName +
+                    "(a_char CHAR(15) NOT NULL, " +
+                    "b_char CHAR(10) NOT NULL, " +
+                    "\"av\".\"_\" CHAR(1), " +
+                    "\"bv\".\"_\" CHAR(1), " +
+                    "\"cv\".\"_\" CHAR(1), " +
+                    "\"dv\".\"_\" CHAR(1) CONSTRAINT PK PRIMARY KEY (a_char, b_char)) " + properties;
+            conn.createStatement().execute(query);
+            conn.commit();
+            String[] args = {"-tb", tableName, "-s", schemaName};
+            SchemaExtractionTool set = new SchemaExtractionTool();
+            set.setConf(conn.unwrap(PhoenixConnection.class).getQueryServices().getConfiguration());
+            set.run(args);
+
+            Assert.assertTrue(compareProperties(properties, set.getOutput().substring(set.getOutput().lastIndexOf(")")+1)));
+        }
+    }
+
+    @Test
+    public void testCreateTableWithMultipleCFProperties() throws Exception {
+        String tableName = generateUniqueName();
+        String schemaName = generateUniqueName();
+        Properties props = PropertiesUtil.deepCopy(TEST_PROPERTIES);
+        String properties = "\"av\".VERSIONS=2, \"bv\".VERSIONS=3, " +
+                "\"cv\".VERSIONS=4, DATA_BLOCK_ENCODING=DIFF, " +
+                "IMMUTABLE_STORAGE_SCHEME=ONE_CELL_PER_COLUMN, SALT_BUCKETS=16, DISABLE_TABLE_SOR=true, MULTI_TENANT=true";
+
+        try (Connection conn = DriverManager.getConnection(getUrl(), props)) {
+            String pTableFullName = SchemaUtil.getQualifiedTableName(schemaName, tableName);
+            String query = "create table " + pTableFullName +
+                    "(a_char CHAR(15) NOT NULL, " +
+                    "b_char CHAR(10) NOT NULL, " +
+                    "\"av\".\"_\" CHAR(1), " +
+                    "\"bv\".\"_\" CHAR(1), " +
+                    "\"cv\".\"_\" CHAR(1), " +
+                    "\"dv\".\"_\" CHAR(1) CONSTRAINT PK PRIMARY KEY (a_char, b_char)) " + properties;
+            conn.createStatement().execute(query);
+            conn.commit();
+            String[] args = {"-tb", tableName, "-s", schemaName};
+            SchemaExtractionTool set = new SchemaExtractionTool();
+            set.setConf(conn.unwrap(PhoenixConnection.class).getQueryServices().getConfiguration());
+            set.run(args);
+
+            Assert.assertTrue(compareProperties(properties, set.getOutput().substring(set.getOutput().lastIndexOf(")")+1)));
+        }
+    }
+
+    @Test
+    public void testCreateTableWithMultipleCFProperties2() throws Exception {
+        String tableName = generateUniqueName();
+        String schemaName = generateUniqueName();
+        Properties props = PropertiesUtil.deepCopy(TEST_PROPERTIES);
+        String properties = "\"av\".VERSIONS=2, \"bv\".VERSIONS=2, " +
+                "DATA_BLOCK_ENCODING=DIFF, " +
+                "IMMUTABLE_STORAGE_SCHEME=ONE_CELL_PER_COLUMN, SALT_BUCKETS=16, DISABLE_TABLE_SOR=true, MULTI_TENANT=true";
+
+        try (Connection conn = DriverManager.getConnection(getUrl(), props)) {
+            String pTableFullName = SchemaUtil.getQualifiedTableName(schemaName, tableName);
+            String query = "create table " + pTableFullName +
+                    "(a_char CHAR(15) NOT NULL, " +
+                    "b_char CHAR(10) NOT NULL, " +
+                    "\"av\".\"_\" CHAR(1), " +
+                    "\"bv\".\"_\" CHAR(1), " +
+                    "\"cv\".\"_\" CHAR(1) CONSTRAINT PK PRIMARY KEY (a_char, b_char)) " + properties;
+            conn.createStatement().execute(query);
+            conn.commit();
+            String[] args = {"-tb", tableName, "-s", schemaName};
+            SchemaExtractionTool set = new SchemaExtractionTool();
+            set.setConf(conn.unwrap(PhoenixConnection.class).getQueryServices().getConfiguration());
+            set.run(args);
+
+            Assert.assertTrue(compareProperties(properties, set.getOutput().substring(set.getOutput().lastIndexOf(")")+1)));

Review comment:
       likewise, please specify what you're trying to show here. You also might want to extract "set.getOutput().substring(set.getOutput().lastIndexOf(")")+1)" into a function with a user-friendly name since you do this operation in many tests, I think to get the properties. 

##########
File path: phoenix-tools/src/it/java/org/apache/phoenix/schema/SchemaExtractionToolIT.java
##########
@@ -210,4 +204,242 @@ public void testCreateViewIndexStatement() throws Exception {
             Assert.assertEquals(createIndexStatement.toUpperCase(), set.getOutput().toUpperCase());
         }
     }
+
+    @Test
+    public void testSaltedTableStatement() throws Exception {
+        String tableName = generateUniqueName();
+        String schemaName = generateUniqueName();
+        Properties props = PropertiesUtil.deepCopy(TEST_PROPERTIES);
+
+        try (Connection conn = DriverManager.getConnection(getUrl(), props)) {
+            String pTableFullName = SchemaUtil.getQualifiedTableName(schemaName, tableName);
+            String query = "create table " + pTableFullName +
+                    "(a_integer integer not null CONSTRAINT pk PRIMARY KEY (a_integer)) SALT_BUCKETS=16";
+            conn.createStatement().execute(query);
+            conn.commit();
+            String [] args = {"-tb", tableName, "-s", schemaName};
+            SchemaExtractionTool set = new SchemaExtractionTool();
+            set.setConf(conn.unwrap(PhoenixConnection.class).getQueryServices().getConfiguration());
+            set.run(args);
+            String actualProperties = set.getOutput().substring(set.getOutput().lastIndexOf(")")+1);
+            Assert.assertEquals(true, actualProperties.contains("SALT_BUCKETS=16"));
+        }
+    }
+
+    @Test
+    public void testCreateTableWithPKConstraint() throws Exception {
+        String tableName = generateUniqueName();
+        String schemaName = generateUniqueName();
+        Properties props = PropertiesUtil.deepCopy(TEST_PROPERTIES);
+
+        try (Connection conn = DriverManager.getConnection(getUrl(), props)) {
+            String pTableFullName = SchemaUtil.getQualifiedTableName(schemaName, tableName);
+            String query = "create table " + pTableFullName +
+                    "(a_char CHAR(15) NOT NULL, " +
+                    "b_char CHAR(15) NOT NULL, " +
+                    "c_bigint BIGINT NOT NULL CONSTRAINT PK PRIMARY KEY (a_char, b_char, c_bigint)) IMMUTABLE_ROWS=TRUE";
+            conn.createStatement().execute(query);
+            conn.commit();
+            String [] args = {"-tb", tableName, "-s", schemaName};
+            SchemaExtractionTool set = new SchemaExtractionTool();
+            set.setConf(conn.unwrap(PhoenixConnection.class).getQueryServices().getConfiguration());
+            set.run(args);
+
+            Assert.assertEquals(query.toUpperCase(), set.getOutput().toUpperCase());
+        }
+    }
+
+    @Test
+    public void testCreateTableWithArrayColumn() throws Exception {
+        String tableName = generateUniqueName();
+        String schemaName = generateUniqueName();
+        Properties props = PropertiesUtil.deepCopy(TEST_PROPERTIES);
+
+        try (Connection conn = DriverManager.getConnection(getUrl(), props)) {
+            String pTableFullName = SchemaUtil.getQualifiedTableName(schemaName, tableName);
+            String query = "create table " + pTableFullName +
+                    "(a_char CHAR(15) NOT NULL, " +
+                    "b_char CHAR(10) NOT NULL, " +
+                    "c_var_array VARCHAR ARRAY, " +
+                    "d_char_array CHAR(15) ARRAY[3] CONSTRAINT PK PRIMARY KEY (a_char, b_char)) " +
+                    "TTL=2592000, IMMUTABLE_STORAGE_SCHEME=ONE_CELL_PER_COLUMN, DISABLE_TABLE_SOR=true, REPLICATION_SCOPE=1";
+            conn.createStatement().execute(query);
+            conn.commit();
+            String[] args = {"-tb", tableName, "-s", schemaName};
+            SchemaExtractionTool set = new SchemaExtractionTool();
+            set.setConf(conn.unwrap(PhoenixConnection.class).getQueryServices().getConfiguration());
+            set.run(args);
+
+            Assert.assertEquals(query.toUpperCase(), set.getOutput().toUpperCase());
+        }
+    }
+
+    @Test
+    public void testCreateTableWithNonDefaultColumnFamily() throws Exception {
+        String tableName = generateUniqueName();
+        String schemaName = generateUniqueName();
+        Properties props = PropertiesUtil.deepCopy(TEST_PROPERTIES);
+
+        try (Connection conn = DriverManager.getConnection(getUrl(), props)) {
+            String pTableFullName = SchemaUtil.getQualifiedTableName(schemaName, tableName);
+            String query = "create table " + pTableFullName +
+                    "(a_char CHAR(15) NOT NULL, " +
+                    "b_char CHAR(10) NOT NULL, " +
+                    "\"av\".\"_\" CHAR(1), " +
+                    "\"bv\".\"_\" CHAR(1), " +
+                    "\"cv\".\"_\" CHAR(1), " +
+                    "\"dv\".\"_\" CHAR(1) CONSTRAINT PK PRIMARY KEY (a_char, b_char)) " +
+                    "TTL=1209600, IMMUTABLE_ROWS=true, IMMUTABLE_STORAGE_SCHEME=ONE_CELL_PER_COLUMN, SALT_BUCKETS=16, DISABLE_TABLE_SOR=true, MULTI_TENANT=true";
+            conn.createStatement().execute(query);
+            conn.commit();
+            String[] args = {"-tb", tableName, "-s", schemaName};
+            SchemaExtractionTool set = new SchemaExtractionTool();
+            set.setConf(conn.unwrap(PhoenixConnection.class).getQueryServices().getConfiguration());
+            set.run(args);
+
+            Assert.assertEquals(query.toUpperCase(), set.getOutput().toUpperCase());
+        }
+    }
+
+    @Test
+    public void testCreateTableWithUniversalCFProperties() throws Exception {
+        String tableName = generateUniqueName();
+        String schemaName = generateUniqueName();
+        Properties props = PropertiesUtil.deepCopy(TEST_PROPERTIES);
+        String properties = "KEEP_DELETED_CELLS=TRUE, TTL=1209600, IMMUTABLE_STORAGE_SCHEME=ONE_CELL_PER_COLUMN, REPLICATION_SCOPE=1";
+
+        try (Connection conn = DriverManager.getConnection(getUrl(), props)) {
+            String pTableFullName = SchemaUtil.getQualifiedTableName(schemaName, tableName);
+            String query = "create table " + pTableFullName +
+                    "(a_char CHAR(15) NOT NULL, " +
+                    "b_char CHAR(10) NOT NULL, " +
+                    "\"av\".\"_\" CHAR(1), " +
+                    "\"bv\".\"_\" CHAR(1), " +
+                    "\"cv\".\"_\" CHAR(1), " +
+                    "\"dv\".\"_\" CHAR(1) CONSTRAINT PK PRIMARY KEY (a_char, b_char)) " + properties;
+            conn.createStatement().execute(query);
+            conn.commit();
+            String[] args = {"-tb", tableName, "-s", schemaName};
+            SchemaExtractionTool set = new SchemaExtractionTool();
+            set.setConf(conn.unwrap(PhoenixConnection.class).getQueryServices().getConfiguration());
+            set.run(args);
+
+            Assert.assertEquals(query.toUpperCase(), set.getOutput().toUpperCase());
+        }
+    }
+
+    @Test
+    public void testCreateTableWithDefaultCFProperties() throws Exception {
+        String tableName = generateUniqueName();
+        String schemaName = generateUniqueName();
+        Properties props = PropertiesUtil.deepCopy(TEST_PROPERTIES);
+        String properties = "KEEP_DELETED_CELLS=TRUE, TTL=1209600, IMMUTABLE_STORAGE_SCHEME=ONE_CELL_PER_COLUMN, REPLICATION_SCOPE=1, DEFAULT_COLUMN_FAMILY=cv";
+
+        try (Connection conn = DriverManager.getConnection(getUrl(), props)) {
+            String pTableFullName = SchemaUtil.getQualifiedTableName(schemaName, tableName);
+            String query = "create table " + pTableFullName +
+                    "(a_char CHAR(15) NOT NULL, " +
+                    "b_char CHAR(10) NOT NULL, " +
+                    "\"av\".\"_\" CHAR(1), " +
+                    "\"bv\".\"_\" CHAR(1), " +
+                    "\"cv\".\"_\" CHAR(1), " +
+                    "\"dv\".\"_\" CHAR(1) CONSTRAINT PK PRIMARY KEY (a_char, b_char)) " + properties;
+            conn.createStatement().execute(query);
+            conn.commit();
+            String[] args = {"-tb", tableName, "-s", schemaName};
+            SchemaExtractionTool set = new SchemaExtractionTool();
+            set.setConf(conn.unwrap(PhoenixConnection.class).getQueryServices().getConfiguration());
+            set.run(args);
+
+            Assert.assertTrue(compareProperties(properties, set.getOutput().substring(set.getOutput().lastIndexOf(")")+1)));
+        }
+    }
+
+    @Test
+    public void testCreateTableWithMultipleCFProperties() throws Exception {
+        String tableName = generateUniqueName();
+        String schemaName = generateUniqueName();
+        Properties props = PropertiesUtil.deepCopy(TEST_PROPERTIES);
+        String properties = "\"av\".VERSIONS=2, \"bv\".VERSIONS=3, " +
+                "\"cv\".VERSIONS=4, DATA_BLOCK_ENCODING=DIFF, " +
+                "IMMUTABLE_STORAGE_SCHEME=ONE_CELL_PER_COLUMN, SALT_BUCKETS=16, DISABLE_TABLE_SOR=true, MULTI_TENANT=true";
+
+        try (Connection conn = DriverManager.getConnection(getUrl(), props)) {
+            String pTableFullName = SchemaUtil.getQualifiedTableName(schemaName, tableName);
+            String query = "create table " + pTableFullName +
+                    "(a_char CHAR(15) NOT NULL, " +
+                    "b_char CHAR(10) NOT NULL, " +
+                    "\"av\".\"_\" CHAR(1), " +
+                    "\"bv\".\"_\" CHAR(1), " +
+                    "\"cv\".\"_\" CHAR(1), " +
+                    "\"dv\".\"_\" CHAR(1) CONSTRAINT PK PRIMARY KEY (a_char, b_char)) " + properties;
+            conn.createStatement().execute(query);
+            conn.commit();
+            String[] args = {"-tb", tableName, "-s", schemaName};
+            SchemaExtractionTool set = new SchemaExtractionTool();
+            set.setConf(conn.unwrap(PhoenixConnection.class).getQueryServices().getConfiguration());
+            set.run(args);
+
+            Assert.assertTrue(compareProperties(properties, set.getOutput().substring(set.getOutput().lastIndexOf(")")+1)));
+        }
+    }
+
+    @Test
+    public void testCreateTableWithMultipleCFProperties2() throws Exception {

Review comment:
       nit: please use a more informative name. 

##########
File path: phoenix-tools/src/main/java/org/apache/phoenix/schema/SchemaExtractionProcessor.java
##########
@@ -250,12 +247,31 @@ private void setHTableProperties(HTableDescriptor htd) {
         }
     }
 
-    private void setHColumnFamilyProperties(HColumnDescriptor columnDescriptor) {
-        Map<ImmutableBytesWritable, ImmutableBytesWritable> propsMap = columnDescriptor.getValues();
-        for (Map.Entry<ImmutableBytesWritable, ImmutableBytesWritable> entry : propsMap.entrySet()) {
+    private void setHColumnFamilyProperties(HColumnDescriptor[] columnDescriptors) {
+        Map<ImmutableBytesWritable, ImmutableBytesWritable> propsMap = columnDescriptors[0].getValues();
+        for(Map.Entry<ImmutableBytesWritable, ImmutableBytesWritable> entry : propsMap.entrySet()) {
             ImmutableBytesWritable key = entry.getKey();
-            ImmutableBytesWritable value = entry.getValue();
-            definedProps.put(Bytes.toString(key.get()), Bytes.toString(value.get()));
+            ImmutableBytesWritable globalValue = entry.getValue();
+            Map<String, String> cfToPropertyValueMap = new HashMap<String, String>();
+            Set<ImmutableBytesWritable> cfPropertyValueSet = new HashSet<ImmutableBytesWritable>();
+            for(HColumnDescriptor columnDescriptor: columnDescriptors) {
+                String columnFamilyName = Bytes.toString(columnDescriptor.getName());
+                ImmutableBytesWritable value = columnDescriptor.getValues().get(key);
+                // check if it is universal properties
+                if (SYNCED_DATA_TABLE_AND_INDEX_COL_FAM_PROPERTIES.contains(Bytes.toString(key.get()))) {
+                    definedProps.put(Bytes.toString(key.get()), Bytes.toString(value.get()));
+                    break;
+                }
+                cfToPropertyValueMap.put(columnFamilyName, Bytes.toString(value.get()));
+                cfPropertyValueSet.add(value);
+            }
+            if (cfPropertyValueSet.size() > 1) {
+                for(Map.Entry<String, String> mapEntry: cfToPropertyValueMap.entrySet()) {
+                    definedProps.put(String.format("%s.%s",  mapEntry.getKey(), Bytes.toString(key.get())), mapEntry.getValue());

Review comment:
       I'm curious why we're qualifying the CF properties in one flat HashSet instead of a HashSet<String, HashSet<String, String>>? Can we guarantee that no CF properties will ever have a period in them?

##########
File path: phoenix-tools/src/it/java/org/apache/phoenix/schema/SchemaExtractionToolIT.java
##########
@@ -210,4 +204,242 @@ public void testCreateViewIndexStatement() throws Exception {
             Assert.assertEquals(createIndexStatement.toUpperCase(), set.getOutput().toUpperCase());
         }
     }
+
+    @Test
+    public void testSaltedTableStatement() throws Exception {
+        String tableName = generateUniqueName();
+        String schemaName = generateUniqueName();
+        Properties props = PropertiesUtil.deepCopy(TEST_PROPERTIES);
+
+        try (Connection conn = DriverManager.getConnection(getUrl(), props)) {
+            String pTableFullName = SchemaUtil.getQualifiedTableName(schemaName, tableName);
+            String query = "create table " + pTableFullName +
+                    "(a_integer integer not null CONSTRAINT pk PRIMARY KEY (a_integer)) SALT_BUCKETS=16";
+            conn.createStatement().execute(query);
+            conn.commit();
+            String [] args = {"-tb", tableName, "-s", schemaName};
+            SchemaExtractionTool set = new SchemaExtractionTool();
+            set.setConf(conn.unwrap(PhoenixConnection.class).getQueryServices().getConfiguration());
+            set.run(args);
+            String actualProperties = set.getOutput().substring(set.getOutput().lastIndexOf(")")+1);
+            Assert.assertEquals(true, actualProperties.contains("SALT_BUCKETS=16"));
+        }
+    }
+
+    @Test
+    public void testCreateTableWithPKConstraint() throws Exception {
+        String tableName = generateUniqueName();
+        String schemaName = generateUniqueName();
+        Properties props = PropertiesUtil.deepCopy(TEST_PROPERTIES);
+
+        try (Connection conn = DriverManager.getConnection(getUrl(), props)) {
+            String pTableFullName = SchemaUtil.getQualifiedTableName(schemaName, tableName);
+            String query = "create table " + pTableFullName +
+                    "(a_char CHAR(15) NOT NULL, " +
+                    "b_char CHAR(15) NOT NULL, " +
+                    "c_bigint BIGINT NOT NULL CONSTRAINT PK PRIMARY KEY (a_char, b_char, c_bigint)) IMMUTABLE_ROWS=TRUE";
+            conn.createStatement().execute(query);
+            conn.commit();
+            String [] args = {"-tb", tableName, "-s", schemaName};
+            SchemaExtractionTool set = new SchemaExtractionTool();
+            set.setConf(conn.unwrap(PhoenixConnection.class).getQueryServices().getConfiguration());
+            set.run(args);
+
+            Assert.assertEquals(query.toUpperCase(), set.getOutput().toUpperCase());
+        }
+    }
+
+    @Test
+    public void testCreateTableWithArrayColumn() throws Exception {
+        String tableName = generateUniqueName();
+        String schemaName = generateUniqueName();
+        Properties props = PropertiesUtil.deepCopy(TEST_PROPERTIES);
+
+        try (Connection conn = DriverManager.getConnection(getUrl(), props)) {
+            String pTableFullName = SchemaUtil.getQualifiedTableName(schemaName, tableName);
+            String query = "create table " + pTableFullName +
+                    "(a_char CHAR(15) NOT NULL, " +
+                    "b_char CHAR(10) NOT NULL, " +
+                    "c_var_array VARCHAR ARRAY, " +
+                    "d_char_array CHAR(15) ARRAY[3] CONSTRAINT PK PRIMARY KEY (a_char, b_char)) " +
+                    "TTL=2592000, IMMUTABLE_STORAGE_SCHEME=ONE_CELL_PER_COLUMN, DISABLE_TABLE_SOR=true, REPLICATION_SCOPE=1";
+            conn.createStatement().execute(query);
+            conn.commit();
+            String[] args = {"-tb", tableName, "-s", schemaName};
+            SchemaExtractionTool set = new SchemaExtractionTool();
+            set.setConf(conn.unwrap(PhoenixConnection.class).getQueryServices().getConfiguration());
+            set.run(args);
+
+            Assert.assertEquals(query.toUpperCase(), set.getOutput().toUpperCase());
+        }
+    }
+
+    @Test
+    public void testCreateTableWithNonDefaultColumnFamily() throws Exception {
+        String tableName = generateUniqueName();
+        String schemaName = generateUniqueName();
+        Properties props = PropertiesUtil.deepCopy(TEST_PROPERTIES);
+
+        try (Connection conn = DriverManager.getConnection(getUrl(), props)) {
+            String pTableFullName = SchemaUtil.getQualifiedTableName(schemaName, tableName);
+            String query = "create table " + pTableFullName +
+                    "(a_char CHAR(15) NOT NULL, " +
+                    "b_char CHAR(10) NOT NULL, " +
+                    "\"av\".\"_\" CHAR(1), " +
+                    "\"bv\".\"_\" CHAR(1), " +
+                    "\"cv\".\"_\" CHAR(1), " +
+                    "\"dv\".\"_\" CHAR(1) CONSTRAINT PK PRIMARY KEY (a_char, b_char)) " +
+                    "TTL=1209600, IMMUTABLE_ROWS=true, IMMUTABLE_STORAGE_SCHEME=ONE_CELL_PER_COLUMN, SALT_BUCKETS=16, DISABLE_TABLE_SOR=true, MULTI_TENANT=true";
+            conn.createStatement().execute(query);
+            conn.commit();
+            String[] args = {"-tb", tableName, "-s", schemaName};
+            SchemaExtractionTool set = new SchemaExtractionTool();
+            set.setConf(conn.unwrap(PhoenixConnection.class).getQueryServices().getConfiguration());
+            set.run(args);
+
+            Assert.assertEquals(query.toUpperCase(), set.getOutput().toUpperCase());
+        }
+    }
+
+    @Test
+    public void testCreateTableWithUniversalCFProperties() throws Exception {
+        String tableName = generateUniqueName();
+        String schemaName = generateUniqueName();
+        Properties props = PropertiesUtil.deepCopy(TEST_PROPERTIES);
+        String properties = "KEEP_DELETED_CELLS=TRUE, TTL=1209600, IMMUTABLE_STORAGE_SCHEME=ONE_CELL_PER_COLUMN, REPLICATION_SCOPE=1";
+
+        try (Connection conn = DriverManager.getConnection(getUrl(), props)) {
+            String pTableFullName = SchemaUtil.getQualifiedTableName(schemaName, tableName);
+            String query = "create table " + pTableFullName +
+                    "(a_char CHAR(15) NOT NULL, " +
+                    "b_char CHAR(10) NOT NULL, " +
+                    "\"av\".\"_\" CHAR(1), " +
+                    "\"bv\".\"_\" CHAR(1), " +
+                    "\"cv\".\"_\" CHAR(1), " +
+                    "\"dv\".\"_\" CHAR(1) CONSTRAINT PK PRIMARY KEY (a_char, b_char)) " + properties;
+            conn.createStatement().execute(query);
+            conn.commit();
+            String[] args = {"-tb", tableName, "-s", schemaName};
+            SchemaExtractionTool set = new SchemaExtractionTool();
+            set.setConf(conn.unwrap(PhoenixConnection.class).getQueryServices().getConfiguration());
+            set.run(args);
+
+            Assert.assertEquals(query.toUpperCase(), set.getOutput().toUpperCase());
+        }
+    }
+
+    @Test
+    public void testCreateTableWithDefaultCFProperties() throws Exception {
+        String tableName = generateUniqueName();
+        String schemaName = generateUniqueName();
+        Properties props = PropertiesUtil.deepCopy(TEST_PROPERTIES);
+        String properties = "KEEP_DELETED_CELLS=TRUE, TTL=1209600, IMMUTABLE_STORAGE_SCHEME=ONE_CELL_PER_COLUMN, REPLICATION_SCOPE=1, DEFAULT_COLUMN_FAMILY=cv";
+
+        try (Connection conn = DriverManager.getConnection(getUrl(), props)) {
+            String pTableFullName = SchemaUtil.getQualifiedTableName(schemaName, tableName);
+            String query = "create table " + pTableFullName +
+                    "(a_char CHAR(15) NOT NULL, " +
+                    "b_char CHAR(10) NOT NULL, " +
+                    "\"av\".\"_\" CHAR(1), " +
+                    "\"bv\".\"_\" CHAR(1), " +
+                    "\"cv\".\"_\" CHAR(1), " +
+                    "\"dv\".\"_\" CHAR(1) CONSTRAINT PK PRIMARY KEY (a_char, b_char)) " + properties;
+            conn.createStatement().execute(query);
+            conn.commit();
+            String[] args = {"-tb", tableName, "-s", schemaName};
+            SchemaExtractionTool set = new SchemaExtractionTool();
+            set.setConf(conn.unwrap(PhoenixConnection.class).getQueryServices().getConfiguration());
+            set.run(args);
+
+            Assert.assertTrue(compareProperties(properties, set.getOutput().substring(set.getOutput().lastIndexOf(")")+1)));

Review comment:
       nit: comment on what exactly you're checking here would be useful. 

##########
File path: phoenix-tools/src/it/java/org/apache/phoenix/schema/SchemaExtractionToolIT.java
##########
@@ -210,4 +204,242 @@ public void testCreateViewIndexStatement() throws Exception {
             Assert.assertEquals(createIndexStatement.toUpperCase(), set.getOutput().toUpperCase());
         }
     }
+
+    @Test
+    public void testSaltedTableStatement() throws Exception {
+        String tableName = generateUniqueName();
+        String schemaName = generateUniqueName();
+        Properties props = PropertiesUtil.deepCopy(TEST_PROPERTIES);
+
+        try (Connection conn = DriverManager.getConnection(getUrl(), props)) {
+            String pTableFullName = SchemaUtil.getQualifiedTableName(schemaName, tableName);
+            String query = "create table " + pTableFullName +
+                    "(a_integer integer not null CONSTRAINT pk PRIMARY KEY (a_integer)) SALT_BUCKETS=16";
+            conn.createStatement().execute(query);
+            conn.commit();
+            String [] args = {"-tb", tableName, "-s", schemaName};
+            SchemaExtractionTool set = new SchemaExtractionTool();
+            set.setConf(conn.unwrap(PhoenixConnection.class).getQueryServices().getConfiguration());
+            set.run(args);
+            String actualProperties = set.getOutput().substring(set.getOutput().lastIndexOf(")")+1);
+            Assert.assertEquals(true, actualProperties.contains("SALT_BUCKETS=16"));
+        }
+    }
+
+    @Test
+    public void testCreateTableWithPKConstraint() throws Exception {
+        String tableName = generateUniqueName();
+        String schemaName = generateUniqueName();
+        Properties props = PropertiesUtil.deepCopy(TEST_PROPERTIES);
+
+        try (Connection conn = DriverManager.getConnection(getUrl(), props)) {
+            String pTableFullName = SchemaUtil.getQualifiedTableName(schemaName, tableName);
+            String query = "create table " + pTableFullName +
+                    "(a_char CHAR(15) NOT NULL, " +
+                    "b_char CHAR(15) NOT NULL, " +
+                    "c_bigint BIGINT NOT NULL CONSTRAINT PK PRIMARY KEY (a_char, b_char, c_bigint)) IMMUTABLE_ROWS=TRUE";
+            conn.createStatement().execute(query);
+            conn.commit();
+            String [] args = {"-tb", tableName, "-s", schemaName};
+            SchemaExtractionTool set = new SchemaExtractionTool();
+            set.setConf(conn.unwrap(PhoenixConnection.class).getQueryServices().getConfiguration());
+            set.run(args);
+
+            Assert.assertEquals(query.toUpperCase(), set.getOutput().toUpperCase());
+        }
+    }
+
+    @Test
+    public void testCreateTableWithArrayColumn() throws Exception {
+        String tableName = generateUniqueName();
+        String schemaName = generateUniqueName();
+        Properties props = PropertiesUtil.deepCopy(TEST_PROPERTIES);
+
+        try (Connection conn = DriverManager.getConnection(getUrl(), props)) {
+            String pTableFullName = SchemaUtil.getQualifiedTableName(schemaName, tableName);
+            String query = "create table " + pTableFullName +
+                    "(a_char CHAR(15) NOT NULL, " +
+                    "b_char CHAR(10) NOT NULL, " +
+                    "c_var_array VARCHAR ARRAY, " +
+                    "d_char_array CHAR(15) ARRAY[3] CONSTRAINT PK PRIMARY KEY (a_char, b_char)) " +

Review comment:
       Good edge case to check!

##########
File path: phoenix-tools/src/main/java/org/apache/phoenix/schema/SchemaExtractionProcessor.java
##########
@@ -312,9 +336,8 @@ public Connection getConnection() throws SQLException {
 
     private String getColumnInfoStringForTable(PTable table) {
         StringBuilder colInfo = new StringBuilder();
-
-        List<PColumn> columns = table.getColumns();
-        List<PColumn> pkColumns = table.getPKColumns();
+        List<PColumn> columns = table.getBucketNum() == null ? table.getColumns() : table.getColumns().subList(1, table.getColumns().size());

Review comment:
       Good catch for salted tables, those are easy to miss. 




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [phoenix] swaroopak commented on a change in pull request #848: Phoenix-5947 Add tests and extensions to Schema Extraction Tool utility

Posted by GitBox <gi...@apache.org>.
swaroopak commented on a change in pull request #848:
URL: https://github.com/apache/phoenix/pull/848#discussion_r461722705



##########
File path: phoenix-tools/src/main/java/org/apache/phoenix/schema/SchemaExtractionProcessor.java
##########
@@ -54,9 +55,6 @@ public String process() throws Exception {
         if (ddl != null) {
             return ddl;
         }
-        if(this.table.getColumnFamilies().size() > 1 ) {

Review comment:
       This PR does not completely support the multiple column families, especially if there are different properties for each column family as mentioned by Goeffrey on the previous review. Let's modify this if clause to be specific unless you want to include the change in this one and completely get rid of "Not Supported Exception". 




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [phoenix] Qinrui98 commented on a change in pull request #848: Phoenix-5947 Add tests and extensions to Schema Extraction Tool utility

Posted by GitBox <gi...@apache.org>.
Qinrui98 commented on a change in pull request #848:
URL: https://github.com/apache/phoenix/pull/848#discussion_r463285405



##########
File path: phoenix-tools/src/main/java/org/apache/phoenix/schema/SchemaExtractionProcessor.java
##########
@@ -250,12 +247,31 @@ private void setHTableProperties(HTableDescriptor htd) {
         }
     }
 
-    private void setHColumnFamilyProperties(HColumnDescriptor columnDescriptor) {
-        Map<ImmutableBytesWritable, ImmutableBytesWritable> propsMap = columnDescriptor.getValues();
-        for (Map.Entry<ImmutableBytesWritable, ImmutableBytesWritable> entry : propsMap.entrySet()) {
+    private void setHColumnFamilyProperties(HColumnDescriptor[] columnDescriptors) {
+        Map<ImmutableBytesWritable, ImmutableBytesWritable> propsMap = columnDescriptors[0].getValues();
+        for(Map.Entry<ImmutableBytesWritable, ImmutableBytesWritable> entry : propsMap.entrySet()) {
             ImmutableBytesWritable key = entry.getKey();
-            ImmutableBytesWritable value = entry.getValue();
-            definedProps.put(Bytes.toString(key.get()), Bytes.toString(value.get()));
+            ImmutableBytesWritable globalValue = entry.getValue();
+            Map<String, String> cfToPropertyValueMap = new HashMap<String, String>();
+            Set<ImmutableBytesWritable> cfPropertyValueSet = new HashSet<ImmutableBytesWritable>();
+            for(HColumnDescriptor columnDescriptor: columnDescriptors) {
+                String columnFamilyName = Bytes.toString(columnDescriptor.getName());
+                ImmutableBytesWritable value = columnDescriptor.getValues().get(key);
+                // check if it is universal properties
+                if (SYNCED_DATA_TABLE_AND_INDEX_COL_FAM_PROPERTIES.contains(Bytes.toString(key.get()))) {
+                    definedProps.put(Bytes.toString(key.get()), Bytes.toString(value.get()));
+                    break;
+                }
+                cfToPropertyValueMap.put(columnFamilyName, Bytes.toString(value.get()));
+                cfPropertyValueSet.add(value);
+            }
+            if (cfPropertyValueSet.size() > 1) {
+                for(Map.Entry<String, String> mapEntry: cfToPropertyValueMap.entrySet()) {
+                    definedProps.put(String.format("%s.%s",  mapEntry.getKey(), Bytes.toString(key.get())), mapEntry.getValue());

Review comment:
       @gjacoby126 We are trying to find common CF property so that we don't need to specify it for every column family in the output string. Are you suggesting that we could use hashmap instead? 
   
   Update: I reviewed the grammar of the column family name and we use underscore as the separator between words.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [phoenix] swaroopak commented on a change in pull request #848: Phoenix-5947 Add tests and extensions to Schema Extraction Tool utility

Posted by GitBox <gi...@apache.org>.
swaroopak commented on a change in pull request #848:
URL: https://github.com/apache/phoenix/pull/848#discussion_r462648136



##########
File path: phoenix-tools/src/main/java/org/apache/phoenix/schema/SchemaExtractionProcessor.java
##########
@@ -1,35 +1,35 @@
 package org.apache.phoenix.schema;
 
+import com.google.common.collect.ImmutableList;
 import com.google.common.collect.Sets;
+import com.google.inject.internal.util.$ImmutableCollection;
 import org.apache.commons.lang.StringUtils;
 import org.apache.hadoop.conf.Configuration;
 import org.apache.hadoop.hbase.HColumnDescriptor;
+import org.apache.hadoop.hbase.HConstants;
 import org.apache.hadoop.hbase.HTableDescriptor;
 import org.apache.hadoop.hbase.TableName;
 import org.apache.hadoop.hbase.io.ImmutableBytesWritable;
 import org.apache.hadoop.hbase.util.Bytes;
 import org.apache.phoenix.jdbc.PhoenixConnection;
 import org.apache.phoenix.mapreduce.util.ConnectionUtil;
 import org.apache.phoenix.query.ConnectionQueryServices;
+import org.apache.phoenix.query.QueryConstants;
 import org.apache.phoenix.util.MetaDataUtil;
 import org.apache.phoenix.util.PhoenixRuntime;
 import org.apache.phoenix.util.SchemaUtil;
 
 import java.io.IOException;
 import java.sql.Connection;
 import java.sql.SQLException;
-import java.util.ArrayList;
-import java.util.HashMap;
-import java.util.HashSet;
-import java.util.List;
-import java.util.Map;
-import java.util.Set;
+import java.util.*;
 
 public class SchemaExtractionProcessor {
 
-    public static final String
-            FEATURE_NOT_SUPPORTED_ON_EXTRACTION_TOOL =
-            "Multiple CF feature not supported on extraction tool";
+    public static final List<String> SYNCED_DATA_TABLE_AND_INDEX_COL_FAM_PROPERTIES = ImmutableList.of(

Review comment:
       This should be available in other parts of the code. You can just import here. 

##########
File path: phoenix-tools/src/main/java/org/apache/phoenix/schema/SchemaExtractionProcessor.java
##########
@@ -1,35 +1,35 @@
 package org.apache.phoenix.schema;
 
+import com.google.common.collect.ImmutableList;
 import com.google.common.collect.Sets;
+import com.google.inject.internal.util.$ImmutableCollection;
 import org.apache.commons.lang.StringUtils;
 import org.apache.hadoop.conf.Configuration;
 import org.apache.hadoop.hbase.HColumnDescriptor;
+import org.apache.hadoop.hbase.HConstants;
 import org.apache.hadoop.hbase.HTableDescriptor;
 import org.apache.hadoop.hbase.TableName;
 import org.apache.hadoop.hbase.io.ImmutableBytesWritable;
 import org.apache.hadoop.hbase.util.Bytes;
 import org.apache.phoenix.jdbc.PhoenixConnection;
 import org.apache.phoenix.mapreduce.util.ConnectionUtil;
 import org.apache.phoenix.query.ConnectionQueryServices;
+import org.apache.phoenix.query.QueryConstants;
 import org.apache.phoenix.util.MetaDataUtil;
 import org.apache.phoenix.util.PhoenixRuntime;
 import org.apache.phoenix.util.SchemaUtil;
 
 import java.io.IOException;
 import java.sql.Connection;
 import java.sql.SQLException;
-import java.util.ArrayList;
-import java.util.HashMap;
-import java.util.HashSet;
-import java.util.List;
-import java.util.Map;
-import java.util.Set;
+import java.util.*;

Review comment:
       Avoid import *

##########
File path: phoenix-tools/src/main/java/org/apache/phoenix/schema/SchemaExtractionProcessor.java
##########
@@ -250,12 +247,31 @@ private void setHTableProperties(HTableDescriptor htd) {
         }
     }
 
-    private void setHColumnFamilyProperties(HColumnDescriptor columnDescriptor) {
-        Map<ImmutableBytesWritable, ImmutableBytesWritable> propsMap = columnDescriptor.getValues();
-        for (Map.Entry<ImmutableBytesWritable, ImmutableBytesWritable> entry : propsMap.entrySet()) {
+    private void setHColumnFamilyProperties(HColumnDescriptor[] columnDescriptors) {
+        Map<ImmutableBytesWritable, ImmutableBytesWritable> propsMap = columnDescriptors[0].getValues();
+        for(Map.Entry<ImmutableBytesWritable, ImmutableBytesWritable> entry : propsMap.entrySet()){
             ImmutableBytesWritable key = entry.getKey();
-            ImmutableBytesWritable value = entry.getValue();
-            definedProps.put(Bytes.toString(key.get()), Bytes.toString(value.get()));
+            ImmutableBytesWritable defaultValue = entry.getValue();
+            Map<String, String> CFMap = new HashMap<String, String>();

Review comment:
       nit: cfMap

##########
File path: phoenix-tools/src/main/java/org/apache/phoenix/schema/SchemaExtractionProcessor.java
##########
@@ -250,12 +247,31 @@ private void setHTableProperties(HTableDescriptor htd) {
         }
     }
 
-    private void setHColumnFamilyProperties(HColumnDescriptor columnDescriptor) {
-        Map<ImmutableBytesWritable, ImmutableBytesWritable> propsMap = columnDescriptor.getValues();
-        for (Map.Entry<ImmutableBytesWritable, ImmutableBytesWritable> entry : propsMap.entrySet()) {
+    private void setHColumnFamilyProperties(HColumnDescriptor[] columnDescriptors) {
+        Map<ImmutableBytesWritable, ImmutableBytesWritable> propsMap = columnDescriptors[0].getValues();
+        for(Map.Entry<ImmutableBytesWritable, ImmutableBytesWritable> entry : propsMap.entrySet()){
             ImmutableBytesWritable key = entry.getKey();
-            ImmutableBytesWritable value = entry.getValue();
-            definedProps.put(Bytes.toString(key.get()), Bytes.toString(value.get()));
+            ImmutableBytesWritable defaultValue = entry.getValue();
+            Map<String, String> CFMap = new HashMap<String, String>();
+            Set<ImmutableBytesWritable> set = new HashSet<ImmutableBytesWritable>();
+            for(HColumnDescriptor columnDescriptor: columnDescriptors){

Review comment:
       space before `{`

##########
File path: phoenix-tools/src/main/java/org/apache/phoenix/schema/SchemaExtractionProcessor.java
##########
@@ -250,12 +247,31 @@ private void setHTableProperties(HTableDescriptor htd) {
         }
     }
 
-    private void setHColumnFamilyProperties(HColumnDescriptor columnDescriptor) {
-        Map<ImmutableBytesWritable, ImmutableBytesWritable> propsMap = columnDescriptor.getValues();
-        for (Map.Entry<ImmutableBytesWritable, ImmutableBytesWritable> entry : propsMap.entrySet()) {
+    private void setHColumnFamilyProperties(HColumnDescriptor[] columnDescriptors) {
+        Map<ImmutableBytesWritable, ImmutableBytesWritable> propsMap = columnDescriptors[0].getValues();
+        for(Map.Entry<ImmutableBytesWritable, ImmutableBytesWritable> entry : propsMap.entrySet()){
             ImmutableBytesWritable key = entry.getKey();
-            ImmutableBytesWritable value = entry.getValue();
-            definedProps.put(Bytes.toString(key.get()), Bytes.toString(value.get()));
+            ImmutableBytesWritable defaultValue = entry.getValue();
+            Map<String, String> CFMap = new HashMap<String, String>();
+            Set<ImmutableBytesWritable> set = new HashSet<ImmutableBytesWritable>();
+            for(HColumnDescriptor columnDescriptor: columnDescriptors){
+                String columnFamilyName = Bytes.toString(columnDescriptor.getName());
+                ImmutableBytesWritable value = columnDescriptor.getValues().get(key);
+                // check if it is universal properties
+                if (SYNCED_DATA_TABLE_AND_INDEX_COL_FAM_PROPERTIES.contains(Bytes.toString(key.get()))){
+                    definedProps.put(Bytes.toString(key.get()), Bytes.toString(value.get()));
+                    continue;
+                }
+                CFMap.put(columnFamilyName, Bytes.toString(value.get()));
+                set.add(value);
+            }
+            if (set.size() > 1){

Review comment:
       space before {

##########
File path: phoenix-tools/src/main/java/org/apache/phoenix/schema/SchemaExtractionProcessor.java
##########
@@ -250,12 +247,31 @@ private void setHTableProperties(HTableDescriptor htd) {
         }
     }
 
-    private void setHColumnFamilyProperties(HColumnDescriptor columnDescriptor) {
-        Map<ImmutableBytesWritable, ImmutableBytesWritable> propsMap = columnDescriptor.getValues();
-        for (Map.Entry<ImmutableBytesWritable, ImmutableBytesWritable> entry : propsMap.entrySet()) {
+    private void setHColumnFamilyProperties(HColumnDescriptor[] columnDescriptors) {
+        Map<ImmutableBytesWritable, ImmutableBytesWritable> propsMap = columnDescriptors[0].getValues();
+        for(Map.Entry<ImmutableBytesWritable, ImmutableBytesWritable> entry : propsMap.entrySet()){
             ImmutableBytesWritable key = entry.getKey();
-            ImmutableBytesWritable value = entry.getValue();
-            definedProps.put(Bytes.toString(key.get()), Bytes.toString(value.get()));
+            ImmutableBytesWritable defaultValue = entry.getValue();
+            Map<String, String> CFMap = new HashMap<String, String>();
+            Set<ImmutableBytesWritable> set = new HashSet<ImmutableBytesWritable>();
+            for(HColumnDescriptor columnDescriptor: columnDescriptors){
+                String columnFamilyName = Bytes.toString(columnDescriptor.getName());
+                ImmutableBytesWritable value = columnDescriptor.getValues().get(key);
+                // check if it is universal properties
+                if (SYNCED_DATA_TABLE_AND_INDEX_COL_FAM_PROPERTIES.contains(Bytes.toString(key.get()))){
+                    definedProps.put(Bytes.toString(key.get()), Bytes.toString(value.get()));
+                    continue;
+                }
+                CFMap.put(columnFamilyName, Bytes.toString(value.get()));
+                set.add(value);
+            }
+            if (set.size() > 1){

Review comment:
       space before `{` and at other similar spots

##########
File path: phoenix-tools/src/main/java/org/apache/phoenix/schema/SchemaExtractionProcessor.java
##########
@@ -250,12 +247,31 @@ private void setHTableProperties(HTableDescriptor htd) {
         }
     }
 
-    private void setHColumnFamilyProperties(HColumnDescriptor columnDescriptor) {
-        Map<ImmutableBytesWritable, ImmutableBytesWritable> propsMap = columnDescriptor.getValues();
-        for (Map.Entry<ImmutableBytesWritable, ImmutableBytesWritable> entry : propsMap.entrySet()) {
+    private void setHColumnFamilyProperties(HColumnDescriptor[] columnDescriptors) {
+        Map<ImmutableBytesWritable, ImmutableBytesWritable> propsMap = columnDescriptors[0].getValues();
+        for(Map.Entry<ImmutableBytesWritable, ImmutableBytesWritable> entry : propsMap.entrySet()){
             ImmutableBytesWritable key = entry.getKey();
-            ImmutableBytesWritable value = entry.getValue();
-            definedProps.put(Bytes.toString(key.get()), Bytes.toString(value.get()));
+            ImmutableBytesWritable defaultValue = entry.getValue();

Review comment:
       let's call it globalValue

##########
File path: phoenix-tools/src/main/java/org/apache/phoenix/schema/SchemaExtractionProcessor.java
##########
@@ -250,12 +247,31 @@ private void setHTableProperties(HTableDescriptor htd) {
         }
     }
 
-    private void setHColumnFamilyProperties(HColumnDescriptor columnDescriptor) {
-        Map<ImmutableBytesWritable, ImmutableBytesWritable> propsMap = columnDescriptor.getValues();
-        for (Map.Entry<ImmutableBytesWritable, ImmutableBytesWritable> entry : propsMap.entrySet()) {
+    private void setHColumnFamilyProperties(HColumnDescriptor[] columnDescriptors) {
+        Map<ImmutableBytesWritable, ImmutableBytesWritable> propsMap = columnDescriptors[0].getValues();
+        for(Map.Entry<ImmutableBytesWritable, ImmutableBytesWritable> entry : propsMap.entrySet()){
             ImmutableBytesWritable key = entry.getKey();
-            ImmutableBytesWritable value = entry.getValue();
-            definedProps.put(Bytes.toString(key.get()), Bytes.toString(value.get()));
+            ImmutableBytesWritable defaultValue = entry.getValue();
+            Map<String, String> CFMap = new HashMap<String, String>();
+            Set<ImmutableBytesWritable> set = new HashSet<ImmutableBytesWritable>();
+            for(HColumnDescriptor columnDescriptor: columnDescriptors){
+                String columnFamilyName = Bytes.toString(columnDescriptor.getName());
+                ImmutableBytesWritable value = columnDescriptor.getValues().get(key);
+                // check if it is universal properties
+                if (SYNCED_DATA_TABLE_AND_INDEX_COL_FAM_PROPERTIES.contains(Bytes.toString(key.get()))){
+                    definedProps.put(Bytes.toString(key.get()), Bytes.toString(value.get()));
+                    continue;
+                }
+                CFMap.put(columnFamilyName, Bytes.toString(value.get()));
+                set.add(value);
+            }
+            if (set.size() > 1){

Review comment:
       this would fail if two CF have same non-default value for `VERSIONS` or any other property

##########
File path: phoenix-tools/src/main/java/org/apache/phoenix/schema/SchemaExtractionProcessor.java
##########
@@ -250,12 +247,31 @@ private void setHTableProperties(HTableDescriptor htd) {
         }
     }
 
-    private void setHColumnFamilyProperties(HColumnDescriptor columnDescriptor) {
-        Map<ImmutableBytesWritable, ImmutableBytesWritable> propsMap = columnDescriptor.getValues();
-        for (Map.Entry<ImmutableBytesWritable, ImmutableBytesWritable> entry : propsMap.entrySet()) {
+    private void setHColumnFamilyProperties(HColumnDescriptor[] columnDescriptors) {
+        Map<ImmutableBytesWritable, ImmutableBytesWritable> propsMap = columnDescriptors[0].getValues();
+        for(Map.Entry<ImmutableBytesWritable, ImmutableBytesWritable> entry : propsMap.entrySet()){
             ImmutableBytesWritable key = entry.getKey();
-            ImmutableBytesWritable value = entry.getValue();
-            definedProps.put(Bytes.toString(key.get()), Bytes.toString(value.get()));
+            ImmutableBytesWritable defaultValue = entry.getValue();
+            Map<String, String> CFMap = new HashMap<String, String>();
+            Set<ImmutableBytesWritable> set = new HashSet<ImmutableBytesWritable>();
+            for(HColumnDescriptor columnDescriptor: columnDescriptors){
+                String columnFamilyName = Bytes.toString(columnDescriptor.getName());
+                ImmutableBytesWritable value = columnDescriptor.getValues().get(key);
+                // check if it is universal properties
+                if (SYNCED_DATA_TABLE_AND_INDEX_COL_FAM_PROPERTIES.contains(Bytes.toString(key.get()))){
+                    definedProps.put(Bytes.toString(key.get()), Bytes.toString(value.get()));
+                    continue;
+                }
+                CFMap.put(columnFamilyName, Bytes.toString(value.get()));
+                set.add(value);
+            }
+            if (set.size() > 1){

Review comment:
       this would fail if two CF have same non-default value for `VERSIONS` or any other property

##########
File path: phoenix-tools/src/main/java/org/apache/phoenix/schema/SchemaExtractionProcessor.java
##########
@@ -250,12 +247,31 @@ private void setHTableProperties(HTableDescriptor htd) {
         }
     }
 
-    private void setHColumnFamilyProperties(HColumnDescriptor columnDescriptor) {
-        Map<ImmutableBytesWritable, ImmutableBytesWritable> propsMap = columnDescriptor.getValues();
-        for (Map.Entry<ImmutableBytesWritable, ImmutableBytesWritable> entry : propsMap.entrySet()) {
+    private void setHColumnFamilyProperties(HColumnDescriptor[] columnDescriptors) {
+        Map<ImmutableBytesWritable, ImmutableBytesWritable> propsMap = columnDescriptors[0].getValues();
+        for(Map.Entry<ImmutableBytesWritable, ImmutableBytesWritable> entry : propsMap.entrySet()){
             ImmutableBytesWritable key = entry.getKey();
-            ImmutableBytesWritable value = entry.getValue();
-            definedProps.put(Bytes.toString(key.get()), Bytes.toString(value.get()));
+            ImmutableBytesWritable defaultValue = entry.getValue();
+            Map<String, String> CFMap = new HashMap<String, String>();
+            Set<ImmutableBytesWritable> set = new HashSet<ImmutableBytesWritable>();

Review comment:
       nit: cfPropertyValueSet

##########
File path: phoenix-tools/src/main/java/org/apache/phoenix/schema/SchemaExtractionProcessor.java
##########
@@ -250,12 +247,31 @@ private void setHTableProperties(HTableDescriptor htd) {
         }
     }
 
-    private void setHColumnFamilyProperties(HColumnDescriptor columnDescriptor) {
-        Map<ImmutableBytesWritable, ImmutableBytesWritable> propsMap = columnDescriptor.getValues();
-        for (Map.Entry<ImmutableBytesWritable, ImmutableBytesWritable> entry : propsMap.entrySet()) {
+    private void setHColumnFamilyProperties(HColumnDescriptor[] columnDescriptors) {
+        Map<ImmutableBytesWritable, ImmutableBytesWritable> propsMap = columnDescriptors[0].getValues();
+        for(Map.Entry<ImmutableBytesWritable, ImmutableBytesWritable> entry : propsMap.entrySet()){
             ImmutableBytesWritable key = entry.getKey();
-            ImmutableBytesWritable value = entry.getValue();
-            definedProps.put(Bytes.toString(key.get()), Bytes.toString(value.get()));
+            ImmutableBytesWritable defaultValue = entry.getValue();
+            Map<String, String> CFMap = new HashMap<String, String>();

Review comment:
       nit: cfMap or cfToPropertyValueMap

##########
File path: phoenix-tools/src/main/java/org/apache/phoenix/schema/SchemaExtractionProcessor.java
##########
@@ -250,12 +247,31 @@ private void setHTableProperties(HTableDescriptor htd) {
         }
     }
 
-    private void setHColumnFamilyProperties(HColumnDescriptor columnDescriptor) {
-        Map<ImmutableBytesWritable, ImmutableBytesWritable> propsMap = columnDescriptor.getValues();
-        for (Map.Entry<ImmutableBytesWritable, ImmutableBytesWritable> entry : propsMap.entrySet()) {
+    private void setHColumnFamilyProperties(HColumnDescriptor[] columnDescriptors) {
+        Map<ImmutableBytesWritable, ImmutableBytesWritable> propsMap = columnDescriptors[0].getValues();
+        for(Map.Entry<ImmutableBytesWritable, ImmutableBytesWritable> entry : propsMap.entrySet()){
             ImmutableBytesWritable key = entry.getKey();
-            ImmutableBytesWritable value = entry.getValue();
-            definedProps.put(Bytes.toString(key.get()), Bytes.toString(value.get()));
+            ImmutableBytesWritable defaultValue = entry.getValue();
+            Map<String, String> CFMap = new HashMap<String, String>();
+            Set<ImmutableBytesWritable> set = new HashSet<ImmutableBytesWritable>();
+            for(HColumnDescriptor columnDescriptor: columnDescriptors){
+                String columnFamilyName = Bytes.toString(columnDescriptor.getName());
+                ImmutableBytesWritable value = columnDescriptor.getValues().get(key);
+                // check if it is universal properties
+                if (SYNCED_DATA_TABLE_AND_INDEX_COL_FAM_PROPERTIES.contains(Bytes.toString(key.get()))){
+                    definedProps.put(Bytes.toString(key.get()), Bytes.toString(value.get()));
+                    continue;
+                }
+                CFMap.put(columnFamilyName, Bytes.toString(value.get()));
+                set.add(value);
+            }
+            if (set.size() > 1){

Review comment:
       Let's say there are 3 CFs and only 2 of them have VERSIONS=2. Would this work? Let's add a test if we don't have one to cover this scenario. 

##########
File path: phoenix-tools/src/main/java/org/apache/phoenix/schema/SchemaExtractionProcessor.java
##########
@@ -250,12 +247,31 @@ private void setHTableProperties(HTableDescriptor htd) {
         }
     }
 
-    private void setHColumnFamilyProperties(HColumnDescriptor columnDescriptor) {
-        Map<ImmutableBytesWritable, ImmutableBytesWritable> propsMap = columnDescriptor.getValues();
-        for (Map.Entry<ImmutableBytesWritable, ImmutableBytesWritable> entry : propsMap.entrySet()) {
+    private void setHColumnFamilyProperties(HColumnDescriptor[] columnDescriptors) {
+        Map<ImmutableBytesWritable, ImmutableBytesWritable> propsMap = columnDescriptors[0].getValues();
+        for(Map.Entry<ImmutableBytesWritable, ImmutableBytesWritable> entry : propsMap.entrySet()){
             ImmutableBytesWritable key = entry.getKey();
-            ImmutableBytesWritable value = entry.getValue();
-            definedProps.put(Bytes.toString(key.get()), Bytes.toString(value.get()));
+            ImmutableBytesWritable defaultValue = entry.getValue();
+            Map<String, String> CFMap = new HashMap<String, String>();
+            Set<ImmutableBytesWritable> set = new HashSet<ImmutableBytesWritable>();
+            for(HColumnDescriptor columnDescriptor: columnDescriptors){
+                String columnFamilyName = Bytes.toString(columnDescriptor.getName());
+                ImmutableBytesWritable value = columnDescriptor.getValues().get(key);
+                // check if it is universal properties
+                if (SYNCED_DATA_TABLE_AND_INDEX_COL_FAM_PROPERTIES.contains(Bytes.toString(key.get()))){
+                    definedProps.put(Bytes.toString(key.get()), Bytes.toString(value.get()));
+                    continue;
+                }
+                CFMap.put(columnFamilyName, Bytes.toString(value.get()));
+                set.add(value);

Review comment:
       I think we can maintain a map instead of [propertyValue and occurances] and execute line no. 273 only when exactly one entry from the map has occurrences equal to the no. of column families. 
   Also, good to refactor this logic in another function.

##########
File path: phoenix-tools/src/main/java/org/apache/phoenix/schema/SchemaExtractionProcessor.java
##########
@@ -250,12 +247,31 @@ private void setHTableProperties(HTableDescriptor htd) {
         }
     }
 
-    private void setHColumnFamilyProperties(HColumnDescriptor columnDescriptor) {
-        Map<ImmutableBytesWritable, ImmutableBytesWritable> propsMap = columnDescriptor.getValues();
-        for (Map.Entry<ImmutableBytesWritable, ImmutableBytesWritable> entry : propsMap.entrySet()) {
+    private void setHColumnFamilyProperties(HColumnDescriptor[] columnDescriptors) {
+        Map<ImmutableBytesWritable, ImmutableBytesWritable> propsMap = columnDescriptors[0].getValues();
+        for(Map.Entry<ImmutableBytesWritable, ImmutableBytesWritable> entry : propsMap.entrySet()){
             ImmutableBytesWritable key = entry.getKey();
-            ImmutableBytesWritable value = entry.getValue();
-            definedProps.put(Bytes.toString(key.get()), Bytes.toString(value.get()));
+            ImmutableBytesWritable defaultValue = entry.getValue();
+            Map<String, String> CFMap = new HashMap<String, String>();
+            Set<ImmutableBytesWritable> set = new HashSet<ImmutableBytesWritable>();
+            for(HColumnDescriptor columnDescriptor: columnDescriptors){
+                String columnFamilyName = Bytes.toString(columnDescriptor.getName());
+                ImmutableBytesWritable value = columnDescriptor.getValues().get(key);
+                // check if it is universal properties
+                if (SYNCED_DATA_TABLE_AND_INDEX_COL_FAM_PROPERTIES.contains(Bytes.toString(key.get()))){
+                    definedProps.put(Bytes.toString(key.get()), Bytes.toString(value.get()));
+                    continue;
+                }
+                CFMap.put(columnFamilyName, Bytes.toString(value.get()));
+                set.add(value);

Review comment:
       I think we can maintain a map of [propertyValue and occurances] and execute line no. 273 only when exactly one entry from the map has occurrences equal to the no. of column families. 
   Also, good to refactor this logic in another function.

##########
File path: phoenix-tools/src/main/java/org/apache/phoenix/schema/SchemaExtractionProcessor.java
##########
@@ -374,20 +397,49 @@ private String getColumnInfoStringForView(PTable table, PTable baseTable) {
 
     private String extractColumn(PColumn column) {
         String colName = column.getName().getString();
+        if (column.getFamilyName() != null){
+            String colFamilyName = column.getFamilyName().getString();
+            // check if it is default column family name
+            colName = colFamilyName.equals(QueryConstants.DEFAULT_COLUMN_FAMILY)? colName : String.format("\"%s\".\"%s\"", colFamilyName, colName);
+        }
+        boolean isArrayType = column.getDataType().isArrayType();
         String type = column.getDataType().getSqlTypeName();
+        Integer maxLength = column.getMaxLength();
+        Integer arrSize = column.getArraySize();
+        Integer scale = column.getScale();
         StringBuilder buf = new StringBuilder(colName);
         buf.append(' ');
-        buf.append(type);
-        Integer maxLength = column.getMaxLength();
-        if (maxLength != null) {
-            buf.append('(');
-            buf.append(maxLength);
-            Integer scale = column.getScale();
-            if (scale != null) {
-                buf.append(',');
-                buf.append(scale); // has both max length and scale. For ex- decimal(10,2)
+
+        if (isArrayType) {
+            String arrayPrefix = type.split("\\s+")[0];
+            buf.append(arrayPrefix);
+            if (maxLength != null) {

Review comment:
       nit: this if condition is duplicated in the else part. Please refactor it by extracting common parts and executing specific Array related logic when if clause passes. 

##########
File path: phoenix-tools/src/it/java/org/apache/phoenix/schema/SchemaExtractionToolIT.java
##########
@@ -210,4 +206,188 @@ public void testCreateViewIndexStatement() throws Exception {
             Assert.assertEquals(createIndexStatement.toUpperCase(), set.getOutput().toUpperCase());
         }
     }
+
+    @Test
+    public void testSaltedTableStatement() throws Exception {
+        String tableName = generateUniqueName();
+        String schemaName = generateUniqueName();
+        Properties props = PropertiesUtil.deepCopy(TEST_PROPERTIES);
+
+        try (Connection conn = DriverManager.getConnection(getUrl(), props)) {
+            String pTableFullName = SchemaUtil.getQualifiedTableName(schemaName, tableName);
+            String query = "create table " + pTableFullName +
+                    "(a_integer integer not null CONSTRAINT pk PRIMARY KEY (a_integer)) SALT_BUCKETS=16";
+            conn.createStatement().execute(query);
+            conn.commit();
+            String [] args = {"-tb", tableName, "-s", schemaName};
+            SchemaExtractionTool set = new SchemaExtractionTool();
+            set.setConf(conn.unwrap(PhoenixConnection.class).getQueryServices().getConfiguration());
+            set.run(args);
+            String actualProperties = set.getOutput().substring(set.getOutput().lastIndexOf(")")+1);
+            Assert.assertEquals(true, actualProperties.contains("SALT_BUCKETS=16"));
+        }
+    }
+
+    @Test
+    public void testCreateTableWithPKConstraint() throws Exception {
+        String tableName = generateUniqueName();
+        String schemaName = generateUniqueName();
+        Properties props = PropertiesUtil.deepCopy(TEST_PROPERTIES);
+
+        try (Connection conn = DriverManager.getConnection(getUrl(), props)) {
+            String pTableFullName = SchemaUtil.getQualifiedTableName(schemaName, tableName);
+            String query = "create table " + pTableFullName +
+                    "(a_char CHAR(15) NOT NULL, " +
+                    "b_char CHAR(15) NOT NULL, " +
+                    "c_bigint BIGINT NOT NULL CONSTRAINT PK PRIMARY KEY (a_char, b_char, c_bigint)) IMMUTABLE_ROWS=TRUE";
+            conn.createStatement().execute(query);
+            conn.commit();
+            String [] args = {"-tb", tableName, "-s", schemaName};
+            SchemaExtractionTool set = new SchemaExtractionTool();
+            set.setConf(conn.unwrap(PhoenixConnection.class).getQueryServices().getConfiguration());
+            set.run(args);
+
+            Assert.assertEquals(query.toUpperCase(), set.getOutput().toUpperCase());
+        }
+    }
+
+    @Test
+    public void testCreateTableWithArrayColumn() throws Exception {
+        String tableName = generateUniqueName();
+        String schemaName = generateUniqueName();
+        Properties props = PropertiesUtil.deepCopy(TEST_PROPERTIES);
+
+        try (Connection conn = DriverManager.getConnection(getUrl(), props)) {
+            String pTableFullName = SchemaUtil.getQualifiedTableName(schemaName, tableName);
+            String query = "create table " + pTableFullName +
+                    "(a_char CHAR(15) NOT NULL, " +
+                    "b_char CHAR(10) NOT NULL, " +
+                    "c_var_array VARCHAR ARRAY, " +
+                    "d_char_array CHAR(15) ARRAY[3] CONSTRAINT PK PRIMARY KEY (a_char, b_char)) " +
+                    "TTL=2592000, IMMUTABLE_STORAGE_SCHEME=ONE_CELL_PER_COLUMN, DISABLE_TABLE_SOR=true, REPLICATION_SCOPE=1";
+            conn.createStatement().execute(query);
+            conn.commit();
+            String[] args = {"-tb", tableName, "-s", schemaName};
+            SchemaExtractionTool set = new SchemaExtractionTool();
+            set.setConf(conn.unwrap(PhoenixConnection.class).getQueryServices().getConfiguration());
+            set.run(args);
+
+            Assert.assertEquals(query.toUpperCase(), set.getOutput().toUpperCase());
+        }
+    }
+
+    @Test
+    public void testCreateTableWithNonDefaultColumnFamily() throws Exception {
+        String tableName = generateUniqueName();
+        String schemaName = generateUniqueName();
+        Properties props = PropertiesUtil.deepCopy(TEST_PROPERTIES);
+
+        try (Connection conn = DriverManager.getConnection(getUrl(), props)) {
+            String pTableFullName = SchemaUtil.getQualifiedTableName(schemaName, tableName);
+            String query = "create table " + pTableFullName +
+                    "(a_char CHAR(15) NOT NULL, " +
+                    "b_char CHAR(10) NOT NULL, " +
+                    "\"av\".\"_\" CHAR(1), " +
+                    "\"bv\".\"_\" CHAR(1), " +
+                    "\"cv\".\"_\" CHAR(1), " +
+                    "\"dv\".\"_\" CHAR(1) CONSTRAINT PK PRIMARY KEY (a_char, b_char)) " +
+                    "TTL=1209600, IMMUTABLE_ROWS=true, IMMUTABLE_STORAGE_SCHEME=ONE_CELL_PER_COLUMN, SALT_BUCKETS=16, DISABLE_TABLE_SOR=true, MULTI_TENANT=true";
+            conn.createStatement().execute(query);
+            conn.commit();
+            String[] args = {"-tb", tableName, "-s", schemaName};
+            SchemaExtractionTool set = new SchemaExtractionTool();
+            set.setConf(conn.unwrap(PhoenixConnection.class).getQueryServices().getConfiguration());
+            set.run(args);
+
+            Assert.assertEquals(query.toUpperCase(), set.getOutput().toUpperCase());
+        }
+    }
+
+    @Test
+    public void testCreateTableWithUniversalCFProperties() throws Exception {
+        String tableName = generateUniqueName();
+        String schemaName = generateUniqueName();
+        Properties props = PropertiesUtil.deepCopy(TEST_PROPERTIES);
+        String properties = "KEEP_DELETED_CELLS=TRUE, TTL=1209600, IMMUTABLE_STORAGE_SCHEME=ONE_CELL_PER_COLUMN, REPLICATION_SCOPE=1";
+
+        try (Connection conn = DriverManager.getConnection(getUrl(), props)) {
+            String pTableFullName = SchemaUtil.getQualifiedTableName(schemaName, tableName);
+            String query = "create table " + pTableFullName +
+                    "(a_char CHAR(15) NOT NULL, " +
+                    "b_char CHAR(10) NOT NULL, " +
+                    "\"av\".\"_\" CHAR(1), " +
+                    "\"bv\".\"_\" CHAR(1), " +
+                    "\"cv\".\"_\" CHAR(1), " +
+                    "\"dv\".\"_\" CHAR(1) CONSTRAINT PK PRIMARY KEY (a_char, b_char)) " + properties;
+            conn.createStatement().execute(query);
+            conn.commit();
+            String[] args = {"-tb", tableName, "-s", schemaName};
+            SchemaExtractionTool set = new SchemaExtractionTool();
+            set.setConf(conn.unwrap(PhoenixConnection.class).getQueryServices().getConfiguration());
+            set.run(args);
+
+            Assert.assertEquals(query.toUpperCase(), set.getOutput().toUpperCase());
+        }
+    }
+
+
+    @Test
+    public void testCreateTableWithMultipleCFs() throws Exception {
+        String tableName = generateUniqueName();
+        String schemaName = generateUniqueName();
+        Properties props = PropertiesUtil.deepCopy(TEST_PROPERTIES);
+        String properties = "\"av\".VERSIONS=2, \"bv\".VERSIONS=3, " +
+                "\"cv\".VERSIONS=4, DATA_BLOCK_ENCODING=DIFF, " +
+                "IMMUTABLE_STORAGE_SCHEME=ONE_CELL_PER_COLUMN, SALT_BUCKETS=16, DISABLE_TABLE_SOR=true, MULTI_TENANT=true";
+
+        try (Connection conn = DriverManager.getConnection(getUrl(), props)) {
+            String pTableFullName = SchemaUtil.getQualifiedTableName(schemaName, tableName);
+            String query = "create table " + pTableFullName +
+                    "(a_char CHAR(15) NOT NULL, " +
+                    "b_char CHAR(10) NOT NULL, " +
+                    "\"av\".\"_\" CHAR(1), " +
+                    "\"bv\".\"_\" CHAR(1), " +
+                    "\"cv\".\"_\" CHAR(1), " +
+                    "\"dv\".\"_\" CHAR(1) CONSTRAINT PK PRIMARY KEY (a_char, b_char)) " + properties;
+            conn.createStatement().execute(query);
+            conn.commit();
+            String[] args = {"-tb", tableName, "-s", schemaName};
+            SchemaExtractionTool set = new SchemaExtractionTool();
+            set.setConf(conn.unwrap(PhoenixConnection.class).getQueryServices().getConfiguration());
+            set.run(args);
+
+            Assert.assertEquals(true, compareProperties(properties, set.getOutput().substring(set.getOutput().lastIndexOf(")")+1)));
+        }
+    }
+
+    @Test
+    public void testCreateIndexStatementWithColumnFamily() throws Exception {
+        String tableName = generateUniqueName();
+        String schemaName = generateUniqueName();
+        String indexName = generateUniqueName();
+        Properties props = PropertiesUtil.deepCopy(TEST_PROPERTIES);
+        try (Connection conn = DriverManager.getConnection(getUrl(), props)) {
+            String pTableFullName = SchemaUtil.getQualifiedTableName(schemaName, tableName);
+            conn.createStatement().execute("CREATE TABLE "+pTableFullName + "(k VARCHAR NOT NULL PRIMARY KEY, \"av\".\"_\" CHAR(1), v2 VARCHAR)");
+            String createIndexStatement = "CREATE INDEX "+ indexName + " ON "+pTableFullName+ "(\"av\".\"_\")";
+            conn.createStatement().execute(createIndexStatement);
+            conn.commit();
+
+            SchemaExtractionTool set = new SchemaExtractionTool();
+            set.setConf(conn.unwrap(PhoenixConnection.class).getQueryServices().getConfiguration());
+            String [] args2 = {"-tb", indexName, "-s", schemaName};
+            set.run(args2);
+            Assert.assertEquals(createIndexStatement.toUpperCase(), set.getOutput().toUpperCase());
+        }
+    }
+
+    private boolean compareProperties(String prop1, String prop2){

Review comment:
       Thanks for adding this.

##########
File path: phoenix-tools/src/it/java/org/apache/phoenix/schema/SchemaExtractionToolIT.java
##########
@@ -33,22 +30,21 @@ public void testCreateTableStatement() throws Exception {
         String tableName = generateUniqueName();
         String schemaName = generateUniqueName();
         Properties props = PropertiesUtil.deepCopy(TEST_PROPERTIES);
+        String pTableFullName = SchemaUtil.getQualifiedTableName(schemaName, tableName);
         String properties = "TTL=2592000,IMMUTABLE_ROWS=true,DISABLE_WAL=true";
+        String createTable = "CREATE TABLE "+ pTableFullName + "(K VARCHAR NOT NULL PRIMARY KEY, "
+                + "V1 VARCHAR, V2 VARCHAR) TTL=2592000, IMMUTABLE_ROWS=TRUE, DISABLE_WAL=TRUE";
 
         try (Connection conn = DriverManager.getConnection(getUrl(), props)) {
-
-            String pTableFullName = SchemaUtil.getQualifiedTableName(schemaName, tableName);
-            conn.createStatement().execute("CREATE TABLE "+ pTableFullName + "(k VARCHAR NOT NULL PRIMARY KEY, "
-                    + "v1 VARCHAR, v2 VARCHAR)"
-                    + properties);
+            conn.createStatement().execute(createTable);
             conn.commit();
             String [] args = {"-tb", tableName, "-s", schemaName};
 
             SchemaExtractionTool set = new SchemaExtractionTool();
             set.setConf(conn.unwrap(PhoenixConnection.class).getQueryServices().getConfiguration());
             set.run(args);
-            String actualProperties = set.getOutput().substring(set.getOutput().lastIndexOf(")")+1).replace(" ","");
-            Assert.assertEquals(3, actualProperties.split(",").length);
+            //String actualProperties = set.getOutput().substring(set.getOutput().lastIndexOf(")")+1).replace(" ","");

Review comment:
       nit: remove the commented code

##########
File path: phoenix-tools/src/it/java/org/apache/phoenix/schema/SchemaExtractionToolIT.java
##########
@@ -210,4 +206,188 @@ public void testCreateViewIndexStatement() throws Exception {
             Assert.assertEquals(createIndexStatement.toUpperCase(), set.getOutput().toUpperCase());
         }
     }
+
+    @Test
+    public void testSaltedTableStatement() throws Exception {
+        String tableName = generateUniqueName();
+        String schemaName = generateUniqueName();
+        Properties props = PropertiesUtil.deepCopy(TEST_PROPERTIES);
+
+        try (Connection conn = DriverManager.getConnection(getUrl(), props)) {
+            String pTableFullName = SchemaUtil.getQualifiedTableName(schemaName, tableName);
+            String query = "create table " + pTableFullName +
+                    "(a_integer integer not null CONSTRAINT pk PRIMARY KEY (a_integer)) SALT_BUCKETS=16";
+            conn.createStatement().execute(query);
+            conn.commit();
+            String [] args = {"-tb", tableName, "-s", schemaName};
+            SchemaExtractionTool set = new SchemaExtractionTool();
+            set.setConf(conn.unwrap(PhoenixConnection.class).getQueryServices().getConfiguration());
+            set.run(args);
+            String actualProperties = set.getOutput().substring(set.getOutput().lastIndexOf(")")+1);
+            Assert.assertEquals(true, actualProperties.contains("SALT_BUCKETS=16"));
+        }
+    }
+
+    @Test
+    public void testCreateTableWithPKConstraint() throws Exception {
+        String tableName = generateUniqueName();
+        String schemaName = generateUniqueName();
+        Properties props = PropertiesUtil.deepCopy(TEST_PROPERTIES);
+
+        try (Connection conn = DriverManager.getConnection(getUrl(), props)) {
+            String pTableFullName = SchemaUtil.getQualifiedTableName(schemaName, tableName);
+            String query = "create table " + pTableFullName +
+                    "(a_char CHAR(15) NOT NULL, " +
+                    "b_char CHAR(15) NOT NULL, " +
+                    "c_bigint BIGINT NOT NULL CONSTRAINT PK PRIMARY KEY (a_char, b_char, c_bigint)) IMMUTABLE_ROWS=TRUE";
+            conn.createStatement().execute(query);
+            conn.commit();
+            String [] args = {"-tb", tableName, "-s", schemaName};
+            SchemaExtractionTool set = new SchemaExtractionTool();
+            set.setConf(conn.unwrap(PhoenixConnection.class).getQueryServices().getConfiguration());
+            set.run(args);
+
+            Assert.assertEquals(query.toUpperCase(), set.getOutput().toUpperCase());
+        }
+    }
+
+    @Test
+    public void testCreateTableWithArrayColumn() throws Exception {
+        String tableName = generateUniqueName();
+        String schemaName = generateUniqueName();
+        Properties props = PropertiesUtil.deepCopy(TEST_PROPERTIES);
+
+        try (Connection conn = DriverManager.getConnection(getUrl(), props)) {
+            String pTableFullName = SchemaUtil.getQualifiedTableName(schemaName, tableName);
+            String query = "create table " + pTableFullName +
+                    "(a_char CHAR(15) NOT NULL, " +
+                    "b_char CHAR(10) NOT NULL, " +
+                    "c_var_array VARCHAR ARRAY, " +
+                    "d_char_array CHAR(15) ARRAY[3] CONSTRAINT PK PRIMARY KEY (a_char, b_char)) " +
+                    "TTL=2592000, IMMUTABLE_STORAGE_SCHEME=ONE_CELL_PER_COLUMN, DISABLE_TABLE_SOR=true, REPLICATION_SCOPE=1";
+            conn.createStatement().execute(query);
+            conn.commit();
+            String[] args = {"-tb", tableName, "-s", schemaName};
+            SchemaExtractionTool set = new SchemaExtractionTool();
+            set.setConf(conn.unwrap(PhoenixConnection.class).getQueryServices().getConfiguration());
+            set.run(args);
+
+            Assert.assertEquals(query.toUpperCase(), set.getOutput().toUpperCase());
+        }
+    }
+
+    @Test
+    public void testCreateTableWithNonDefaultColumnFamily() throws Exception {
+        String tableName = generateUniqueName();
+        String schemaName = generateUniqueName();
+        Properties props = PropertiesUtil.deepCopy(TEST_PROPERTIES);
+
+        try (Connection conn = DriverManager.getConnection(getUrl(), props)) {
+            String pTableFullName = SchemaUtil.getQualifiedTableName(schemaName, tableName);
+            String query = "create table " + pTableFullName +
+                    "(a_char CHAR(15) NOT NULL, " +
+                    "b_char CHAR(10) NOT NULL, " +
+                    "\"av\".\"_\" CHAR(1), " +
+                    "\"bv\".\"_\" CHAR(1), " +
+                    "\"cv\".\"_\" CHAR(1), " +
+                    "\"dv\".\"_\" CHAR(1) CONSTRAINT PK PRIMARY KEY (a_char, b_char)) " +
+                    "TTL=1209600, IMMUTABLE_ROWS=true, IMMUTABLE_STORAGE_SCHEME=ONE_CELL_PER_COLUMN, SALT_BUCKETS=16, DISABLE_TABLE_SOR=true, MULTI_TENANT=true";
+            conn.createStatement().execute(query);
+            conn.commit();
+            String[] args = {"-tb", tableName, "-s", schemaName};
+            SchemaExtractionTool set = new SchemaExtractionTool();
+            set.setConf(conn.unwrap(PhoenixConnection.class).getQueryServices().getConfiguration());
+            set.run(args);
+
+            Assert.assertEquals(query.toUpperCase(), set.getOutput().toUpperCase());
+        }
+    }
+
+    @Test
+    public void testCreateTableWithUniversalCFProperties() throws Exception {
+        String tableName = generateUniqueName();
+        String schemaName = generateUniqueName();
+        Properties props = PropertiesUtil.deepCopy(TEST_PROPERTIES);
+        String properties = "KEEP_DELETED_CELLS=TRUE, TTL=1209600, IMMUTABLE_STORAGE_SCHEME=ONE_CELL_PER_COLUMN, REPLICATION_SCOPE=1";
+
+        try (Connection conn = DriverManager.getConnection(getUrl(), props)) {
+            String pTableFullName = SchemaUtil.getQualifiedTableName(schemaName, tableName);
+            String query = "create table " + pTableFullName +
+                    "(a_char CHAR(15) NOT NULL, " +
+                    "b_char CHAR(10) NOT NULL, " +
+                    "\"av\".\"_\" CHAR(1), " +
+                    "\"bv\".\"_\" CHAR(1), " +
+                    "\"cv\".\"_\" CHAR(1), " +
+                    "\"dv\".\"_\" CHAR(1) CONSTRAINT PK PRIMARY KEY (a_char, b_char)) " + properties;
+            conn.createStatement().execute(query);
+            conn.commit();
+            String[] args = {"-tb", tableName, "-s", schemaName};
+            SchemaExtractionTool set = new SchemaExtractionTool();
+            set.setConf(conn.unwrap(PhoenixConnection.class).getQueryServices().getConfiguration());
+            set.run(args);
+
+            Assert.assertEquals(query.toUpperCase(), set.getOutput().toUpperCase());
+        }
+    }
+
+
+    @Test
+    public void testCreateTableWithMultipleCFs() throws Exception {
+        String tableName = generateUniqueName();
+        String schemaName = generateUniqueName();
+        Properties props = PropertiesUtil.deepCopy(TEST_PROPERTIES);
+        String properties = "\"av\".VERSIONS=2, \"bv\".VERSIONS=3, " +
+                "\"cv\".VERSIONS=4, DATA_BLOCK_ENCODING=DIFF, " +
+                "IMMUTABLE_STORAGE_SCHEME=ONE_CELL_PER_COLUMN, SALT_BUCKETS=16, DISABLE_TABLE_SOR=true, MULTI_TENANT=true";
+
+        try (Connection conn = DriverManager.getConnection(getUrl(), props)) {
+            String pTableFullName = SchemaUtil.getQualifiedTableName(schemaName, tableName);
+            String query = "create table " + pTableFullName +
+                    "(a_char CHAR(15) NOT NULL, " +
+                    "b_char CHAR(10) NOT NULL, " +
+                    "\"av\".\"_\" CHAR(1), " +
+                    "\"bv\".\"_\" CHAR(1), " +
+                    "\"cv\".\"_\" CHAR(1), " +
+                    "\"dv\".\"_\" CHAR(1) CONSTRAINT PK PRIMARY KEY (a_char, b_char)) " + properties;
+            conn.createStatement().execute(query);
+            conn.commit();
+            String[] args = {"-tb", tableName, "-s", schemaName};
+            SchemaExtractionTool set = new SchemaExtractionTool();
+            set.setConf(conn.unwrap(PhoenixConnection.class).getQueryServices().getConfiguration());
+            set.run(args);
+
+            Assert.assertEquals(true, compareProperties(properties, set.getOutput().substring(set.getOutput().lastIndexOf(")")+1)));

Review comment:
       can use assertTrue directly.

##########
File path: phoenix-tools/src/it/java/org/apache/phoenix/schema/SchemaExtractionToolIT.java
##########
@@ -33,22 +30,21 @@ public void testCreateTableStatement() throws Exception {
         String tableName = generateUniqueName();
         String schemaName = generateUniqueName();
         Properties props = PropertiesUtil.deepCopy(TEST_PROPERTIES);
+        String pTableFullName = SchemaUtil.getQualifiedTableName(schemaName, tableName);
         String properties = "TTL=2592000,IMMUTABLE_ROWS=true,DISABLE_WAL=true";

Review comment:
       nit: could you please remove this variable? It is not used anymore. 




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [phoenix] Qinrui98 commented on a change in pull request #848: Phoenix-5947 Add tests and extensions to Schema Extraction Tool utility

Posted by GitBox <gi...@apache.org>.
Qinrui98 commented on a change in pull request #848:
URL: https://github.com/apache/phoenix/pull/848#discussion_r463285405



##########
File path: phoenix-tools/src/main/java/org/apache/phoenix/schema/SchemaExtractionProcessor.java
##########
@@ -250,12 +247,31 @@ private void setHTableProperties(HTableDescriptor htd) {
         }
     }
 
-    private void setHColumnFamilyProperties(HColumnDescriptor columnDescriptor) {
-        Map<ImmutableBytesWritable, ImmutableBytesWritable> propsMap = columnDescriptor.getValues();
-        for (Map.Entry<ImmutableBytesWritable, ImmutableBytesWritable> entry : propsMap.entrySet()) {
+    private void setHColumnFamilyProperties(HColumnDescriptor[] columnDescriptors) {
+        Map<ImmutableBytesWritable, ImmutableBytesWritable> propsMap = columnDescriptors[0].getValues();
+        for(Map.Entry<ImmutableBytesWritable, ImmutableBytesWritable> entry : propsMap.entrySet()) {
             ImmutableBytesWritable key = entry.getKey();
-            ImmutableBytesWritable value = entry.getValue();
-            definedProps.put(Bytes.toString(key.get()), Bytes.toString(value.get()));
+            ImmutableBytesWritable globalValue = entry.getValue();
+            Map<String, String> cfToPropertyValueMap = new HashMap<String, String>();
+            Set<ImmutableBytesWritable> cfPropertyValueSet = new HashSet<ImmutableBytesWritable>();
+            for(HColumnDescriptor columnDescriptor: columnDescriptors) {
+                String columnFamilyName = Bytes.toString(columnDescriptor.getName());
+                ImmutableBytesWritable value = columnDescriptor.getValues().get(key);
+                // check if it is universal properties
+                if (SYNCED_DATA_TABLE_AND_INDEX_COL_FAM_PROPERTIES.contains(Bytes.toString(key.get()))) {
+                    definedProps.put(Bytes.toString(key.get()), Bytes.toString(value.get()));
+                    break;
+                }
+                cfToPropertyValueMap.put(columnFamilyName, Bytes.toString(value.get()));
+                cfPropertyValueSet.add(value);
+            }
+            if (cfPropertyValueSet.size() > 1) {
+                for(Map.Entry<String, String> mapEntry: cfToPropertyValueMap.entrySet()) {
+                    definedProps.put(String.format("%s.%s",  mapEntry.getKey(), Bytes.toString(key.get())), mapEntry.getValue());

Review comment:
       @gjacoby126 We are trying to find common CF property so that we don't need to specify it for every column family in the output string. Are you suggesting that we could use hashmap instead? 




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org