Skip to content

PHOENIX-6271: Effective DDL generated by SchemaExtractionTool should …#1251

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
stoty wants to merge 1 commit into from
Closed

PHOENIX-6271: Effective DDL generated by SchemaExtractionTool should … #1251

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 10 additions & 3 deletions phoenix-core/src/main/java/org/apache/phoenix/util/SchemaUtil.java
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -98,6 +98,7 @@
import org.apache.phoenix.thirdparty.com.google.common.collect.Iterables;
import org.apache.phoenix.thirdparty.com.google.common.collect.Lists;
import org.apache.phoenix.thirdparty.com.google.common.collect.Maps;
import org.apache.phoenix.thirdparty.com.google.common.base.Strings;

/**
*
Expand DownExpand Up@@ -1253,15 +1254,21 @@ public static String getPTableFullNameWithQuotes(String pSchemaName, String pTab
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
Expand Down
4 changes: 0 additions & 4 deletions phoenix-tools/pom.xml
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -47,10 +47,6 @@
<groupId>org.apache.hadoop</groupId>
<artifactId>hadoop-common</artifactId>
</dependency>
<dependency>
<groupId>org.apache.phoenix.thirdparty</groupId>
<artifactId>phoenix-shaded-guava</artifactId>
</dependency>
<dependency>
<groupId>org.apache.commons</groupId>
<artifactId>commons-lang3</artifactId>
Expand Down
159 changes: 147 additions & 12 deletions phoenix-tools/src/it/java/org/apache/phoenix/schema/SchemaExtractionToolIT.java
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -31,7 +31,9 @@

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;
Expand DownExpand Up@@ -154,18 +156,46 @@ public void testCreateViewIndexStatement() throws Exception {
+ "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();
Expand DownExpand Up@@ -218,8 +248,7 @@ public void testCreateTableWithPKConstraint() throws Exception {
@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, " +
Expand All@@ -228,7 +257,7 @@ public void testCreateTableWithArrayColumn() throws Exception {
"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());
}

Expand DownExpand Up@@ -297,9 +326,13 @@ public void testCreateTableWithMultipleCF() throws Exception {
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, " +
Expand All@@ -317,19 +350,120 @@ public void testCreateTableWithMultipleCFProperties() throws Exception {
Assert.assertTrue(compareProperties(simplifiedProperties, getProperties(result)));
}

@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 {
Expand All@@ -338,7 +472,8 @@ private Connection getTenantConnection(String url, String tenantId) throws SQLEx
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) {
Expand Down
61 changes: 35 additions & 26 deletions phoenix-tools/src/main/java/org/apache/phoenix/schema/SchemaExtractionProcessor.java
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -29,7 +29,6 @@
import org.apache.phoenix.mapreduce.util.ConnectionUtil;
import org.apache.phoenix.query.ConnectionQueryServices;
import org.apache.phoenix.query.QueryConstants;
import org.apache.phoenix.thirdparty.com.google.common.collect.Sets;
import org.apache.phoenix.util.MetaDataUtil;
import org.apache.phoenix.util.PhoenixRuntime;
import org.apache.phoenix.util.SchemaUtil;
Expand DownExpand Up@@ -105,40 +104,60 @@ private String getIndexedColumnsString(PTable indexPTable, PTable dataPTable, St

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)) {
Expand DownExpand Up@@ -404,21 +423,11 @@ private String getColumnInfoStringForView(PTable table, PTable baseTable) {
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);
}
Expand Down