Repository: tajo Updated Branches: refs/heads/branch-0.8.0 a18755686 -> 50d433f53
TAJO-718: A group-by clause with the same columns but aliased causes planning error. (hyunsik) Project: http://git-wip-us.apache.org/repos/asf/tajo/repo Commit: http://git-wip-us.apache.org/repos/asf/tajo/commit/50d433f5 Tree: http://git-wip-us.apache.org/repos/asf/tajo/tree/50d433f5 Diff: http://git-wip-us.apache.org/repos/asf/tajo/diff/50d433f5 Branch: refs/heads/branch-0.8.0 Commit: 50d433f53f1de94097e2012e32338cf4c81e33fd Parents: a187556 Author: Hyunsik Choi <[email protected]> Authored: Fri Mar 28 22:07:13 2014 +0900 Committer: Hyunsik Choi <[email protected]> Committed: Fri Mar 28 22:17:17 2014 +0900 ---------------------------------------------------------------------- CHANGES.txt | 9 +++- .../tajo/engine/planner/ExprNormalizer.java | 7 +-- .../tajo/engine/planner/LogicalPlanner.java | 14 ++++-- .../planner/rewrite/ProjectionPushDownRule.java | 51 ++++++++++++++++---- .../tajo/engine/query/TestCaseByCases.java | 7 +++ .../queries/TestCaseByCases/testTAJO718Case.sql | 10 ++++ .../TestCaseByCases/testTAJO718Case.result | 5 ++ 7 files changed, 82 insertions(+), 21 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/tajo/blob/50d433f5/CHANGES.txt ---------------------------------------------------------------------- diff --git a/CHANGES.txt b/CHANGES.txt index 866b1aa..50cc01a 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -289,8 +289,11 @@ Release 0.8.0 - unreleased BUG FIXES - TAJO-679: TimestampDatum, TimeDatum, DateDatum should be able to be compared - with NullDatum. (Alvin Henrick via jihoon) + TAJO-718: A group-by clause with the same columns but aliased causes + planning error. (hyunsik) + + TAJO-679: TimestampDatum, TimeDatum, DateDatum should be able to be + compared with NullDatum. (Alvin Henrick via jihoon) TAJO-716: Using column names actually aliased in aggregation functions can cause planning error. (hyunsik) @@ -302,6 +305,8 @@ Release 0.8.0 - unreleased TAJO-692: Missing Null handling for INET4 in RowStoreUtil. (jihoon) + TAJO-712: Fix some bugs after database is supported. (hyunsik) + TAJO-701: Invalid bytes when creating BlobDatum with offset. (jinho) TAJO-705: CTAS always stores tables with CSV storage type into catalog. http://git-wip-us.apache.org/repos/asf/tajo/blob/50d433f5/tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/ExprNormalizer.java ---------------------------------------------------------------------- diff --git a/tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/ExprNormalizer.java b/tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/ExprNormalizer.java index 9c732b4..9030629 100644 --- a/tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/ExprNormalizer.java +++ b/tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/ExprNormalizer.java @@ -245,12 +245,9 @@ class ExprNormalizer extends SimpleAlgebraVisitor<ExprNormalizer.ExprNormalizedR @Override public Expr visitColumnReference(ExprNormalizedResult ctx, Stack<Expr> stack, ColumnReferenceExpr expr) throws PlanningException { - // normalize column references. + // if a column reference is not qualified, it finds and sets the qualified column name. if (!(expr.hasQualifier() && CatalogUtil.isFQTableName(expr.getQualifier()))) { - if (ctx.block.namedExprsMgr.contains(expr.getCanonicalName())) { - NamedExpr namedExpr = ctx.block.namedExprsMgr.getNamedExpr(expr.getCanonicalName()); - return new ColumnReferenceExpr(namedExpr.getAlias()); - } else { + if (!ctx.block.namedExprsMgr.contains(expr.getCanonicalName())) { String normalized = ctx.plan.getNormalizedColumnName(ctx.block, expr); expr.setName(normalized); } http://git-wip-us.apache.org/repos/asf/tajo/blob/50d433f5/tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/LogicalPlanner.java ---------------------------------------------------------------------- diff --git a/tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/LogicalPlanner.java b/tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/LogicalPlanner.java index c479f1c..15fe6c0 100644 --- a/tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/LogicalPlanner.java +++ b/tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/LogicalPlanner.java @@ -367,10 +367,13 @@ public class LogicalPlanner extends BaseAlgebraVisitor<LogicalPlanner.PlanContex } } else if (projectable instanceof GroupbyNode) { GroupbyNode groupbyNode = (GroupbyNode) projectable; - for (Column c : groupbyNode.getGroupingColumns()) { - if (!projectable.getInSchema().contains(c)) { - throw new PlanningException(String.format("Cannot get the field \"%s\" at node (%d)", - c, projectable.getPID())); + // It checks if all column references within each target can be evaluated with the input schema. + int groupingColumnNum = groupbyNode.getGroupingColumns().length; + for (int i = 0; i < groupingColumnNum; i++) { + Set<Column> columns = EvalTreeUtil.findUniqueColumns(groupbyNode.getTargets()[i].getEvalTree()); + if (!projectable.getInSchema().containsAll(columns)) { + throw new PlanningException(String.format("Cannot get the field(s) \"%s\" at node (%d)", + TUtil.collectionToString(columns), projectable.getPID())); } } if (groupbyNode.hasAggFunctions()) { @@ -610,6 +613,7 @@ public class LogicalPlanner extends BaseAlgebraVisitor<LogicalPlanner.PlanContex Expr groupingKey = aggregation.getGroupSet()[0].getGroupingSets()[i]; normalizedResults[i] = normalizer.normalize(context, groupingKey); } + String [] groupingKeyRefNames = new String[groupingKeyNum]; for (int i = 0; i < groupingKeyNum; i++) { groupingKeyRefNames[i] = block.namedExprsMgr.addExpr(normalizedResults[i].baseExpr); @@ -674,7 +678,7 @@ public class LogicalPlanner extends BaseAlgebraVisitor<LogicalPlanner.PlanContex // Build grouping keys for (int i = 0; i < groupingKeyNum; i++) { - Target target = new Target(new FieldEval(groupingNode.getGroupingColumns()[i])); + Target target = block.namedExprsMgr.getTarget(groupingNode.getGroupingColumns()[i].getQualifiedName()); targets[i] = target; } http://git-wip-us.apache.org/repos/asf/tajo/blob/50d433f5/tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/rewrite/ProjectionPushDownRule.java ---------------------------------------------------------------------- diff --git a/tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/rewrite/ProjectionPushDownRule.java b/tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/rewrite/ProjectionPushDownRule.java index 4f3c6d4..668ed68 100644 --- a/tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/rewrite/ProjectionPushDownRule.java +++ b/tajo-core/tajo-core-backend/src/main/java/org/apache/tajo/engine/planner/rewrite/ProjectionPushDownRule.java @@ -486,6 +486,29 @@ public class ProjectionPushDownRule extends LogicalNode child = super.visitSort(newContext, plan, block, node, stack); + // it rewrite sortkeys. This rewrite sets right column names and eliminates duplicated sort keys. + List<SortSpec> sortSpecs = new ArrayList<SortSpec>(); + for (int i = 0; i < keyNames.length; i++) { + String sortKey = keyNames[i]; + Target target = context.targetListMgr.getTarget(sortKey); + if (context.targetListMgr.isEvaluated(sortKey)) { + Column c = target.getNamedColumn(); + SortSpec sortSpec = new SortSpec(c, node.getSortKeys()[i].isAscending(), node.getSortKeys()[i].isNullFirst()); + if (!sortSpecs.contains(sortSpec)) { + sortSpecs.add(sortSpec); + } + } else { + if (target.getEvalTree().getType() == EvalType.FIELD) { + Column c = ((FieldEval)target.getEvalTree()).getColumnRef(); + SortSpec sortSpec = new SortSpec(c, node.getSortKeys()[i].isAscending(), node.getSortKeys()[i].isNullFirst()); + if (!sortSpecs.contains(sortSpec)) { + sortSpecs.add(sortSpec); + } + } + } + } + node.setSortSpecs(sortSpecs.toArray(new SortSpec[sortSpecs.size()])); + node.setInSchema(child.getOutSchema()); node.setOutSchema(child.getOutSchema()); return node; @@ -550,31 +573,41 @@ public class ProjectionPushDownRule extends node.setInSchema(child.getOutSchema()); List<Target> targets = Lists.newArrayList(); - if (groupingKeyNum > 0) { + if (groupingKeyNum > 0 && groupingKeyNames != null) { // Restoring grouping key columns - final Column [] groupingColumns = new Column[groupingKeyNum]; + final List<Column> groupingColumns = new ArrayList<Column>(); for (int i = 0; i < groupingKeyNum; i++) { String groupingKey = groupingKeyNames[i]; Target target = context.targetListMgr.getTarget(groupingKey); + + // it rewrite grouping keys. + // This rewrite sets right column names and eliminates duplicated grouping keys. if (context.targetListMgr.isEvaluated(groupingKey)) { - groupingColumns[i] = target.getNamedColumn(); - targets.add(new Target(new FieldEval(target.getNamedColumn()))); + Column c = target.getNamedColumn(); + if (!groupingColumns.contains(c)) { + groupingColumns.add(c); + targets.add(new Target(new FieldEval(target.getNamedColumn()))); + } } else { if (target.getEvalTree().getType() == EvalType.FIELD) { - groupingColumns[i] = ((FieldEval)target.getEvalTree()).getColumnRef(); - targets.add(target); - context.targetListMgr.markAsEvaluated(target); + Column c = ((FieldEval)target.getEvalTree()).getColumnRef(); + if (!groupingColumns.contains(c)) { + groupingColumns.add(c); + targets.add(target); + context.targetListMgr.markAsEvaluated(target); + } } else { throw new PlanningException("Cannot evaluate this expression in grouping keys: " + target.getEvalTree()); } } } - node.setGroupingColumns(groupingColumns); + + node.setGroupingColumns(groupingColumns.toArray(new Column[groupingColumns.size()])); } // Getting projected targets - if (node.hasAggFunctions()) { + if (node.hasAggFunctions() && aggEvalNames != null) { AggregationFunctionCallEval [] aggEvals = new AggregationFunctionCallEval[aggEvalNames.length]; int i = 0; for (Iterator<String> it = getFilteredReferences(aggEvalNames, TUtil.newList(aggEvalNames)); it.hasNext();) { http://git-wip-us.apache.org/repos/asf/tajo/blob/50d433f5/tajo-core/tajo-core-backend/src/test/java/org/apache/tajo/engine/query/TestCaseByCases.java ---------------------------------------------------------------------- diff --git a/tajo-core/tajo-core-backend/src/test/java/org/apache/tajo/engine/query/TestCaseByCases.java b/tajo-core/tajo-core-backend/src/test/java/org/apache/tajo/engine/query/TestCaseByCases.java index 6472c68..2c42b2a 100644 --- a/tajo-core/tajo-core-backend/src/test/java/org/apache/tajo/engine/query/TestCaseByCases.java +++ b/tajo-core/tajo-core-backend/src/test/java/org/apache/tajo/engine/query/TestCaseByCases.java @@ -53,4 +53,11 @@ public class TestCaseByCases extends QueryTestCaseBase { assertResultSet(res); cleanupQuery(res); } + + @Test + public final void testTAJO718Case() throws Exception { + ResultSet res = executeQuery(); + assertResultSet(res); + cleanupQuery(res); + } } http://git-wip-us.apache.org/repos/asf/tajo/blob/50d433f5/tajo-core/tajo-core-backend/src/test/resources/queries/TestCaseByCases/testTAJO718Case.sql ---------------------------------------------------------------------- diff --git a/tajo-core/tajo-core-backend/src/test/resources/queries/TestCaseByCases/testTAJO718Case.sql b/tajo-core/tajo-core-backend/src/test/resources/queries/TestCaseByCases/testTAJO718Case.sql new file mode 100644 index 0000000..73e9f6a --- /dev/null +++ b/tajo-core/tajo-core-backend/src/test/resources/queries/TestCaseByCases/testTAJO718Case.sql @@ -0,0 +1,10 @@ +SELECT + "lineitem".l_orderkey AS l_orderkey, + "lineitem".l_orderkey AS l_orderkey1, + COUNT ("lineitem".l_orderkey) AS T57801e5322bc50 +FROM + "lineitem" +GROUP BY + l_orderkey, l_orderkey1 +ORDER BY + l_orderkey, l_orderkey1; \ No newline at end of file http://git-wip-us.apache.org/repos/asf/tajo/blob/50d433f5/tajo-core/tajo-core-backend/src/test/resources/results/TestCaseByCases/testTAJO718Case.result ---------------------------------------------------------------------- diff --git a/tajo-core/tajo-core-backend/src/test/resources/results/TestCaseByCases/testTAJO718Case.result b/tajo-core/tajo-core-backend/src/test/resources/results/TestCaseByCases/testTAJO718Case.result new file mode 100644 index 0000000..86c4d57 --- /dev/null +++ b/tajo-core/tajo-core-backend/src/test/resources/results/TestCaseByCases/testTAJO718Case.result @@ -0,0 +1,5 @@ +l_orderkey,l_orderkey1,t57801e5322bc50 +------------------------------- +1,1,2 +2,2,1 +3,3,2 \ No newline at end of file
