You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kylin.apache.org by li...@apache.org on 2016/06/07 13:44:41 UTC

[1/2] kylin git commit: KYLIN-1762 Query threw NPE with 3 or more join conditions

Repository: kylin
Updated Branches:
  refs/heads/master 0c9496859 -> 800fbd218


KYLIN-1762 Query threw NPE with 3 or more join conditions

Signed-off-by: Yang Li <li...@apache.org>


Project: http://git-wip-us.apache.org/repos/asf/kylin/repo
Commit: http://git-wip-us.apache.org/repos/asf/kylin/commit/4e52d2fc
Tree: http://git-wip-us.apache.org/repos/asf/kylin/tree/4e52d2fc
Diff: http://git-wip-us.apache.org/repos/asf/kylin/diff/4e52d2fc

Branch: refs/heads/master
Commit: 4e52d2fcb92aa337a47546d2fb28b1c5acbb79dd
Parents: 0c94968
Author: sunyerui <su...@gmail.com>
Authored: Sat Jun 4 23:04:48 2016 +0800
Committer: Yang Li <li...@apache.org>
Committed: Tue Jun 7 21:35:06 2016 +0800

----------------------------------------------------------------------
 .../src/test/resources/query/sql/query100.sql   | 28 ++++++++++++++++++++
 .../apache/kylin/query/relnode/OLAPJoinRel.java |  9 +++++--
 2 files changed, 35 insertions(+), 2 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/kylin/blob/4e52d2fc/kylin-it/src/test/resources/query/sql/query100.sql
----------------------------------------------------------------------
diff --git a/kylin-it/src/test/resources/query/sql/query100.sql b/kylin-it/src/test/resources/query/sql/query100.sql
new file mode 100644
index 0000000..1edf92d
--- /dev/null
+++ b/kylin-it/src/test/resources/query/sql/query100.sql
@@ -0,0 +1,28 @@
+--
+-- Licensed to the Apache Software Foundation (ASF) under one
+-- or more contributor license agreements.  See the NOTICE file
+-- distributed with this work for additional information
+-- regarding copyright ownership.  The ASF licenses this file
+-- to you under the Apache License, Version 2.0 (the
+-- "License"); you may not use this file except in compliance
+-- with the License.  You may obtain a copy of the License at
+--
+--     http://www.apache.org/licenses/LICENSE-2.0
+--
+-- Unless required by applicable law or agreed to in writing, software
+-- distributed under the License is distributed on an "AS IS" BASIS,
+-- WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+-- See the License for the specific language governing permissions and
+-- limitations under the License.
+--
+
+select t1.leaf_categ_id, max_price, min_price, sum_price
+from
+(select leaf_categ_id, sum(price) as sum_price from test_kylin_fact group by leaf_categ_id) t1
+join
+(select leaf_categ_id, max(price) as max_price from test_kylin_fact group by leaf_categ_id) t2
+on t1.leaf_categ_id = t2.leaf_categ_id
+join
+(select leaf_categ_id, min(price) as min_price from test_kylin_fact group by leaf_categ_id) t3
+on t1.leaf_categ_id = t3.leaf_categ_id
+order by t1.leaf_categ_id

