Repository: kylin
Updated Branches:
  refs/heads/ranger 9864dc683 -> 108e93c60 (forced update)


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/ranger
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

Reply via email to