This is an automated email from the ASF dual-hosted git repository. morningman pushed a commit to branch branch-1.2-lts in repository https://gitbox.apache.org/repos/asf/doris.git
commit cf97d0cd4b807b616dce069689402a96e3aa2aee Author: morrySnow <[email protected]> AuthorDate: Wed Mar 22 11:00:55 2023 +0800 [fix](planner) should not bind slot on brother's tuple in subquery (#17813) consider the query like this: ```sql SELECT k3, k4 FROM test WHERE EXISTS( SELECT d.* FROM (SELECT k1 AS _1234, SUM(k2) FROM `test` d GROUP BY _1234) d LEFT JOIN (SELECT k1 AS _1234, SUM(k2) FROM `test` GROUP BY _1234) temp ON d._1234 = temp._1234) ORDER BY k3, k4 ``` when we analyze group by exprs in `temp` inline view. we bind the `_1234` on `d._1234` by mistake. that because, when we do analyze in a **SUB-QUERY**, we will resolve SlotRef by itself **AND** parent's tuple. in the meanwhile, we register child's tuple to parent's analyzer. So, in a **SUB-QUERY**, the brother's tuple will affect the resolve result of current inlineview's slot. This PR: 1. add a flag on the function `resolveColumnRef` in `Analyzer` ```java private TupleDescriptor resolveColumnRef(String colName, boolean requestFromChild); private TupleDescriptor resolveColumnRef(TableName tblName, String colName, boolean requestByChild); ``` 2. add a flag to specify whether the tuple is from child. ```java // alias name -> <from child, tupleDesc> private final Multimap<String, Pair<Boolean, TupleDescriptor>> tupleByAlias; ``` when `requestByChild == true`, we **SKIP** the tuple from other child to avoid resolve error. --- .../java/org/apache/doris/analysis/Analyzer.java | 45 ++++++++++++++++------ .../org/apache/doris/analysis/InlineViewRef.java | 2 +- .../apache/doris/analysis/GroupByClauseTest.java | 5 ++- .../data/query_p0/subquery/test_subquery.out | 5 +++ .../suites/query_p0/subquery/test_subquery.groovy | 25 +++++++++++- 5 files changed, 67 insertions(+), 15 deletions(-) diff --git a/fe/fe-core/src/main/java/org/apache/doris/analysis/Analyzer.java b/fe/fe-core/src/main/java/org/apache/doris/analysis/Analyzer.java index 3165f5ba12..9b3265ee56 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/analysis/Analyzer.java +++ b/fe/fe-core/src/main/java/org/apache/doris/analysis/Analyzer.java @@ -111,7 +111,9 @@ public class Analyzer { // UniqueAlias used to check whether the table ref or the alias is unique // table/view used db.table, inline use alias private final Set<String> uniqueTableAliasSet = Sets.newHashSet(); - private final Multimap<String, TupleDescriptor> tupleByAlias = ArrayListMultimap.create(); + + // alias name -> <from child, tupleDesc> + private final Multimap<String, Pair<Boolean, TupleDescriptor>> tupleByAlias = ArrayListMultimap.create(); // NOTE: Alias of column is case ignorance // map from lowercase table alias to descriptor. @@ -605,10 +607,14 @@ public class Analyzer { if (globalState.conjuncts.get(id).substitute(sMap) instanceof BoolLiteral) { continue; } - globalState.conjuncts.put(id, (Predicate) globalState.conjuncts.get(id).substitute(sMap)); + globalState.conjuncts.put(id, globalState.conjuncts.get(id).substitute(sMap)); } } + public TupleDescriptor registerTableRef(TableRef ref) throws AnalysisException { + return registerTableRef(ref, false); + } + /** * Creates an returns an empty TupleDescriptor for the given table ref and registers * it against all its legal aliases. For tables refs with an explicit alias, only the @@ -619,7 +625,7 @@ public class Analyzer { * Throws if an existing explicit alias or implicit fully-qualified alias * has already been registered for another table ref. */ - public TupleDescriptor registerTableRef(TableRef ref) throws AnalysisException { + public TupleDescriptor registerTableRef(TableRef ref, boolean fromChild) throws AnalysisException { String uniqueAlias = ref.getUniqueAlias(); if (uniqueTableAliasSet.contains(uniqueAlias)) { ErrorReport.reportAnalysisException(ErrorCode.ERR_NONUNIQ_TABLE, uniqueAlias); @@ -652,7 +658,7 @@ public class Analyzer { for (String alias : aliases) { // TODO(zc) // aliasMap_.put(alias, result); - tupleByAlias.put(alias, result); + tupleByAlias.put(alias, Pair.of(fromChild, result)); } tableRefMap.put(result.getId(), ref); @@ -696,7 +702,7 @@ public class Analyzer { globalState.descTbl.computeStatAndMemLayout(); tableRefMap.put(result.getId(), ref); for (String alias : tableRef.getAliases()) { - tupleByAlias.put(alias, result); + tupleByAlias.put(alias, Pair.of(false, result)); } return result; } @@ -810,7 +816,7 @@ public class Analyzer { * @return null if not registered. */ public Collection<TupleDescriptor> getDescriptor(TableName name) { - return tupleByAlias.get(name.toString()); + return tupleByAlias.get(name.toString()).stream().map(p -> p.second).collect(Collectors.toList()); } public TupleDescriptor getTupleDesc(TupleId id) { @@ -877,9 +883,9 @@ public class Analyzer { if (d == null && hasAncestors() && isSubquery) { // analyzer father for subquery if (newTblName == null) { - d = getParentAnalyzer().resolveColumnRef(colName); + d = getParentAnalyzer().resolveColumnRef(colName, true); } else { - d = getParentAnalyzer().resolveColumnRef(newTblName, colName); + d = getParentAnalyzer().resolveColumnRef(newTblName, colName, true); } } if (d == null) { @@ -936,15 +942,24 @@ public class Analyzer { return result; } + TupleDescriptor resolveColumnRef(TableName tblName, String colName) throws AnalysisException { + return resolveColumnRef(tblName, colName, false); + } + /** * Resolves column name in context of any of the registered table aliases. * Returns null if not found or multiple bindings to different tables exist, * otherwise returns the table alias. */ - private TupleDescriptor resolveColumnRef(TableName tblName, String colName) throws AnalysisException { + private TupleDescriptor resolveColumnRef(TableName tblName, String colName, + boolean requestByChild) throws AnalysisException { TupleDescriptor result = null; // find table all name - for (TupleDescriptor desc : tupleByAlias.get(tblName.toString())) { + for (Pair<Boolean, TupleDescriptor> p : tupleByAlias.get(tblName.toString())) { + if (p.first && requestByChild) { + continue; + } + TupleDescriptor desc = p.second; //result = desc; if (!colName.equalsIgnoreCase(Column.DELETE_SIGN) && !isVisible(desc.getId())) { ErrorReport.reportAnalysisException(ErrorCode.ERR_ILLEGAL_COLUMN_REFERENCE_ERROR, @@ -963,8 +978,16 @@ public class Analyzer { } private TupleDescriptor resolveColumnRef(String colName) throws AnalysisException { + return resolveColumnRef(colName, false); + } + + private TupleDescriptor resolveColumnRef(String colName, boolean requestFromChild) throws AnalysisException { TupleDescriptor result = null; - for (TupleDescriptor desc : tupleByAlias.values()) { + for (Pair<Boolean, TupleDescriptor> p : tupleByAlias.values()) { + if (p.first && requestFromChild) { + continue; + } + TupleDescriptor desc = p.second; if (!isVisible(desc.getId())) { continue; } diff --git a/fe/fe-core/src/main/java/org/apache/doris/analysis/InlineViewRef.java b/fe/fe-core/src/main/java/org/apache/doris/analysis/InlineViewRef.java index cf7b459005..b16130c237 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/analysis/InlineViewRef.java +++ b/fe/fe-core/src/main/java/org/apache/doris/analysis/InlineViewRef.java @@ -200,7 +200,7 @@ public class InlineViewRef extends TableRef { } //TODO(chenhao16): fix TableName in Db.Table style // name.analyze(analyzer); - desc = analyzer.registerTableRef(this); + desc = analyzer.registerTableRef(this, true); isAnalyzed = true; // true now that we have assigned desc // For constant selects we materialize its exprs into a tuple. diff --git a/fe/fe-core/src/test/java/org/apache/doris/analysis/GroupByClauseTest.java b/fe/fe-core/src/test/java/org/apache/doris/analysis/GroupByClauseTest.java index 09208b307a..d294be5505 100644 --- a/fe/fe-core/src/test/java/org/apache/doris/analysis/GroupByClauseTest.java +++ b/fe/fe-core/src/test/java/org/apache/doris/analysis/GroupByClauseTest.java @@ -18,6 +18,7 @@ package org.apache.doris.analysis; import org.apache.doris.common.AnalysisException; +import org.apache.doris.common.Pair; import org.apache.doris.datasource.InternalCatalog; import com.google.common.collect.ArrayListMultimap; @@ -46,10 +47,10 @@ public class GroupByClauseTest { try { Field f = analyzer.getClass().getDeclaredField("tupleByAlias"); f.setAccessible(true); - Multimap<String, TupleDescriptor> tupleByAlias = ArrayListMultimap.create(); + Multimap<String, Pair<Boolean, TupleDescriptor>> tupleByAlias = ArrayListMultimap.create(); TupleDescriptor td = new TupleDescriptor(new TupleId(0)); td.setTable(analyzerBase.getTableOrAnalysisException(new TableName(internalCtl, "testdb", "t"))); - tupleByAlias.put("testdb.t", td); + tupleByAlias.put("testdb.t", Pair.of(false, td)); f.set(analyzer, tupleByAlias); } catch (NoSuchFieldException e) { e.printStackTrace(); diff --git a/regression-test/data/query_p0/subquery/test_subquery.out b/regression-test/data/query_p0/subquery/test_subquery.out index 0af57d6613..c88a878ff9 100644 --- a/regression-test/data/query_p0/subquery/test_subquery.out +++ b/regression-test/data/query_p0/subquery/test_subquery.out @@ -24,3 +24,8 @@ -- !sql8 -- +-- !sql_same_alias_in_subquery -- +1001 11011902 +1001 11011903 +1002 11011905 + diff --git a/regression-test/suites/query_p0/subquery/test_subquery.groovy b/regression-test/suites/query_p0/subquery/test_subquery.groovy index 2fdbe8dd54..c9d18d9b0a 100644 --- a/regression-test/suites/query_p0/subquery/test_subquery.groovy +++ b/regression-test/suites/query_p0/subquery/test_subquery.groovy @@ -50,5 +50,28 @@ suite("test_subquery") { select * from (select k1, -1 as c from test_query_db.test union all select k1, -2 as c from test_query_db.baseall) t where t.c > 0; """ -} + qt_sql_same_alias_in_subquery """ + SELECT + k3, k4 + FROM + test_query_db.test + WHERE + EXISTS( SELECT + d.* + FROM + (SELECT + k1 AS _1234, SUM(k2) + FROM + test_query_db.`test` d + GROUP BY _1234) d + LEFT JOIN + (SELECT + k1 AS _1234, + SUM(k2) + FROM + test_query_db.`test` + GROUP BY _1234) temp ON d._1234 = temp._1234) + ORDER BY k3, k4 + """ +} --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