http://git-wip-us.apache.org/repos/asf/kylin/blob/4e52d2fc/query/src/main/java/org/apache/kylin/query/relnode/OLAPJoinRel.java
----------------------------------------------------------------------
diff --git a/query/src/main/java/org/apache/kylin/query/relnode/OLAPJoinRel.java b/query/src/main/java/org/apache/kylin/query/relnode/OLAPJoinRel.java
index efe404b..4b293bc 100644
--- a/query/src/main/java/org/apache/kylin/query/relnode/OLAPJoinRel.java
+++ b/query/src/main/java/org/apache/kylin/query/relnode/OLAPJoinRel.java
@@ -121,12 +121,17 @@ public class OLAPJoinRel extends EnumerableJoin implements OLAPRel {
         implementor.visitChild(this.left, this);
         if (this.context != implementor.getContext() || ((OLAPRel) this.left).hasSubQuery()) {
             this.hasSubQuery = true;
-            implementor.freeContext();
+            // child join node didn't allocated a new context, and free context should be skipped
+            if (!(this.left instanceof OLAPJoinRel)) {
+                implementor.freeContext();
+            }
         }
         implementor.visitChild(this.right, this);
         if (this.context != implementor.getContext() || ((OLAPRel) this.right).hasSubQuery()) {
             this.hasSubQuery = true;
-            implementor.freeContext();
+            if (!(this.right instanceof OLAPJoinRel)) {
+                implementor.freeContext();
+            }
         }
 
         this.columnRowType = buildColumnRowType();


[2/2] kylin git commit: KYLIN-1762 code review

Posted by li...@apache.org.
KYLIN-1762 code review


Project: http://git-wip-us.apache.org/repos/asf/kylin/repo
Commit: http://git-wip-us.apache.org/repos/asf/kylin/commit/800fbd21
Tree: http://git-wip-us.apache.org/repos/asf/kylin/tree/800fbd21
Diff: http://git-wip-us.apache.org/repos/asf/kylin/diff/800fbd21

Branch: refs/heads/master
Commit: 800fbd2181a106890c05015f5137aad55637c15f
Parents: 4e52d2f
Author: Yang Li <li...@apache.org>
Authored: Tue Jun 7 21:44:33 2016 +0800
Committer: Yang Li <li...@apache.org>
Committed: Tue Jun 7 21:44:33 2016 +0800

----------------------------------------------------------------------
 .../src/test/resources/query/sql/query100.sql   | 28 --------------------
 .../resources/query/sql_subquery/query10.sql    | 28 ++++++++++++++++++++
 .../apache/kylin/query/relnode/OLAPJoinRel.java |  7 ++---
 3 files changed, 32 insertions(+), 31 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/kylin/blob/800fbd21/kylin-it/src/test/resources/query/sql/query100.sql
----------------------------------------------------------------------
diff --git a/kylin-it/src/test/resources/query/sql/query100.sql b/kylin-it/src/test/resources/query/sql/query100.sql
deleted file mode 100644
index 1edf92d..0000000
--- a/kylin-it/src/test/resources/query/sql/query100.sql
+++ /dev/null
@@ -1,28 +0,0 @@
---
--- Licensed to the Apache Software Foundation (ASF) under one
--- or more contributor license agreements.  See the NOTICE file
--- distributed with this work for additional information
--- regarding copyright ownership.  The ASF licenses this file
--- to you under the Apache License, Version 2.0 (the
--- "License"); you may not use this file except in compliance
--- with the License.  You may obtain a copy of the License at
---
---     http://www.apache.org/licenses/LICENSE-2.0
---
--- Unless required by applicable law or agreed to in writing, software
--- distributed under the License is distributed on an "AS IS" BASIS,
--- WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
--- See the License for the specific language governing permissions and
--- limitations under the License.
---
-
-select t1.leaf_categ_id, max_price, min_price, sum_price
-from
-(select leaf_categ_id, sum(price) as sum_price from test_kylin_fact group by leaf_categ_id) t1
-join
-(select leaf_categ_id, max(price) as max_price from test_kylin_fact group by leaf_categ_id) t2
-on t1.leaf_categ_id = t2.leaf_categ_id
-join
-(select leaf_categ_id, min(price) as min_price from test_kylin_fact group by leaf_categ_id) t3
-on t1.leaf_categ_id = t3.leaf_categ_id
-order by t1.leaf_categ_id

http://git-wip-us.apache.org/repos/asf/kylin/blob/800fbd21/kylin-it/src/test/resources/query/sql_subquery/query10.sql
----------------------------------------------------------------------
diff --git a/kylin-it/src/test/resources/query/sql_subquery/query10.sql b/kylin-it/src/test/resources/query/sql_subquery/query10.sql
new file mode 100644
index 0000000..1edf92d
--- /dev/null
+++ b/kylin-it/src/test/resources/query/sql_subquery/query10.sql
@@ -0,0 +1,28 @@
+--
+-- Licensed to the Apache Software Foundation (ASF) under one
+-- or more contributor license agreements.  See the NOTICE file
+-- distributed with this work for additional information
+-- regarding copyright ownership.  The ASF licenses this file
+-- to you under the Apache License, Version 2.0 (the
+-- "License"); you may not use this file except in compliance
+-- with the License.  You may obtain a copy of the License at
+--
+--     http://www.apache.org/licenses/LICENSE-2.0
+--
+-- Unless required by applicable law or agreed to in writing, software
+-- distributed under the License is distributed on an "AS IS" BASIS,
+-- WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+-- See the License for the specific language governing permissions and
+-- limitations under the License.
+--
+
+select t1.leaf_categ_id, max_price, min_price, sum_price
+from
+(select leaf_categ_id, sum(price) as sum_price from test_kylin_fact group by leaf_categ_id) t1
+join
+(select leaf_categ_id, max(price) as max_price from test_kylin_fact group by leaf_categ_id) t2
+on t1.leaf_categ_id = t2.leaf_categ_id
+join
+(select leaf_categ_id, min(price) as min_price from test_kylin_fact group by leaf_categ_id) t3
+on t1.leaf_categ_id = t3.leaf_categ_id
+order by t1.leaf_categ_id

http://git-wip-us.apache.org/repos/asf/kylin/blob/800fbd21/query/src/main/java/org/apache/kylin/query/relnode/OLAPJoinRel.java
----------------------------------------------------------------------
diff --git a/query/src/main/java/org/apache/kylin/query/relnode/OLAPJoinRel.java b/query/src/main/java/org/apache/kylin/query/relnode/OLAPJoinRel.java
index 4b293bc..dc16dad 100644
--- a/query/src/main/java/org/apache/kylin/query/relnode/OLAPJoinRel.java
+++ b/query/src/main/java/org/apache/kylin/query/relnode/OLAPJoinRel.java
@@ -121,15 +121,16 @@ public class OLAPJoinRel extends EnumerableJoin implements OLAPRel {
         implementor.visitChild(this.left, this);
         if (this.context != implementor.getContext() || ((OLAPRel) this.left).hasSubQuery()) {
             this.hasSubQuery = true;
-            // child join node didn't allocated a new context, and free context should be skipped
-            if (!(this.left instanceof OLAPJoinRel)) {
+            // if child is also an OLAPJoin, then the context has already been popped
+            if (this.context != implementor.getContext()) {
                 implementor.freeContext();
             }
         }
         implementor.visitChild(this.right, this);
         if (this.context != implementor.getContext() || ((OLAPRel) this.right).hasSubQuery()) {
             this.hasSubQuery = true;
-            if (!(this.right instanceof OLAPJoinRel)) {
+            // if child is also an OLAPJoin, then the context has already been popped
+            if (this.context != implementor.getContext()) {
                 implementor.freeContext();
             }
         }