You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@phoenix.apache.org by st...@apache.org on 2021/06/18 13:16:44 UTC
[phoenix] branch 4.16 updated: PHOENIX-6271: Effective DDL
generated by SchemaExtractionTool should maintain the order of PK and other
columns (#1212)
This is an automated email from the ASF dual-hosted git repository.
stoty pushed a commit to branch 4.16
in repository https://gitbox.apache.org/repos/asf/phoenix.git
The following commit(s) were added to refs/heads/4.16 by this push:
new d4ea7d3 PHOENIX-6271: Effective DDL generated by SchemaExtractionTool should maintain the order of PK and other columns (#1212)
d4ea7d3 is described below
commit d4ea7d3832485b6f7cfa26508e8e72b25e9eb0de
Author: Swaroopa Kadam <sw...@gmail.com>
AuthorDate: Thu Apr 29 11:05:38 2021 -0700
PHOENIX-6271: Effective DDL generated by SchemaExtractionTool should maintain the order of PK and other columns (#1212)
Co-authored-by: Swaroopa Kadam <s....@apache.org>
---
.../java/org/apache/phoenix/util/SchemaUtil.java | 13 +-
.../phoenix/schema/SchemaExtractionToolIT.java | 159 +++++++++++++++++++--
.../phoenix/schema/SchemaExtractionProcessor.java | 68 +++++----
3 files changed, 194 insertions(+), 46 deletions(-)
diff --git a/phoenix-core/src/main/java/org/apache/phoenix/util/SchemaUtil.java b/phoenix-core/src/main/java/org/apache/phoenix/util/SchemaUtil.java
index 014ea24..49961f4 100644
--- a/phoenix-core/src/main/java/org/apache/phoenix/util/SchemaUtil.java
+++ b/phoenix-core/src/main/java/org/apache/phoenix/util/SchemaUtil.java
@@ -47,6 +47,7 @@ import java.util.TreeSet;
import javax.annotation.Nullable;
+import com.google.common.base.Strings;
import org.apache.hadoop.conf.Configuration;
import org.apache.hadoop.hbase.Cell;
import org.apache.hadoop.hbase.HColumnDescriptor;
@@ -1271,15 +1272,21 @@ public class SchemaUtil {
pTableName = "\""+pTableName+"\"";
}
if(tableNameNeedsQuotes || schemaNameNeedsQuotes) {
- pTableFullName = pSchemaName + "." + pTableName;
+ if (!Strings.isNullOrEmpty(pSchemaName)) {
+ return String.format("%s.%s", pSchemaName, pTableName);
+ } else {
+ return pTableName;
+ }
}
-
return pTableFullName;
}
private static boolean isQuotesNeeded(String name) {
// first char numeric or non-underscore
- if(!Character.isAlphabetic(name.charAt(0)) && name.charAt(0)!='_') {
+ if (Strings.isNullOrEmpty(name)) {
+ return false;
+ }
+ if (!Character.isAlphabetic(name.charAt(0)) && name.charAt(0)!='_') {
return true;
}
// for all other chars
diff --git a/phoenix-tools/src/it/java/org/apache/phoenix/schema/SchemaExtractionToolIT.java b/phoenix-tools/src/it/java/org/apache/phoenix/schema/SchemaExtractionToolIT.java
index 4f9b11b..34b09db 100644
--- a/phoenix-tools/src/it/java/org/apache/phoenix/schema/SchemaExtractionToolIT.java
+++ b/phoenix-tools/src/it/java/org/apache/phoenix/schema/SchemaExtractionToolIT.java
@@ -31,7 +31,9 @@ import org.junit.Test;
import java.sql.Connection;
import java.sql.DriverManager;
+import java.sql.ResultSet;
import java.sql.SQLException;
+import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.ArrayList;
@@ -154,18 +156,46 @@ public class SchemaExtractionToolIT extends ParallelStatsEnabledIT {
+ "id3 VARCHAR NOT NULL CONSTRAINT PKVIEW PRIMARY KEY (id2, id3 DESC)) "
+ "AS SELECT * FROM "+pTableFullName;
String createView1 = "CREATE VIEW "+childviewName + " AS SELECT * FROM "+viewFullName;
- String createIndexStatement = "CREATE INDEX "+indexName + " ON "+childviewName+"(id1) INCLUDE (v1)";
+ String createIndexStatement = "CREATE INDEX "+indexName + " ON "+childviewName+"(id2, id1) INCLUDE (v1)";
List<String> queries = new ArrayList<String>(){};
queries.add(createTableStmt);
queries.add(createView);
queries.add(createView1);
queries.add(createIndexStatement);
+ String expected = "CREATE INDEX %s ON " +childviewName +"(ID2, ID1, K, ID3 DESC) INCLUDE (V1)";
String result = runSchemaExtractionTool(schemaName, indexName, null, queries);
- Assert.assertEquals(createIndexStatement.toUpperCase(), result.toUpperCase());
+ Assert.assertEquals(String.format(expected, indexName).toUpperCase(), result.toUpperCase());
+ queries.clear();
+ String newIndex =indexName+"_NEW";
+ queries.add(String.format(expected, newIndex));
+ Properties props = PropertiesUtil.deepCopy(TEST_PROPERTIES);
+ try (Connection conn = DriverManager.getConnection(getUrl(), props)) {
+ executeCreateStatements(conn, queries);
+ }
+ compareOrdinalPositions(indexName, newIndex);
+ }
+
+ private void compareOrdinalPositions(String table, String newTable) throws SQLException {
+ String ordinalQuery = "SELECT COLUMN_NAME, "
+ + "ORDINAL_POSITION FROM SYSTEM.CATALOG"
+ + " WHERE TABLE_NAME='%s' AND ORDINAL_POSITION IS NOT NULL ORDER BY COLUMN_NAME";
+ Properties props = PropertiesUtil.deepCopy(TEST_PROPERTIES);
+ Map<String, Integer> ordinalMap = new HashMap<>();
+ try (Connection conn = DriverManager.getConnection(getUrl(), props)) {
+ ResultSet rs = conn.createStatement().executeQuery(String.format(ordinalQuery, table));
+ while(rs.next()) {
+ ordinalMap.put(rs.getString(1), rs.getInt(2));
+ }
+ rs = conn.createStatement().executeQuery(String.format(ordinalQuery, newTable));
+ while(rs.next()) {
+ Assert.assertEquals(ordinalMap.get(rs.getString(1)).intValue(),
+ rs.getInt(2));
+ }
+ }
}
@Test
- public void testCreateTableStatement_tenant() throws Exception {
+ public void testCreateViewStatement_tenant() throws Exception {
String tableName = generateUniqueName();
String viewName = generateUniqueName();
String schemaName = generateUniqueName();
@@ -218,8 +248,7 @@ public class SchemaExtractionToolIT extends ParallelStatsEnabledIT {
@Test
public void testCreateTableWithArrayColumn() throws Exception {
String tableName = generateUniqueName();
- String schemaName = generateUniqueName();
- String pTableFullName = SchemaUtil.getQualifiedTableName(schemaName, tableName);
+ String pTableFullName = tableName;
String query = "create table " + pTableFullName +
"(a_char CHAR(15) NOT NULL, " +
"b_char CHAR(10) NOT NULL, " +
@@ -228,7 +257,7 @@ public class SchemaExtractionToolIT extends ParallelStatsEnabledIT {
"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);
+ String result = runSchemaExtractionTool("", tableName, null, queries);
Assert.assertEquals(query.toUpperCase(), result.toUpperCase());
}
@@ -297,9 +326,13 @@ public class SchemaExtractionToolIT extends ParallelStatsEnabledIT {
public void testCreateTableWithMultipleCFProperties() throws Exception {
String tableName = "07"+generateUniqueName();
String schemaName = generateUniqueName();
- String properties = "\"av\".DATA_BLOCK_ENCODING='DIFF', \"bv\".DATA_BLOCK_ENCODING='DIFF', \"cv\".DATA_BLOCK_ENCODING='DIFF', " +
- "IMMUTABLE_STORAGE_SCHEME='ONE_CELL_PER_COLUMN', SALT_BUCKETS=16, MULTI_TENANT=true, BLOOMFITER='ROW'";
- String simplifiedProperties = "DATA_BLOCK_ENCODING='DIFF', IMMUTABLE_STORAGE_SCHEME='ONE_CELL_PER_COLUMN', SALT_BUCKETS=16, MULTI_TENANT=true, BLOOMFITER='ROW'";
+ String properties = "\"av\".DATA_BLOCK_ENCODING='DIFF', \"bv\".DATA_BLOCK_ENCODING='DIFF', "
+ + "\"cv\".DATA_BLOCK_ENCODING='DIFF', " +
+ "IMMUTABLE_STORAGE_SCHEME='ONE_CELL_PER_COLUMN', "
+ + "SALT_BUCKETS=16, MULTI_TENANT=true, BLOOMFITER='ROW'";
+ String simplifiedProperties = "DATA_BLOCK_ENCODING='DIFF', "
+ + "IMMUTABLE_STORAGE_SCHEME='ONE_CELL_PER_COLUMN', "
+ + "SALT_BUCKETS=16, MULTI_TENANT=true, BLOOMFITER='ROW'";
String query = "create table " + schemaName+".\""+tableName+"\"" +
"(a_char CHAR(15) NOT NULL, " +
"b_char CHAR(10) NOT NULL, " +
@@ -318,18 +351,119 @@ public class SchemaExtractionToolIT extends ParallelStatsEnabledIT {
}
@Test
+ public void testColumnAndPKOrdering() throws Exception {
+ String table = "CREATE TABLE IF NOT EXISTS MY_SCHEMA.MY_DATA_TABLE (\n"
+ + " ORGANIZATION_ID CHAR(15) NOT NULL, \n"
+ + " KEY_PREFIX CHAR(3) NOT NULL,\n"
+ + " CREATED_DATE DATE,\n"
+ + " CREATED_BY CHAR(15) \n"
+ + " CONSTRAINT PK PRIMARY KEY (\n"
+ + " ORGANIZATION_ID, \n"
+ + " KEY_PREFIX\n" + " )\n"
+ + ") VERSIONS=1, IMMUTABLE_ROWS=true, MULTI_TENANT=true, REPLICATION_SCOPE=1";
+
+ String view = "CREATE VIEW IF NOT EXISTS MY_SCHEMA.MY_DATA_VIEW (\n"
+ + " DATE_TIME1 DATE NOT NULL,\n"
+ + " TEXT1 VARCHAR NOT NULL,\n"
+ + " INT1 BIGINT NOT NULL,\n"
+ + " DOUBLE1 DECIMAL(12, 3),\n"
+ + " DOUBLE2 DECIMAL(12, 3),\n"
+ + " DOUBLE3 DECIMAL(12, 3),\n"
+ + " CONSTRAINT PKVIEW PRIMARY KEY\n" + " (\n"
+ + " DATE_TIME1, TEXT1, INT1\n" + " )\n" + ")\n"
+ + "AS SELECT * FROM MY_SCHEMA.MY_DATA_TABLE WHERE KEY_PREFIX = '9Yj'";
+
+ String index = "CREATE INDEX IF NOT EXISTS MY_VIEW_INDEX\n"
+ + "ON MY_SCHEMA.MY_DATA_VIEW (TEXT1, DATE_TIME1 DESC, DOUBLE1)\n"
+ + "INCLUDE (CREATED_BY, CREATED_DATE)";
+ List<String> queries = new ArrayList<String>(){};
+
+ queries.add(table);
+ queries.add(view);
+ queries.add(index);
+
+ String expectedIndex = "CREATE INDEX MY_VIEW_INDEX "
+ + "ON MY_SCHEMA.MY_DATA_VIEW(TEXT1, DATE_TIME1 DESC, DOUBLE1, INT1)"
+ + " INCLUDE (CREATED_BY, CREATED_DATE)";
+ String result = runSchemaExtractionTool("MY_SCHEMA", "MY_VIEW_INDEX", null, queries);
+ Assert.assertEquals(expectedIndex.toUpperCase(), result.toUpperCase());
+
+ String expectedView = "CREATE VIEW MY_SCHEMA.MY_DATA_VIEW(DATE_TIME1 DATE NOT NULL, "
+ + "TEXT1 VARCHAR NOT NULL, INT1 BIGINT NOT NULL, DOUBLE1 DECIMAL(12,3), "
+ + "DOUBLE2 DECIMAL(12,3), DOUBLE3 DECIMAL(12,3)"
+ + " CONSTRAINT PKVIEW PRIMARY KEY (DATE_TIME1, TEXT1, INT1))"
+ + " AS SELECT * FROM MY_SCHEMA.MY_DATA_TABLE WHERE KEY_PREFIX = '9YJ'";
+ result = runSchemaExtractionTool("MY_SCHEMA", "MY_DATA_VIEW", null, new ArrayList<String>());
+ Assert.assertEquals(expectedView.toUpperCase(), result.toUpperCase());
+ }
+
+ @Test
+ public void testColumnAndPKOrdering_nonView() throws Exception {
+ String indexName = "MY_DATA_TABLE_INDEX";
+ String table = "CREATE TABLE MY_SCHEMA.MY_SAMPLE_DATA_TABLE("
+ + "ORGANIZATION_ID CHAR(15) NOT NULL,"
+ + " SOME_ID_COLUMN CHAR(3) NOT NULL,"
+ + " SOME_ID_COLUMN_2 CHAR(15) NOT NULL,"
+ + " CREATED_DATE DATE NOT NULL,"
+ + " SOME_ID_COLUMN_3 CHAR(15) NOT NULL,"
+ + " SOME_ID_COLUMN_4 CHAR(15),"
+ + " CREATED_BY_ID VARCHAR,"
+ + " VALUE_FIELD VARCHAR"
+ + " CONSTRAINT PK PRIMARY KEY (ORGANIZATION_ID, SOME_ID_COLUMN, SOME_ID_COLUMN_2,"
+ + " CREATED_DATE DESC, SOME_ID_COLUMN_3))"
+ + " IMMUTABLE_ROWS=true, IMMUTABLE_STORAGE_SCHEME='ONE_CELL_PER_COLUMN',"
+ + " MULTI_TENANT=true, REPLICATION_SCOPE=1\n";
+
+ String index = "CREATE INDEX IF NOT EXISTS MY_DATA_TABLE_INDEX\n"
+ + " ON MY_SCHEMA.MY_SAMPLE_DATA_TABLE (SOME_ID_COLUMN, CREATED_DATE DESC,"
+ + " SOME_ID_COLUMN_2, SOME_ID_COLUMN_3)\n"
+ + " INCLUDE\n"
+ + "(SOME_ID_COLUMN_4, CREATED_BY_ID, VALUE_FIELD)\n";
+ List<String> queries = new ArrayList<String>(){};
+
+ queries.add(table);
+ queries.add(index);
+ String result = runSchemaExtractionTool("MY_SCHEMA",
+ "MY_DATA_TABLE_INDEX", null, queries);
+ String expected = "CREATE INDEX %s ON MY_SCHEMA.MY_SAMPLE_DATA_TABLE"
+ + "(SOME_ID_COLUMN, CREATED_DATE DESC, SOME_ID_COLUMN_2, SOME_ID_COLUMN_3) "
+ + "INCLUDE (SOME_ID_COLUMN_4, CREATED_BY_ID, VALUE_FIELD)";
+
+ Assert.assertEquals(String.format(expected, indexName).toUpperCase(), result.toUpperCase());
+ queries.clear();
+ String newIndex = indexName+"_NEW";
+ queries.add(String.format(expected, newIndex));
+ Properties props = PropertiesUtil.deepCopy(TEST_PROPERTIES);
+ try (Connection conn = DriverManager.getConnection(getUrl(), props)) {
+ executeCreateStatements(conn, queries);
+ }
+ compareOrdinalPositions(indexName, newIndex);
+ }
+
+ @Test
public void testCreateIndexStatementWithColumnFamily() throws Exception {
String tableName = generateUniqueName();
String schemaName = generateUniqueName();
String indexName = generateUniqueName();
String pTableFullName = SchemaUtil.getQualifiedTableName(schemaName, tableName);
- String createTableStmt = "CREATE TABLE "+pTableFullName + "(k VARCHAR NOT NULL PRIMARY KEY, \"av\".\"_\" CHAR(1), v2 VARCHAR)";
+ String createTableStmt = "CREATE TABLE "+pTableFullName + "(k VARCHAR NOT NULL PRIMARY KEY, "
+ + "\"av\".\"_\" CHAR(1), v2 VARCHAR)";
String createIndexStmt = "CREATE INDEX "+ indexName + " ON "+pTableFullName+ "(\"av\".\"_\")";
List<String> queries = new ArrayList<String>() {};
queries.add(createTableStmt);
queries.add(createIndexStmt);
+ //by the principle of having maximal columns in pk
+ String expected = "CREATE INDEX %s ON "+pTableFullName+ "(\"av\".\"_\", K)";
String result = runSchemaExtractionTool(schemaName, indexName, null, queries);
- Assert.assertEquals(createIndexStmt.toUpperCase(), result.toUpperCase());
+ Assert.assertEquals(String.format(expected, indexName).toUpperCase(), result.toUpperCase());
+ queries.clear();
+ String newIndex = indexName+"_NEW";
+ queries.add(String.format(expected, newIndex));
+ Properties props = PropertiesUtil.deepCopy(TEST_PROPERTIES);
+ try (Connection conn = DriverManager.getConnection(getUrl(), props)) {
+ executeCreateStatements(conn, queries);
+ }
+ compareOrdinalPositions(indexName, newIndex);
}
private Connection getTenantConnection(String url, String tenantId) throws SQLException {
@@ -338,7 +472,8 @@ public class SchemaExtractionToolIT extends ParallelStatsEnabledIT {
return DriverManager.getConnection(url, props);
}
- private String runSchemaExtractionTool(String schemaName, String tableName, String tenantId, List<String> queries) throws Exception {
+ private String runSchemaExtractionTool(String schemaName, String tableName, String tenantId,
+ List<String> queries) throws Exception {
Properties props = PropertiesUtil.deepCopy(TEST_PROPERTIES);
String output;
if (tenantId == null) {
diff --git a/phoenix-tools/src/main/java/org/apache/phoenix/schema/SchemaExtractionProcessor.java b/phoenix-tools/src/main/java/org/apache/phoenix/schema/SchemaExtractionProcessor.java
index 93632cb..33095fc 100644
--- a/phoenix-tools/src/main/java/org/apache/phoenix/schema/SchemaExtractionProcessor.java
+++ b/phoenix-tools/src/main/java/org/apache/phoenix/schema/SchemaExtractionProcessor.java
@@ -16,11 +16,7 @@
* limitations under the License.
*/
package org.apache.phoenix.schema;
-
-import com.google.common.base.Strings;
-import com.google.common.collect.Sets;
import org.apache.commons.lang.StringUtils;
-import org.apache.commons.lang.math.NumberUtils;
import org.apache.hadoop.conf.Configuration;
import org.apache.hadoop.hbase.HColumnDescriptor;
import org.apache.hadoop.hbase.HTableDescriptor;
@@ -107,40 +103,60 @@ public class SchemaExtractionProcessor {
List<PColumn> indexPK = indexPTable.getPKColumns();
List<PColumn> dataPK = dataPTable.getPKColumns();
- Set<String> indexPkSet = new HashSet<>();
- Set<String> dataPkSet = new HashSet<>();
- Map<String, SortOrder> sortOrderMap = new HashMap<>();
+ List<String> indexPKName = new ArrayList<>();
+ List<String> dataPKName = new ArrayList<>();
+ Map<String, SortOrder> indexSortOrderMap = new HashMap<>();
StringBuilder indexedColumnsBuilder = new StringBuilder();
+
for (PColumn indexedColumn : indexPK) {
String indexColumn = extractIndexColumn(indexedColumn.getName().getString(), defaultCF);
if(indexColumn.equalsIgnoreCase(MetaDataUtil.VIEW_INDEX_ID_COLUMN_NAME)) {
continue;
}
- indexPkSet.add(indexColumn);
- sortOrderMap.put(indexColumn, indexedColumn.getSortOrder());
+ indexPKName.add(indexColumn);
+ indexSortOrderMap.put(indexColumn, indexedColumn.getSortOrder());
}
-
for(PColumn pColumn : dataPK) {
- dataPkSet.add(pColumn.getName().getString());
+ dataPKName.add(pColumn.getName().getString());
}
- Set<String> effectivePK = Sets.symmetricDifference(indexPkSet, dataPkSet);
- if (effectivePK.isEmpty()) {
- effectivePK = indexPkSet;
+ // This is added because of PHOENIX-2340
+ String tenantIdColumn = dataPKName.get(0);
+ if (dataPTable.isMultiTenant() && indexPKName.contains(tenantIdColumn)) {
+ indexPKName.remove(tenantIdColumn);
}
- for (String column : effectivePK) {
+
+ for (String column : indexPKName) {
if(indexedColumnsBuilder.length()!=0) {
indexedColumnsBuilder.append(", ");
}
indexedColumnsBuilder.append(column);
- if(sortOrderMap.containsKey(column) && sortOrderMap.get(column) != SortOrder.getDefault()) {
+ if(indexSortOrderMap.containsKey(column)
+ && indexSortOrderMap.get(column) != SortOrder.getDefault()) {
indexedColumnsBuilder.append(" ");
- indexedColumnsBuilder.append(sortOrderMap.get(column));
+ indexedColumnsBuilder.append(indexSortOrderMap.get(column));
}
}
return indexedColumnsBuilder.toString();
}
+ private List<PColumn> getSymmetricDifferencePColumns(List<PColumn> firstList, List<PColumn> secondList) {
+ List<PColumn> effectivePK = new ArrayList<>();
+ for(PColumn column : firstList) {
+ if(secondList.contains(column)) {
+ continue;
+ }
+ effectivePK.add(column);
+ }
+ for(PColumn column : secondList) {
+ if(firstList.contains(column)) {
+ continue;
+ }
+ effectivePK.add(column);
+ }
+ return effectivePK;
+ }
+
private String extractIndexColumn(String columnName, String defaultCF) {
String [] columnNameSplit = columnName.split(":");
if(columnNameSplit[0].equals("") || columnNameSplit[0].equalsIgnoreCase(defaultCF)) {
@@ -273,8 +289,8 @@ public class SchemaExtractionProcessor {
for(Map.Entry<ImmutableBytesWritable, ImmutableBytesWritable> entry : propsMap.entrySet()) {
ImmutableBytesWritable key = entry.getKey();
ImmutableBytesWritable globalValue = entry.getValue();
- Map<String, String> cfToPropertyValueMap = new HashMap<String, String>();
- Set<ImmutableBytesWritable> cfPropertyValueSet = new HashSet<ImmutableBytesWritable>();
+ Map<String, String> cfToPropertyValueMap = new HashMap<>();
+ Set<ImmutableBytesWritable> cfPropertyValueSet = new HashSet<>();
for(HColumnDescriptor columnDescriptor: columnDescriptors) {
String columnFamilyName = Bytes.toString(columnDescriptor.getName());
ImmutableBytesWritable value = columnDescriptor.getValues().get(key);
@@ -406,21 +422,11 @@ public class SchemaExtractionProcessor {
List<PColumn> columns = table.getColumns();
List<PColumn> pkColumns = table.getPKColumns();
- Set<PColumn> columnSet = new HashSet<>(columns);
- Set<PColumn> pkSet = new HashSet<>(pkColumns);
-
List<PColumn> baseColumns = baseTable.getColumns();
List<PColumn> basePkColumns = baseTable.getPKColumns();
- Set<PColumn> baseColumnSet = new HashSet<>(baseColumns);
- Set<PColumn> basePkSet = new HashSet<>(basePkColumns);
-
- Set<PColumn> columnsSet = Sets.symmetricDifference(baseColumnSet, columnSet);
- Set<PColumn> pkColumnsSet = Sets.symmetricDifference(basePkSet, pkSet);
-
- columns = new ArrayList<>(columnsSet);
- pkColumns = new ArrayList<>(pkColumnsSet);
-
+ columns = getSymmetricDifferencePColumns(baseColumns, columns);
+ pkColumns = getSymmetricDifferencePColumns(basePkColumns, pkColumns);
return getColumnInfoString(table, colInfo, columns, pkColumns);
}