Repository: kylin Updated Branches: refs/heads/master b3b7a4e7e -> d65376b58
KYLIN-2830 Fix null return values when union two subquery under join Project: http://git-wip-us.apache.org/repos/asf/kylin/repo Commit: http://git-wip-us.apache.org/repos/asf/kylin/commit/3f11a59b Tree: http://git-wip-us.apache.org/repos/asf/kylin/tree/3f11a59b Diff: http://git-wip-us.apache.org/repos/asf/kylin/diff/3f11a59b Branch: refs/heads/master Commit: 3f11a59bd6104231596a0db612c08630cfa96bb5 Parents: 34596d3 Author: Roger Shi <rogershijich...@hotmail.com> Authored: Thu Aug 31 20:22:58 2017 +0800 Committer: Roger Shi <rogershijich...@gmail.com> Committed: Sun Sep 3 18:10:12 2017 +0800 ---------------------------------------------------------------------- .../apache/kylin/query/ITKylinQueryTest.java | 5 +++ .../test/resources/query/sql_union/query01.sql | 13 ++++++++ .../test/resources/query/sql_union/query02.sql | 10 ++++++ .../resources/query/sql_verifyCount/query02.sql | 4 +++ .../query/sql_verifyCount/query02.sql.expected | 2 ++ .../org/apache/kylin/query/relnode/OLAPRel.java | 15 +++++++-- .../kylin/query/relnode/OLAPTableScan.java | 35 ++++++++++++++++++-- .../kylin/query/relnode/OLAPUnionRel.java | 32 +++++++++++++++--- 8 files changed, 106 insertions(+), 10 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/kylin/blob/3f11a59b/kylin-it/src/test/java/org/apache/kylin/query/ITKylinQueryTest.java ---------------------------------------------------------------------- diff --git a/kylin-it/src/test/java/org/apache/kylin/query/ITKylinQueryTest.java b/kylin-it/src/test/java/org/apache/kylin/query/ITKylinQueryTest.java index e332cda..2fb7771 100644 --- a/kylin-it/src/test/java/org/apache/kylin/query/ITKylinQueryTest.java +++ b/kylin-it/src/test/java/org/apache/kylin/query/ITKylinQueryTest.java @@ -239,6 +239,11 @@ public class ITKylinQueryTest extends KylinTestBase { } @Test + public void testUnionQuery() throws Exception { + execAndCompQuery(getQueryFolderPrefix() + "src/test/resources/query/sql_union", null, true); + } + + @Test public void testCachedQuery() throws Exception { execAndCompQuery(getQueryFolderPrefix() + "src/test/resources/query/sql_cache", null, true); } http://git-wip-us.apache.org/repos/asf/kylin/blob/3f11a59b/kylin-it/src/test/resources/query/sql_union/query01.sql ---------------------------------------------------------------------- diff --git a/kylin-it/src/test/resources/query/sql_union/query01.sql b/kylin-it/src/test/resources/query/sql_union/query01.sql new file mode 100644 index 0000000..ec9a029 --- /dev/null +++ b/kylin-it/src/test/resources/query/sql_union/query01.sql @@ -0,0 +1,13 @@ +-- union subquery under join +select sum(TEST_A.PRICE) as ITEM_CNT +FROM TEST_KYLIN_FACT as TEST_A +join ( + select sum(TEST_C.PRICE), TEST_C.TRANS_ID + from ( + select * from TEST_KYLIN_FACT where CAL_DT < DATE '2012-06-01' + union + select * from TEST_KYLIN_FACT where CAL_DT > DATE '2013-06-01' + ) TEST_C group by TRANS_ID +) TEST_B +on TEST_A.TRANS_ID = TEST_B.TRANS_ID +group by TEST_A.SELLER_ID http://git-wip-us.apache.org/repos/asf/kylin/blob/3f11a59b/kylin-it/src/test/resources/query/sql_union/query02.sql ---------------------------------------------------------------------- diff --git a/kylin-it/src/test/resources/query/sql_union/query02.sql b/kylin-it/src/test/resources/query/sql_union/query02.sql new file mode 100644 index 0000000..7ba023f --- /dev/null +++ b/kylin-it/src/test/resources/query/sql_union/query02.sql @@ -0,0 +1,10 @@ +-- union subquery under join +select sum(TEST_A.PRICE) as ITEM_CNT +FROM TEST_KYLIN_FACT as TEST_A +join ( + select * from TEST_KYLIN_FACT where CAL_DT < DATE '2012-06-01' + union + select * from TEST_KYLIN_FACT where CAL_DT > DATE '2013-06-01' +) TEST_B +on TEST_A.TRANS_ID = TEST_B.TRANS_ID +group by TEST_A.SELLER_ID http://git-wip-us.apache.org/repos/asf/kylin/blob/3f11a59b/kylin-it/src/test/resources/query/sql_verifyCount/query02.sql ---------------------------------------------------------------------- diff --git a/kylin-it/src/test/resources/query/sql_verifyCount/query02.sql b/kylin-it/src/test/resources/query/sql_verifyCount/query02.sql new file mode 100644 index 0000000..47ca6bf --- /dev/null +++ b/kylin-it/src/test/resources/query/sql_verifyCount/query02.sql @@ -0,0 +1,4 @@ +-- simple union +select * from TEST_KYLIN_FACT where CAL_DT < DATE '2012-06-01' +union +select * from TEST_KYLIN_FACT where CAL_DT > DATE '2013-07-01' \ No newline at end of file http://git-wip-us.apache.org/repos/asf/kylin/blob/3f11a59b/kylin-it/src/test/resources/query/sql_verifyCount/query02.sql.expected ---------------------------------------------------------------------- diff --git a/kylin-it/src/test/resources/query/sql_verifyCount/query02.sql.expected b/kylin-it/src/test/resources/query/sql_verifyCount/query02.sql.expected new file mode 100644 index 0000000..e530f14 --- /dev/null +++ b/kylin-it/src/test/resources/query/sql_verifyCount/query02.sql.expected @@ -0,0 +1,2 @@ +* +24 http://git-wip-us.apache.org/repos/asf/kylin/blob/3f11a59b/query/src/main/java/org/apache/kylin/query/relnode/OLAPRel.java ---------------------------------------------------------------------- diff --git a/query/src/main/java/org/apache/kylin/query/relnode/OLAPRel.java b/query/src/main/java/org/apache/kylin/query/relnode/OLAPRel.java index 814b0fd..68ee0da 100644 --- a/query/src/main/java/org/apache/kylin/query/relnode/OLAPRel.java +++ b/query/src/main/java/org/apache/kylin/query/relnode/OLAPRel.java @@ -74,18 +74,23 @@ public interface OLAPRel extends RelNode { */ public static class OLAPImplementor { - private RelNode parentNode = null; + private Stack<RelNode> parentNodeStack = new Stack<>(); private int ctxSeq = 0; private Stack<OLAPContext> ctxStack = new Stack<OLAPContext>(); private boolean newOLAPContextRequired = false; public void visitChild(RelNode input, RelNode parentNode) { - this.parentNode = parentNode; + this.parentNodeStack.push(parentNode); ((OLAPRel) input).implementOLAP(this); + this.parentNodeStack.pop(); } public RelNode getParentNode() { - return parentNode; + return parentNodeStack.peek(); + } + + public Stack<RelNode> getParentStack() { + return parentNodeStack; } public OLAPContext getContext() { @@ -177,6 +182,10 @@ public interface OLAPRel extends RelNode { if (ctx.hasJoin) return true; + if (ctx.realization == null) { + return false; + } + String realRootFact = ctx.realization.getModel().getRootFactTable().getTableIdentity(); if (ctx.firstTableScan.getTableName().equals(realRootFact)) return true; http://git-wip-us.apache.org/repos/asf/kylin/blob/3f11a59b/query/src/main/java/org/apache/kylin/query/relnode/OLAPTableScan.java ---------------------------------------------------------------------- diff --git a/query/src/main/java/org/apache/kylin/query/relnode/OLAPTableScan.java b/query/src/main/java/org/apache/kylin/query/relnode/OLAPTableScan.java index dd2160b..3a1ed2a 100644 --- a/query/src/main/java/org/apache/kylin/query/relnode/OLAPTableScan.java +++ b/query/src/main/java/org/apache/kylin/query/relnode/OLAPTableScan.java @@ -21,6 +21,7 @@ package org.apache.kylin.query.relnode; import java.util.ArrayList; import java.util.List; import java.util.Map; +import java.util.Stack; import org.apache.calcite.adapter.enumerable.EnumerableRel; import org.apache.calcite.adapter.enumerable.EnumerableRelImplementor; @@ -229,14 +230,44 @@ public class OLAPTableScan extends TableScan implements OLAPRel, EnumerableRel { context.firstTableScan = this; } - if (implementor.getParentNode() instanceof OLAPToEnumerableConverter) { + if (needCollectionColumns(implementor)) { // OLAPToEnumerableConverter on top of table scan, should be a select * from table for (TblColRef tblColRef : columnRowType.getAllColumns()) { - context.allColumns.add(tblColRef); + if (!tblColRef.getName().startsWith("_KY_")) { + context.allColumns.add(tblColRef); + } } } } + /** + * There're 3 special RelNode in parents stack, OLAPProjectRel, OLAPToEnumerableConverter + * and OLAPUnionRel. OLAPProjectRel will helps collect required columns but the other two + * don't. Go through the parent RelNodes from bottom to top, and the first-met special + * RelNode determines the behavior. + * * OLAPProjectRel -> skip column collection + * * OLAPToEnumerableConverter and OLAPUnionRel -> require column collection + */ + private boolean needCollectionColumns(OLAPImplementor implementor) { + Stack<RelNode> allParents = implementor.getParentStack(); + int index = allParents.size() - 1; + + while (index >= 0) { + RelNode parent = allParents.get(index); + if (parent instanceof OLAPProjectRel) { + return false; + } + + if (parent instanceof OLAPToEnumerableConverter || parent instanceof OLAPUnionRel) { + return true; + } + + index--; + } + + return true; + } + public String getAlias() { return alias; } http://git-wip-us.apache.org/repos/asf/kylin/blob/3f11a59b/query/src/main/java/org/apache/kylin/query/relnode/OLAPUnionRel.java ---------------------------------------------------------------------- diff --git a/query/src/main/java/org/apache/kylin/query/relnode/OLAPUnionRel.java b/query/src/main/java/org/apache/kylin/query/relnode/OLAPUnionRel.java index e04ba6f..dadec58 100644 --- a/query/src/main/java/org/apache/kylin/query/relnode/OLAPUnionRel.java +++ b/query/src/main/java/org/apache/kylin/query/relnode/OLAPUnionRel.java @@ -19,7 +19,9 @@ package org.apache.kylin.query.relnode; import java.util.ArrayList; +import java.util.HashSet; import java.util.List; +import java.util.Set; import org.apache.calcite.adapter.enumerable.EnumerableConvention; import org.apache.calcite.adapter.enumerable.EnumerableRel; @@ -34,6 +36,7 @@ import org.apache.calcite.rel.RelWriter; import org.apache.calcite.rel.core.SetOp; import org.apache.calcite.rel.core.Union; import org.apache.calcite.rel.metadata.RelMetadataQuery; +import org.apache.kylin.metadata.model.TblColRef; import com.google.common.base.Preconditions; @@ -71,20 +74,39 @@ public class OLAPUnionRel extends Union implements OLAPRel { @Override public void implementOLAP(OLAPImplementor implementor) { + // Always create new OlapContext to combine columns from all children contexts. + implementor.allocateContext(); + this.context = implementor.getContext(); for (int i = 0, n = getInputs().size(); i < n; i++) { implementor.fixSharedOlapTableScanAt(this, i); implementor.visitChild(getInputs().get(i), this); + if (implementor.getContext() != this.context) { + implementor.freeContext(); + } } this.columnRowType = buildColumnRowType(); - this.context = implementor.getContext(); } + /** + * Fake ColumnRowType for Union, all the columns are inner columns. + */ private ColumnRowType buildColumnRowType() { - // TODO just hack for now - OLAPRel olapChild = (OLAPRel) getInput(0); - ColumnRowType inputColumnRowType = olapChild.getColumnRowType(); - return inputColumnRowType; + ColumnRowType inputColumnRowType = ((OLAPRel) getInput(0)).getColumnRowType(); + List<TblColRef> columns = new ArrayList<>(); + List<Set<TblColRef>> sourceColumns = new ArrayList<>(); + + for (TblColRef tblColRef : inputColumnRowType.getAllColumns()) { + columns.add(TblColRef.newInnerColumn(tblColRef.getName(), TblColRef.InnerDataTypeEnum.LITERAL)); + } + + for (RelNode child : getInputs()) { + OLAPRel olapChild = (OLAPRel) child; + sourceColumns.add(new HashSet<>(olapChild.getColumnRowType().getAllColumns())); + } + + ColumnRowType fackColumnRowType = new ColumnRowType(columns, sourceColumns); + return fackColumnRowType; } @Override