From 0ece03485848f798787921308618a78fe79dbffd Mon Sep 17 00:00:00 2001 From: Dongjoon Hyun Date: Mon, 20 Apr 2015 08:30:23 +0900 Subject: [PATCH] TAJO-1573: Fix addColumn bug (fieldsByName should maintain a list per a simple field name) --- .../java/org/apache/tajo/catalog/Schema.java | 6 ++++- .../org/apache/tajo/catalog/TestSchema.java | 27 ++++++++++++++----- .../org/apache/tajo/plan/LogicalPlanner.java | 10 +++---- .../tajo/plan/verifier/ExprsVerifier.java | 2 +- 4 files changed, 31 insertions(+), 14 deletions(-) diff --git a/tajo-catalog/tajo-catalog-common/src/main/java/org/apache/tajo/catalog/Schema.java b/tajo-catalog/tajo-catalog-common/src/main/java/org/apache/tajo/catalog/Schema.java index fcbd177392..9b2d1ad546 100644 --- a/tajo-catalog/tajo-catalog-common/src/main/java/org/apache/tajo/catalog/Schema.java +++ b/tajo-catalog/tajo-catalog-common/src/main/java/org/apache/tajo/catalog/Schema.java @@ -321,7 +321,11 @@ public synchronized Schema addColumn(String name, TypeDesc typeDesc) { Column newCol = new Column(normalized, typeDesc); fields.add(newCol); fieldsByQualifiedName.put(newCol.getQualifiedName(), fields.size() - 1); - fieldsByName.put(newCol.getSimpleName(), TUtil.newList(fields.size() - 1)); + if (fieldsByName.containsKey(newCol.getSimpleName())) { + fieldsByName.get(newCol.getSimpleName()).add(fields.size() - 1); + } else { + fieldsByName.put(newCol.getSimpleName(), TUtil.newList(fields.size() - 1)); + } return this; } diff --git a/tajo-catalog/tajo-catalog-common/src/test/java/org/apache/tajo/catalog/TestSchema.java b/tajo-catalog/tajo-catalog-common/src/test/java/org/apache/tajo/catalog/TestSchema.java index edd0f3e5b1..44854a9295 100644 --- a/tajo-catalog/tajo-catalog-common/src/test/java/org/apache/tajo/catalog/TestSchema.java +++ b/tajo-catalog/tajo-catalog-common/src/test/java/org/apache/tajo/catalog/TestSchema.java @@ -147,13 +147,26 @@ public final void testGetColumnString() { assertEquals(col3, schema.getColumn("addr")); } - @Test - public final void testAddField() { - Schema schema = new Schema(); - assertFalse(schema.containsByQualifiedName("studentId")); - schema.addColumn("studentId", Type.INT4); - assertTrue(schema.containsByQualifiedName("studentId")); - } + @Test + public final void testAddField() { + Schema schema = new Schema(); + + assertFalse(schema.contains("studentId")); + assertFalse(schema.containsByQualifiedName("studentId")); + schema.addColumn("default.t1.studentId", Type.INT4); + assertTrue(schema.contains("studentId")); + assertTrue(schema.containsByQualifiedName("default.t1.studentId")); + + assertFalse(schema.containsByQualifiedName("default.t2.studentId")); + schema.addColumn("default.t2.studentId", Type.INT4); + assertTrue(schema.containsByQualifiedName("default.t2.studentId")); + try { + schema.contains("studentId"); + fail(); + } catch (RuntimeException e) { + assertEquals(e.getMessage(), "Ambiguous Column name"); + } + } @Test public final void testEqualsObject() { diff --git a/tajo-plan/src/main/java/org/apache/tajo/plan/LogicalPlanner.java b/tajo-plan/src/main/java/org/apache/tajo/plan/LogicalPlanner.java index d1c1a150e5..0504bb2fd5 100644 --- a/tajo-plan/src/main/java/org/apache/tajo/plan/LogicalPlanner.java +++ b/tajo-plan/src/main/java/org/apache/tajo/plan/LogicalPlanner.java @@ -485,9 +485,9 @@ public static void verifyProjectedFields(QueryBlock block, Projectable projectab for (int i = 0; i < groupingKeyNum; i++) { Target target = groupbyNode.getTargets()[i]; - if (groupbyNode.getTargets()[i].getEvalTree().getType() == EvalType.FIELD) { + if (target.getEvalTree().getType() == EvalType.FIELD) { FieldEval grpKeyEvalNode = target.getEvalTree(); - if (!groupbyNode.getInSchema().contains(grpKeyEvalNode.getColumnRef())) { + if (!groupbyNode.getInSchema().containsByQualifiedName(grpKeyEvalNode.getColumnRef().getQualifiedName())) { throwCannotEvaluateException(projectable, grpKeyEvalNode.getName()); } } @@ -551,7 +551,7 @@ public static void verifyIfTargetsCanBeEvaluated(Schema baseSchema, Projectable for (Target target : projectable.getTargets()) { Set columns = EvalTreeUtil.findUniqueColumns(target.getEvalTree()); for (Column c : columns) { - if (!baseSchema.contains(c)) { + if (!baseSchema.containsByQualifiedName(c.getQualifiedName())) { throwCannotEvaluateException(projectable, c.getQualifiedName()); } } @@ -1200,8 +1200,8 @@ private static EvalNode getNaturalJoinCondition(JoinNode joinNode) { Column rightJoinKey; for (Column common : commons.getColumns()) { - leftJoinKey = leftSchema.getColumn(common.getQualifiedName()); - rightJoinKey = rightSchema.getColumn(common.getQualifiedName()); + leftJoinKey = leftSchema.getColumn(leftSchema.getColumnIdByName(common.getQualifiedName())); + rightJoinKey = rightSchema.getColumn(rightSchema.getColumnIdByName(common.getQualifiedName())); equiQual = new BinaryEval(EvalType.EQUAL, new FieldEval(leftJoinKey), new FieldEval(rightJoinKey)); if (njQual == null) { diff --git a/tajo-plan/src/main/java/org/apache/tajo/plan/verifier/ExprsVerifier.java b/tajo-plan/src/main/java/org/apache/tajo/plan/verifier/ExprsVerifier.java index bc6e69601a..3420d5d559 100644 --- a/tajo-plan/src/main/java/org/apache/tajo/plan/verifier/ExprsVerifier.java +++ b/tajo-plan/src/main/java/org/apache/tajo/plan/verifier/ExprsVerifier.java @@ -48,7 +48,7 @@ public static VerificationState verify(VerificationState state, LogicalNode curr instance.visitChild(state, expression, new Stack()); Set referredColumns = EvalTreeUtil.findUniqueColumns(expression); for (Column referredColumn : referredColumns) { - if (!currentNode.getInSchema().contains(referredColumn)) { + if (!currentNode.getInSchema().containsByQualifiedName(referredColumn.getQualifiedName())) { throw new PlanningException("Invalid State: " + referredColumn + " cannot be accessible at Node (" + currentNode.getPID() + ")"); }