From 4e0b94eb3c334c73a57096248656010e84a2e628 Mon Sep 17 00:00:00 2001 From: Hyunsik Choi Date: Fri, 4 Sep 2015 12:32:44 +0900 Subject: [PATCH 1/5] TAJO-1674: Validation of CTAS schema mismatch. --- .../engine/query/TestTablePartitions.java | 32 +++---------------- .../negative/type_mismatch.sql | 2 ++ .../plan/verifier/LogicalPlanVerifier.java | 8 ++++- 3 files changed, 14 insertions(+), 28 deletions(-) create mode 100644 tajo-core-tests/src/test/resources/queries/TestCreateTable/negative/type_mismatch.sql diff --git a/tajo-core-tests/src/test/java/org/apache/tajo/engine/query/TestTablePartitions.java b/tajo-core-tests/src/test/java/org/apache/tajo/engine/query/TestTablePartitions.java index 52e7b54cfa..94e5e71602 100644 --- a/tajo-core-tests/src/test/java/org/apache/tajo/engine/query/TestTablePartitions.java +++ b/tajo-core-tests/src/test/java/org/apache/tajo/engine/query/TestTablePartitions.java @@ -37,14 +37,11 @@ import org.apache.tajo.engine.planner.global.ExecutionBlock; import org.apache.tajo.engine.planner.global.MasterPlan; import org.apache.tajo.ipc.ClientProtos; -import org.apache.tajo.jdbc.FetchResultSet; -import org.apache.tajo.jdbc.TajoMemoryResultSet; import org.apache.tajo.plan.logical.NodeType; import org.apache.tajo.querymaster.QueryMasterTask; import org.apache.tajo.storage.StorageConstants; import org.apache.tajo.util.CommonTestingUtil; import org.apache.tajo.util.KeyValueSet; -import org.apache.tajo.worker.TajoWorker; import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.Parameterized; @@ -211,7 +208,7 @@ public final void testCreateColumnPartitionedTableWithSelectedColumns() throws E TableDesc tableDesc = catalog.getTableDesc(DEFAULT_DATABASE_NAME, tableName); verifyPartitionDirectoryFromCatalog(DEFAULT_DATABASE_NAME, tableName, new String[]{"key"}, - tableDesc.getStats().getNumRows()); + tableDesc.getStats().getNumRows()); executeString("DROP TABLE " + tableName + " PURGE").close(); } @@ -254,7 +251,7 @@ public final void testColumnPartitionedTableByOneColumn() throws Exception { } verifyPartitionDirectoryFromCatalog(DEFAULT_DATABASE_NAME, tableName, - new String[]{"key"}, desc.getStats().getNumRows()); + new String[]{"key"}, desc.getStats().getNumRows()); executeString("DROP TABLE " + tableName + " PURGE").close(); res.close(); @@ -674,7 +671,7 @@ public final void testColumnPartitionedTableByOneColumnsWithCompression() throws } verifyPartitionDirectoryFromCatalog(DEFAULT_DATABASE_NAME, tableName, new String[]{"col1"}, - desc.getStats().getNumRows()); + desc.getStats().getNumRows()); executeString("DROP TABLE " + tableName + " PURGE").close(); } @@ -733,7 +730,7 @@ public final void testColumnPartitionedTableByTwoColumnsWithCompression() throws } verifyPartitionDirectoryFromCatalog(DEFAULT_DATABASE_NAME, tableName, new String[]{"col1", "col2"}, - desc.getStats().getNumRows()); + desc.getStats().getNumRows()); executeString("DROP TABLE " + tableName + " PURGE").close(); } @@ -1045,7 +1042,7 @@ public final void testColumnPartitionedTableWithSmallerExpressions5() throws Exc TableDesc desc = catalog.getTableDesc(DEFAULT_DATABASE_NAME, tableName); verifyPartitionDirectoryFromCatalog(DEFAULT_DATABASE_NAME, tableName, new String[]{"col2"}, - desc.getStats().getNumRows()); + desc.getStats().getNumRows()); executeString("DROP TABLE " + tableName + " PURGE").close(); } @@ -1081,25 +1078,6 @@ public final void testColumnPartitionedTableWithSmallerExpressions6() throws Exc executeString("DROP TABLE " + tableName + " PURGE").close(); } - private MasterPlan getQueryPlan(ResultSet res) { - QueryId queryId; - if (res instanceof TajoMemoryResultSet) { - queryId = ((TajoMemoryResultSet) res).getQueryId(); - } else { - queryId = ((FetchResultSet) res).getQueryId(); - } - - for (TajoWorker eachWorker : testingCluster.getTajoWorkers()) { - QueryMasterTask queryMasterTask = eachWorker.getWorkerContext().getQueryMaster().getQueryMasterTask(queryId, true); - if (queryMasterTask != null) { - return queryMasterTask.getQuery().getPlan(); - } - } - - fail("Can't find query from workers" + queryId); - return null; - } - @Test public void testScatteredHashShuffle() throws Exception { testingCluster.setAllTajoDaemonConfValue(TajoConf.ConfVars.$DIST_QUERY_TABLE_PARTITION_VOLUME.varname, "2"); diff --git a/tajo-core-tests/src/test/resources/queries/TestCreateTable/negative/type_mismatch.sql b/tajo-core-tests/src/test/resources/queries/TestCreateTable/negative/type_mismatch.sql new file mode 100644 index 0000000000..1bff25cd0d --- /dev/null +++ b/tajo-core-tests/src/test/resources/queries/TestCreateTable/negative/type_mismatch.sql @@ -0,0 +1,2 @@ +CREATE TABLE MISMATCH1 (n_name text, n_comment text, n_nationkey int8, n_regionkey int8) AS SELECT * FROM default.nation; +CREATE TABLE MISMATCH2 (n_name text, n_comment text) PARTITION BY COLUMN (n_nationkey int8, n_regionkey int8) AS SELECT * FROM default.nation; diff --git a/tajo-plan/src/main/java/org/apache/tajo/plan/verifier/LogicalPlanVerifier.java b/tajo-plan/src/main/java/org/apache/tajo/plan/verifier/LogicalPlanVerifier.java index 0b0968e747..9b6ac25674 100644 --- a/tajo-plan/src/main/java/org/apache/tajo/plan/verifier/LogicalPlanVerifier.java +++ b/tajo-plan/src/main/java/org/apache/tajo/plan/verifier/LogicalPlanVerifier.java @@ -19,6 +19,7 @@ package org.apache.tajo.plan.verifier; import com.google.common.base.Preconditions; +import org.apache.tajo.catalog.CatalogUtil; import org.apache.tajo.catalog.Column; import org.apache.tajo.catalog.Schema; import org.apache.tajo.common.TajoDataTypes.Type; @@ -241,7 +242,8 @@ public LogicalNode visitInsert(Context context, LogicalPlan plan, LogicalPlan.Qu private static void ensureDomains(VerificationState state, Schema targetTableScheme, Schema schema) throws TajoException { for (int i = 0; i < schema.size(); i++) { - if (!schema.getColumn(i).getDataType().equals(targetTableScheme.getColumn(i).getDataType())) { + if (!CatalogUtil.isCompatibleType( + schema.getColumn(i).getDataType().getType(), targetTableScheme.getColumn(i).getDataType().getType())) { Column targetColumn = targetTableScheme.getColumn(i); Column insertColumn = schema.getColumn(i); state.addVerification(makeDataTypeMisMatch(insertColumn, targetColumn)); @@ -254,6 +256,10 @@ public LogicalNode visitCreateTable(Context context, LogicalPlan plan, LogicalPl CreateTableNode node, Stack stack) throws TajoException { super.visitCreateTable(context, plan, block, node, stack); // here, we don't need check table existence because this check is performed in PreLogicalPlanVerifier. + + if (node.hasSubQuery()) { + ensureDomains(context.state, node.getLogicalSchema(), node.getChild(0).getOutSchema()); + } return node; } From 261a1e51978611c9a0afce27bd9e06bd4100db83 Mon Sep 17 00:00:00 2001 From: Hyunsik Choi Date: Fri, 4 Sep 2015 14:48:23 +0900 Subject: [PATCH 2/5] Fixed test failures. --- .../java/org/apache/tajo/catalog/CatalogUtil.java | 15 +++++++++++++-- .../tajo/plan/verifier/LogicalPlanVerifier.java | 6 +++++- .../tajo/plan/verifier/SyntaxErrorUtil.java | 5 +++-- 3 files changed, 21 insertions(+), 5 deletions(-) diff --git a/tajo-catalog/tajo-catalog-common/src/main/java/org/apache/tajo/catalog/CatalogUtil.java b/tajo-catalog/tajo-catalog-common/src/main/java/org/apache/tajo/catalog/CatalogUtil.java index ac76387394..a6487b444b 100644 --- a/tajo-catalog/tajo-catalog-common/src/main/java/org/apache/tajo/catalog/CatalogUtil.java +++ b/tajo-catalog/tajo-catalog-common/src/main/java/org/apache/tajo/catalog/CatalogUtil.java @@ -873,7 +873,11 @@ public static Pair, String> getPartitionKeyNamePair(Stri return pair; } - /* It is the relationship graph of type conversions. */ + /* + * It is the relationship graph of type conversions. + * It contains tuples, each of which (LHS type, RHS type, Result type). + */ + public static final Map> OPERATION_CASTING_MAP = Maps.newHashMap(); static { @@ -929,9 +933,16 @@ public static Pair, String> getPartitionKeyNamePair(Stri TUtil.putToNestedMap(OPERATION_CASTING_MAP, Type.FLOAT8, Type.TEXT, Type.TEXT); TUtil.putToNestedMap(OPERATION_CASTING_MAP, Type.TEXT, Type.TIMESTAMP, Type.TIMESTAMP); + TUtil.putToNestedMap(OPERATION_CASTING_MAP, Type.TEXT, Type.TEXT, Type.TEXT); + TUtil.putToNestedMap(OPERATION_CASTING_MAP, Type.TEXT, Type.VARCHAR, Type.TEXT); + + TUtil.putToNestedMap(OPERATION_CASTING_MAP, Type.VARCHAR, Type.TIMESTAMP, Type.TIMESTAMP); + TUtil.putToNestedMap(OPERATION_CASTING_MAP, Type.VARCHAR, Type.TEXT, Type.TEXT); + TUtil.putToNestedMap(OPERATION_CASTING_MAP, Type.VARCHAR, Type.VARCHAR, Type.TEXT); + TUtil.putToNestedMap(OPERATION_CASTING_MAP, Type.TIMESTAMP, Type.TIMESTAMP, Type.TIMESTAMP); TUtil.putToNestedMap(OPERATION_CASTING_MAP, Type.TIMESTAMP, Type.TEXT, Type.TEXT); - TUtil.putToNestedMap(OPERATION_CASTING_MAP, Type.TEXT, Type.TEXT, Type.TEXT); + TUtil.putToNestedMap(OPERATION_CASTING_MAP, Type.TIMESTAMP, Type.VARCHAR, Type.TEXT); TUtil.putToNestedMap(OPERATION_CASTING_MAP, Type.TIME, Type.TIME, Type.TIME); TUtil.putToNestedMap(OPERATION_CASTING_MAP, Type.DATE, Type.DATE, Type.DATE); diff --git a/tajo-plan/src/main/java/org/apache/tajo/plan/verifier/LogicalPlanVerifier.java b/tajo-plan/src/main/java/org/apache/tajo/plan/verifier/LogicalPlanVerifier.java index 9b6ac25674..0d0274efc1 100644 --- a/tajo-plan/src/main/java/org/apache/tajo/plan/verifier/LogicalPlanVerifier.java +++ b/tajo-plan/src/main/java/org/apache/tajo/plan/verifier/LogicalPlanVerifier.java @@ -19,6 +19,7 @@ package org.apache.tajo.plan.verifier; import com.google.common.base.Preconditions; +import com.sun.org.apache.xml.internal.resolver.Catalog; import org.apache.tajo.catalog.CatalogUtil; import org.apache.tajo.catalog.Column; import org.apache.tajo.catalog.Schema; @@ -32,6 +33,7 @@ import org.apache.tajo.plan.logical.*; import org.apache.tajo.plan.util.PlannerUtil; import org.apache.tajo.plan.visitor.BasicLogicalPlanVisitor; +import org.apache.tajo.util.TUtil; import java.util.Stack; @@ -242,7 +244,9 @@ public LogicalNode visitInsert(Context context, LogicalPlan plan, LogicalPlan.Qu private static void ensureDomains(VerificationState state, Schema targetTableScheme, Schema schema) throws TajoException { for (int i = 0; i < schema.size(); i++) { - if (!CatalogUtil.isCompatibleType( + + // checking castable between two data types + if (!TUtil.containsInNestedMap(CatalogUtil.OPERATION_CASTING_MAP, schema.getColumn(i).getDataType().getType(), targetTableScheme.getColumn(i).getDataType().getType())) { Column targetColumn = targetTableScheme.getColumn(i); Column insertColumn = schema.getColumn(i); diff --git a/tajo-plan/src/main/java/org/apache/tajo/plan/verifier/SyntaxErrorUtil.java b/tajo-plan/src/main/java/org/apache/tajo/plan/verifier/SyntaxErrorUtil.java index 3c11b8ce6e..bf4a0d6bbd 100644 --- a/tajo-plan/src/main/java/org/apache/tajo/plan/verifier/SyntaxErrorUtil.java +++ b/tajo-plan/src/main/java/org/apache/tajo/plan/verifier/SyntaxErrorUtil.java @@ -21,6 +21,7 @@ import org.apache.tajo.catalog.Column; import org.apache.tajo.common.TajoDataTypes.DataType; import org.apache.tajo.error.Errors.ResultCode; +import org.apache.tajo.exception.DataTypeMismatchException; import org.apache.tajo.plan.logical.NodeType; public class SyntaxErrorUtil { @@ -29,8 +30,8 @@ public static SyntaxErrorException makeSyntaxError(String message) { return new SyntaxErrorException(ResultCode.SYNTAX_ERROR, message); } - public static SyntaxErrorException makeDataTypeMisMatch(Column src, Column target) { - return new SyntaxErrorException(ResultCode.DATATYPE_MISMATCH, + public static DataTypeMismatchException makeDataTypeMisMatch(Column src, Column target) { + return new DataTypeMismatchException( src.getSimpleName(), src.getDataType().getType().name(), target.getSimpleName(), target.getDataType().getType().name()); } From 2561c45cc0e87957193d014be53167dfd12491e4 Mon Sep 17 00:00:00 2001 From: Hyunsik Choi Date: Fri, 4 Sep 2015 14:58:45 +0900 Subject: [PATCH 3/5] Allow null type. --- .../org/apache/tajo/plan/verifier/LogicalPlanVerifier.java | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tajo-plan/src/main/java/org/apache/tajo/plan/verifier/LogicalPlanVerifier.java b/tajo-plan/src/main/java/org/apache/tajo/plan/verifier/LogicalPlanVerifier.java index 0d0274efc1..cae5430294 100644 --- a/tajo-plan/src/main/java/org/apache/tajo/plan/verifier/LogicalPlanVerifier.java +++ b/tajo-plan/src/main/java/org/apache/tajo/plan/verifier/LogicalPlanVerifier.java @@ -245,6 +245,11 @@ private static void ensureDomains(VerificationState state, Schema targetTableSch throws TajoException { for (int i = 0; i < schema.size(); i++) { + // null can be used anywhere + if (schema.getColumn(i).getDataType().getType() == Type.NULL_TYPE) { + continue; + } + // checking castable between two data types if (!TUtil.containsInNestedMap(CatalogUtil.OPERATION_CASTING_MAP, schema.getColumn(i).getDataType().getType(), targetTableScheme.getColumn(i).getDataType().getType())) { From 672eafde7b55f17b72f98a36a5770d580332fd31 Mon Sep 17 00:00:00 2001 From: Hyunsik Choi Date: Fri, 4 Sep 2015 15:16:16 +0900 Subject: [PATCH 4/5] Fixed wrong import. --- .../java/org/apache/tajo/plan/verifier/LogicalPlanVerifier.java | 1 - 1 file changed, 1 deletion(-) diff --git a/tajo-plan/src/main/java/org/apache/tajo/plan/verifier/LogicalPlanVerifier.java b/tajo-plan/src/main/java/org/apache/tajo/plan/verifier/LogicalPlanVerifier.java index cae5430294..c87ceb3379 100644 --- a/tajo-plan/src/main/java/org/apache/tajo/plan/verifier/LogicalPlanVerifier.java +++ b/tajo-plan/src/main/java/org/apache/tajo/plan/verifier/LogicalPlanVerifier.java @@ -19,7 +19,6 @@ package org.apache.tajo.plan.verifier; import com.google.common.base.Preconditions; -import com.sun.org.apache.xml.internal.resolver.Catalog; import org.apache.tajo.catalog.CatalogUtil; import org.apache.tajo.catalog.Column; import org.apache.tajo.catalog.Schema; From 4bbb0298cec02c6c46feaa284c180f0d03f300b6 Mon Sep 17 00:00:00 2001 From: Hyunsik Choi Date: Mon, 7 Sep 2015 22:00:31 +0900 Subject: [PATCH 5/5] Fixed casting map and unnecessary format changes. --- .../main/java/org/apache/tajo/catalog/CatalogUtil.java | 8 ++++++-- .../apache/tajo/engine/query/TestTablePartitions.java | 10 +++++----- 2 files changed, 11 insertions(+), 7 deletions(-) diff --git a/tajo-catalog/tajo-catalog-common/src/main/java/org/apache/tajo/catalog/CatalogUtil.java b/tajo-catalog/tajo-catalog-common/src/main/java/org/apache/tajo/catalog/CatalogUtil.java index ac76387394..2ae745f7e3 100644 --- a/tajo-catalog/tajo-catalog-common/src/main/java/org/apache/tajo/catalog/CatalogUtil.java +++ b/tajo-catalog/tajo-catalog-common/src/main/java/org/apache/tajo/catalog/CatalogUtil.java @@ -873,7 +873,10 @@ public static Pair, String> getPartitionKeyNamePair(Stri return pair; } - /* It is the relationship graph of type conversions. */ + /* + * It is the relationship graph of type conversions. Each map represents , < + * + */ public static final Map> OPERATION_CASTING_MAP = Maps.newHashMap(); static { @@ -928,9 +931,10 @@ public static Pair, String> getPartitionKeyNamePair(Stri TUtil.putToNestedMap(OPERATION_CASTING_MAP, Type.FLOAT8, Type.FLOAT8, Type.FLOAT8); TUtil.putToNestedMap(OPERATION_CASTING_MAP, Type.FLOAT8, Type.TEXT, Type.TEXT); + /* TUtil.putToNestedMap(OPERATION_CASTING_MAP, Type.TEXT, Type.TIMESTAMP, Type.TIMESTAMP); TUtil.putToNestedMap(OPERATION_CASTING_MAP, Type.TIMESTAMP, Type.TIMESTAMP, Type.TIMESTAMP); - TUtil.putToNestedMap(OPERATION_CASTING_MAP, Type.TIMESTAMP, Type.TEXT, Type.TEXT); + TUtil.putToNestedMap(OPERATION_CASTING_MAP, Type.TIMESTAMP, Type.TEXT, Type.TEXT);*/ TUtil.putToNestedMap(OPERATION_CASTING_MAP, Type.TEXT, Type.TEXT, Type.TEXT); TUtil.putToNestedMap(OPERATION_CASTING_MAP, Type.TIME, Type.TIME, Type.TIME); diff --git a/tajo-core-tests/src/test/java/org/apache/tajo/engine/query/TestTablePartitions.java b/tajo-core-tests/src/test/java/org/apache/tajo/engine/query/TestTablePartitions.java index 94e5e71602..fc0db1224b 100644 --- a/tajo-core-tests/src/test/java/org/apache/tajo/engine/query/TestTablePartitions.java +++ b/tajo-core-tests/src/test/java/org/apache/tajo/engine/query/TestTablePartitions.java @@ -208,7 +208,7 @@ public final void testCreateColumnPartitionedTableWithSelectedColumns() throws E TableDesc tableDesc = catalog.getTableDesc(DEFAULT_DATABASE_NAME, tableName); verifyPartitionDirectoryFromCatalog(DEFAULT_DATABASE_NAME, tableName, new String[]{"key"}, - tableDesc.getStats().getNumRows()); + tableDesc.getStats().getNumRows()); executeString("DROP TABLE " + tableName + " PURGE").close(); } @@ -251,7 +251,7 @@ public final void testColumnPartitionedTableByOneColumn() throws Exception { } verifyPartitionDirectoryFromCatalog(DEFAULT_DATABASE_NAME, tableName, - new String[]{"key"}, desc.getStats().getNumRows()); + new String[]{"key"}, desc.getStats().getNumRows()); executeString("DROP TABLE " + tableName + " PURGE").close(); res.close(); @@ -671,7 +671,7 @@ public final void testColumnPartitionedTableByOneColumnsWithCompression() throws } verifyPartitionDirectoryFromCatalog(DEFAULT_DATABASE_NAME, tableName, new String[]{"col1"}, - desc.getStats().getNumRows()); + desc.getStats().getNumRows()); executeString("DROP TABLE " + tableName + " PURGE").close(); } @@ -730,7 +730,7 @@ public final void testColumnPartitionedTableByTwoColumnsWithCompression() throws } verifyPartitionDirectoryFromCatalog(DEFAULT_DATABASE_NAME, tableName, new String[]{"col1", "col2"}, - desc.getStats().getNumRows()); + desc.getStats().getNumRows()); executeString("DROP TABLE " + tableName + " PURGE").close(); } @@ -1042,7 +1042,7 @@ public final void testColumnPartitionedTableWithSmallerExpressions5() throws Exc TableDesc desc = catalog.getTableDesc(DEFAULT_DATABASE_NAME, tableName); verifyPartitionDirectoryFromCatalog(DEFAULT_DATABASE_NAME, tableName, new String[]{"col2"}, - desc.getStats().getNumRows()); + desc.getStats().getNumRows()); executeString("DROP TABLE " + tableName + " PURGE").close(); }