KYLIN-2630 NPE when a subquery joins another lookup tables
Project: http://git-wip-us.apache.org/repos/asf/kylin/repo Commit: http://git-wip-us.apache.org/repos/asf/kylin/commit/ada37a91 Tree: http://git-wip-us.apache.org/repos/asf/kylin/tree/ada37a91 Diff: http://git-wip-us.apache.org/repos/asf/kylin/diff/ada37a91 Branch: refs/heads/KYLIN-2606 Commit: ada37a9109678abf347d3600b3a19888d6f64056 Parents: 34a65ba Author: Hongbin Ma <mahong...@apache.org> Authored: Thu May 18 20:54:45 2017 +0800 Committer: Hongbin Ma <mahong...@apache.org> Committed: Tue May 23 20:24:46 2017 +0800 ---------------------------------------------------------------------- .../resources/query/sql_subquery/query30.sql | 15 ++++++++++ .../resources/query/sql_subquery/query31.sql | 19 ++++++++++++ .../apache/kylin/query/relnode/OLAPJoinRel.java | 12 ++++++++ .../org/apache/kylin/query/relnode/OLAPRel.java | 31 +++++++++++--------- .../kylin/query/relnode/OLAPTableScan.java | 18 ++++++------ 5 files changed, 72 insertions(+), 23 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/kylin/blob/ada37a91/kylin-it/src/test/resources/query/sql_subquery/query30.sql ---------------------------------------------------------------------- diff --git a/kylin-it/src/test/resources/query/sql_subquery/query30.sql b/kylin-it/src/test/resources/query/sql_subquery/query30.sql new file mode 100644 index 0000000..3b0dfbf --- /dev/null +++ b/kylin-it/src/test/resources/query/sql_subquery/query30.sql @@ -0,0 +1,15 @@ + +SELECT t1.cal_dt, t1.sum_price,t1.lstg_site_id +FROM ( + select cal_dt, lstg_site_id, sum(price) as sum_price + from test_kylin_fact + group by cal_dt, lstg_site_id + +) t1 + +inner JOIN edw.test_cal_dt as test_cal_dt +on t1.cal_dt=test_cal_dt.cal_dt + +inner JOIN edw.test_sites as test_sites +on t1.lstg_site_id = test_sites.site_id + http://git-wip-us.apache.org/repos/asf/kylin/blob/ada37a91/kylin-it/src/test/resources/query/sql_subquery/query31.sql ---------------------------------------------------------------------- diff --git a/kylin-it/src/test/resources/query/sql_subquery/query31.sql b/kylin-it/src/test/resources/query/sql_subquery/query31.sql new file mode 100644 index 0000000..fafc01f --- /dev/null +++ b/kylin-it/src/test/resources/query/sql_subquery/query31.sql @@ -0,0 +1,19 @@ + +SELECT t1.week_beg_dt, t1.sum_price,t1.lstg_site_id +FROM ( + select test_cal_dt.week_beg_dt, sum(price) as sum_price, lstg_site_id + from test_kylin_fact + inner JOIN edw.test_cal_dt as test_cal_dt + ON test_kylin_fact.cal_dt = test_cal_dt.cal_dt + inner JOIN test_category_groupings + ON test_kylin_fact.leaf_categ_id = test_category_groupings.leaf_categ_id AND test_kylin_fact.lstg_site_id = test_category_groupings.site_id + inner JOIN edw.test_sites as test_sites + ON test_kylin_fact.lstg_site_id = test_sites.site_id + group by test_cal_dt.week_beg_dt, lstg_site_id +) t1 +inner JOIN edw.test_cal_dt as test_cal_dt +on t1.week_beg_dt=test_cal_dt.week_beg_dt + inner JOIN edw.test_sites as test_sites + + on t1.lstg_site_id = test_sites.site_id + http://git-wip-us.apache.org/repos/asf/kylin/blob/ada37a91/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 60b5712..1b5970c 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 @@ -140,12 +140,24 @@ public class OLAPJoinRel extends EnumerableJoin implements OLAPRel { implementor.freeContext(); } } + + if (leftHasSubquery) { + // After KYLIN-2579, leftHasSubquery means right side have to be separate olap context + implementor.setNewOLAPContextRequired(true); + } + implementor.fixSharedOlapTableScanOnTheRight(this); implementor.visitChild(this.right, this); if (this.context != implementor.getContext() || ((OLAPRel) this.right).hasSubQuery()) { this.hasSubQuery = true; rightHasSubquery = true; // if child is also an OLAPJoin, then the context has already been popped + + if (leftHasSubquery) { + Preconditions.checkState(!implementor.isNewOLAPContextRequired());//should have been satisfied + Preconditions.checkState(this.context != implementor.getContext(), "missing a new olapcontext"); + } + if (this.context != implementor.getContext()) { implementor.freeContext(); } http://git-wip-us.apache.org/repos/asf/kylin/blob/ada37a91/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 d9cfe02..814b0fd 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 @@ -77,6 +77,7 @@ public interface OLAPRel extends RelNode { private RelNode parentNode = null; 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; @@ -102,6 +103,16 @@ public interface OLAPRel extends RelNode { OLAPContext context = new OLAPContext(ctxSeq++); ctxStack.push(context); OLAPContext.registerContext(context); + setNewOLAPContextRequired(false); + } + + // set the flag to let OLAPImplementor allocate a new context more proactively + public void setNewOLAPContextRequired(boolean newOLAPContextRequired) { + this.newOLAPContextRequired = newOLAPContextRequired; + } + + public boolean isNewOLAPContextRequired() { + return this.newOLAPContextRequired; } public void fixSharedOlapTableScan(SingleRel parent) { @@ -115,19 +126,19 @@ public interface OLAPRel extends RelNode { if (copy != null) parent.replaceInput(0, copy); } - + public void fixSharedOlapTableScanOnTheRight(BiRel parent) { OLAPTableScan copy = copyTableScanIfNeeded(parent.getRight()); if (copy != null) parent.replaceInput(1, copy); } - + public void fixSharedOlapTableScanAt(RelNode parent, int ordinalInParent) { OLAPTableScan copy = copyTableScanIfNeeded(parent.getInputs().get(ordinalInParent)); if (copy != null) parent.replaceInput(ordinalInParent, copy); } - + private OLAPTableScan copyTableScanIfNeeded(RelNode input) { if (input instanceof OLAPTableScan) { OLAPTableScan tableScan = (OLAPTableScan) input; @@ -165,11 +176,11 @@ public interface OLAPRel extends RelNode { public static boolean needRewrite(OLAPContext ctx) { if (ctx.hasJoin) return true; - + String realRootFact = ctx.realization.getModel().getRootFactTable().getTableIdentity(); if (ctx.firstTableScan.getTableName().equals(realRootFact)) return true; - + return false; } } @@ -205,15 +216,7 @@ public interface OLAPRel extends RelNode { @Override public EnumerableRel.Result visitChild(EnumerableRel parent, int ordinal, EnumerableRel child, EnumerableRel.Prefer prefer) { - // OLAPTableScan is shared instance when the same table appears multiple times in the tree. - // Its context must be set (or corrected) right before visiting. - if (child instanceof OLAPTableScan) { - OLAPContext parentContext = relContexts.get(parent); - if (parentContext != null) { - ((OLAPTableScan) child).overrideContext(parentContext); - } - } - + if (calciteDebug) { OLAPContext context; if (child instanceof OLAPRel) http://git-wip-us.apache.org/repos/asf/kylin/blob/ada37a91/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 75c9c3e..a424818 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 @@ -180,7 +180,6 @@ public class OLAPTableScan extends TableScan implements OLAPRel, EnumerableRel { // distinct count will be split into a separated query that is joined with the left query planner.removeRule(AggregateExpandDistinctAggregatesRule.INSTANCE); - // see Dec 26th email @ http://mail-archives.apache.org/mod_mbox/calcite-dev/201412.mbox/browser planner.removeRule(ExpandConversionRule.INSTANCE); } @@ -208,11 +207,12 @@ public class OLAPTableScan extends TableScan implements OLAPRel, EnumerableRel { @Override public void implementOLAP(OLAPImplementor implementor) { Preconditions.checkState(columnRowType == null, "OLAPTableScan MUST NOT be shared by more than one prent"); - + // create context in case of non-join - if (implementor.getContext() == null || !(implementor.getParentNode() instanceof OLAPJoinRel)) { + if (implementor.getContext() == null || !(implementor.getParentNode() instanceof OLAPJoinRel) || implementor.isNewOLAPContextRequired()) { implementor.allocateContext(); } + columnRowType = buildColumnRowType(); context = implementor.getContext(); context.allTableScans.add(this); @@ -231,32 +231,32 @@ public class OLAPTableScan extends TableScan implements OLAPRel, EnumerableRel { public String getAlias() { return alias; } - + private ColumnRowType buildColumnRowType() { this.alias = Integer.toHexString(System.identityHashCode(this)); TableRef tableRef = TblColRef.tableForUnknownModel(this.alias, olapTable.getSourceTable()); - + List<TblColRef> columns = new ArrayList<TblColRef>(); for (ColumnDesc sourceColumn : olapTable.getExposedColumns()) { TblColRef colRef = TblColRef.columnForUnknownModel(tableRef, sourceColumn); columns.add(colRef); } - + if (columns.size() != rowType.getFieldCount()) { throw new IllegalStateException("RowType=" + rowType.getFieldCount() + ", ColumnRowType=" + columns.size()); } return new ColumnRowType(columns); } - + public TableRef getTableRef() { return columnRowType.getColumnByIndex(0).getTableRef(); } - + @SuppressWarnings("deprecation") public TblColRef makeRewriteColumn(String name) { return getTableRef().makeFakeColumn(name); } - + public void fixColumnRowTypeWithModel(DataModelDesc model, Map<String, String> aliasMap) { String newAlias = aliasMap.get(this.alias); for (TblColRef col : columnRowType.getAllColumns()) {
