Repository: tajo Updated Branches: refs/heads/master 5d470bc60 -> 2aaa991c9
TAJO-1900: When a record column and its child column are retrieved together, the record column might not be inferred as record type properly. Closes #792 Project: http://git-wip-us.apache.org/repos/asf/tajo/repo Commit: http://git-wip-us.apache.org/repos/asf/tajo/commit/2aaa991c Tree: http://git-wip-us.apache.org/repos/asf/tajo/tree/2aaa991c Diff: http://git-wip-us.apache.org/repos/asf/tajo/diff/2aaa991c Branch: refs/heads/master Commit: 2aaa991c9998b600bb9027d791e6f9de3bc3f8ab Parents: 5d470bc Author: Jihoon Son <[email protected]> Authored: Fri Oct 9 11:04:15 2015 +0900 Committer: Jihoon Son <[email protected]> Committed: Fri Oct 9 11:04:15 2015 +0900 ---------------------------------------------------------------------- CHANGES | 3 ++ .../engine/query/TestQueryOnSelfDescTable.java | 12 +++++ .../plan/rewrite/SelfDescSchemaBuildPhase.java | 50 ++++++++++++++------ 3 files changed, 51 insertions(+), 14 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/tajo/blob/2aaa991c/CHANGES ---------------------------------------------------------------------- diff --git a/CHANGES b/CHANGES index 716964b..8305a5d 100644 --- a/CHANGES +++ b/CHANGES @@ -336,6 +336,9 @@ Release 0.11.0 - unreleased BUG FIXES + TAJO-1900: When a record column and its child column are retrieved together, + the record column might not be inferred as record type properly. (jihoon) + TAJO-1875: Resource leak after a query failure. (jihoon) TAJO-1903: Insert clause occassionally fails on S3. (jinho) http://git-wip-us.apache.org/repos/asf/tajo/blob/2aaa991c/tajo-core-tests/src/test/java/org/apache/tajo/engine/query/TestQueryOnSelfDescTable.java ---------------------------------------------------------------------- diff --git a/tajo-core-tests/src/test/java/org/apache/tajo/engine/query/TestQueryOnSelfDescTable.java b/tajo-core-tests/src/test/java/org/apache/tajo/engine/query/TestQueryOnSelfDescTable.java index 3672697..7767317 100644 --- a/tajo-core-tests/src/test/java/org/apache/tajo/engine/query/TestQueryOnSelfDescTable.java +++ b/tajo-core-tests/src/test/java/org/apache/tajo/engine/query/TestQueryOnSelfDescTable.java @@ -20,6 +20,7 @@ package org.apache.tajo.engine.query; import org.apache.tajo.QueryTestCaseBase; import org.apache.tajo.exception.AmbiguousColumnException; +import org.apache.tajo.exception.NotImplementedException; import org.apache.tajo.exception.TajoException; import org.junit.After; import org.junit.Test; @@ -139,6 +140,17 @@ public class TestQueryOnSelfDescTable extends QueryTestCaseBase { runSimpleTests(); } + @Test(expected = NotImplementedException.class) + public final void testSelectRecordField() throws Exception { + executeString("select glossary.\"GlossDiv\" " + + "from self_desc_table2 where char_length(glossary.\"GlossDiv\".title) > 0 "); + } + + @Test(expected = NotImplementedException.class) + public final void testSelectRecordField2() throws Exception { + executeString("select glossary from self_desc_table2 where char_length(glossary.\"GlossDiv\".title) > 0 "); + } + @Test @Option(sort = true) @SimpleTest http://git-wip-us.apache.org/repos/asf/tajo/blob/2aaa991c/tajo-plan/src/main/java/org/apache/tajo/plan/rewrite/SelfDescSchemaBuildPhase.java ---------------------------------------------------------------------- diff --git a/tajo-plan/src/main/java/org/apache/tajo/plan/rewrite/SelfDescSchemaBuildPhase.java b/tajo-plan/src/main/java/org/apache/tajo/plan/rewrite/SelfDescSchemaBuildPhase.java index d697d22..bd61342 100644 --- a/tajo-plan/src/main/java/org/apache/tajo/plan/rewrite/SelfDescSchemaBuildPhase.java +++ b/tajo-plan/src/main/java/org/apache/tajo/plan/rewrite/SelfDescSchemaBuildPhase.java @@ -94,7 +94,7 @@ public class SelfDescSchemaBuildPhase extends LogicalPlanPreprocessPhase { public static <T extends Expr> Set<T> finds(Expr expr, OpType type) throws TajoException { FinderContext<T> context = new FinderContext<>(type); ExprFinderIncludeSubquery finder = new ExprFinderIncludeSubquery(); - finder.visit(context, new Stack<>(), expr); + finder.visit(context, new Stack<Expr>(), expr); return context.set; } @@ -138,7 +138,7 @@ public class SelfDescSchemaBuildPhase extends LogicalPlanPreprocessPhase { if (processor == null) { processor = new Processor(); } - return processor.visit(new ProcessorContext(context), new Stack<>(), expr); + return processor.visit(new ProcessorContext(context), new Stack<Expr>(), expr); } static class ProcessorContext { @@ -403,7 +403,17 @@ public class SelfDescSchemaBuildPhase extends LogicalPlanPreprocessPhase { Set<ColumnVertex> rootVertexes = new HashSet<>(); Schema schema = new Schema(); - for (Column eachColumn : columns) { + Set<Column> simpleColumns = new HashSet<>(); + List<Column> columnList = new ArrayList<>(columns); + Collections.sort(columnList, new Comparator<Column>() { + @Override + public int compare(Column c1, Column c2) { + return c2.getSimpleName().split(NestedPathUtil.PATH_DELIMITER).length - + c1.getSimpleName().split(NestedPathUtil.PATH_DELIMITER).length; + } + }); + + for (Column eachColumn : columnList) { String simpleName = eachColumn.getSimpleName(); if (NestedPathUtil.isPath(simpleName)) { String[] paths = simpleName.split(NestedPathUtil.PATH_DELIMITER); @@ -413,24 +423,26 @@ public class SelfDescSchemaBuildPhase extends LogicalPlanPreprocessPhase { parentName = CatalogUtil.buildFQName(eachColumn.getQualifier(), parentName); } // Leaf column type is TEXT; otherwise, RECORD. - Type childDataType = (i == paths.length-2) ? Type.TEXT : Type.RECORD; + ColumnVertex childVertex = new ColumnVertex( + paths[i+1], + StringUtils.join(paths, NestedPathUtil.PATH_DELIMITER, 0, i+2), + Type.RECORD + ); + if (i == paths.length - 2 && !schemaGraph.hasVertex(childVertex)) { + childVertex = new ColumnVertex(childVertex.name, childVertex.path, Type.TEXT); + } + ColumnVertex parentVertex = new ColumnVertex( parentName, StringUtils.join(paths, NestedPathUtil.PATH_DELIMITER, 0, i+1), Type.RECORD); - schemaGraph.addEdge( - new ColumnEdge( - new ColumnVertex( - paths[i+1], - StringUtils.join(paths, NestedPathUtil.PATH_DELIMITER, 0, i+2), childDataType - ), - parentVertex)); + schemaGraph.addEdge(new ColumnEdge(childVertex, parentVertex)); if (i == 0) { rootVertexes.add(parentVertex); } } } else { - schema.addColumn(eachColumn); + simpleColumns.add(eachColumn); } } @@ -441,6 +453,13 @@ public class SelfDescSchemaBuildPhase extends LogicalPlanPreprocessPhase { schema.addColumn(eachRoot.column); } + // Add simple columns + for (Column eachColumn : simpleColumns) { + if (!schema.contains(eachColumn)) { + schema.addColumn(eachColumn); + } + } + return schema; } @@ -461,7 +480,6 @@ public class SelfDescSchemaBuildPhase extends LogicalPlanPreprocessPhase { if (o instanceof ColumnVertex) { ColumnVertex other = (ColumnVertex) o; return this.name.equals(other.name) && - this.type.equals(other.type) && this.path.equals(other.path); } return false; @@ -469,7 +487,7 @@ public class SelfDescSchemaBuildPhase extends LogicalPlanPreprocessPhase { @Override public int hashCode() { - return Objects.hashCode(name, type, path); + return Objects.hashCode(name, path); } } @@ -487,6 +505,10 @@ public class SelfDescSchemaBuildPhase extends LogicalPlanPreprocessPhase { public void addEdge(ColumnEdge edge) { this.addEdge(edge.child, edge.parent, edge); } + + public boolean hasVertex(ColumnVertex vertex) { + return this.directedEdges.containsKey(vertex) || this.reversedEdges.containsKey(vertex); + } } private static class RecordColumnBuilder implements DirectedGraphVisitor<ColumnVertex> {
