From d5f77f41498903bc3ec0d921da9e17780009ae34 Mon Sep 17 00:00:00 2001 From: morrySnow Date: Wed, 22 Mar 2023 18:55:36 +0800 Subject: [PATCH] [fix](planner) forbid subquery register itself in parent's analyzer. in PR #17813 , we want to forbid bind slot on brother's column howerver the fix is not in correct way. the correct way to do that is forbid subquery register itself in parent's analyzer. This reverts commit b91a3b5a72520105638dad1079b71a05f02c10a0. --- .../org/apache/doris/analysis/Analyzer.java | 49 ++++++------------- .../apache/doris/analysis/InlineViewRef.java | 2 +- .../doris/analysis/GroupByClauseTest.java | 5 +- .../data/query_p0/subquery/test_subquery.out | 5 -- .../query_p0/subquery/test_subquery.groovy | 24 --------- 5 files changed, 17 insertions(+), 68 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 fd0d424d90aacb..425249ed011e0b 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 @@ -113,9 +113,7 @@ 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 uniqueTableAliasSet = Sets.newHashSet(); - - // alias name -> - private final Multimap> tupleByAlias = ArrayListMultimap.create(); + private final Multimap tupleByAlias = ArrayListMultimap.create(); // NOTE: Alias of column is case ignorance // map from lowercase table alias to descriptor. @@ -157,6 +155,7 @@ public class Analyzer { // Flag indicating if this analyzer instance belongs to a subquery. private boolean isSubquery = false; + private boolean isFirstScopeInSubquery = false; // Flag indicating if this analyzer instance belongs to an inlineview. private boolean isInlineView = false; @@ -181,6 +180,7 @@ public class Analyzer { public void setIsSubquery() { isSubquery = true; + isFirstScopeInSubquery = true; globalState.containsSubquery = true; } @@ -626,14 +626,10 @@ public void substitute(ExprSubstitutionMap sMap) { if (globalState.conjuncts.get(id).substitute(sMap) instanceof BoolLiteral) { continue; } - globalState.conjuncts.put(id, globalState.conjuncts.get(id).substitute(sMap)); + globalState.conjuncts.put(id, (Predicate) 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 @@ -644,7 +640,7 @@ public TupleDescriptor registerTableRef(TableRef ref) throws AnalysisException { * Throws if an existing explicit alias or implicit fully-qualified alias * has already been registered for another table ref. */ - public TupleDescriptor registerTableRef(TableRef ref, boolean fromChild) throws AnalysisException { + public TupleDescriptor registerTableRef(TableRef ref) throws AnalysisException { String uniqueAlias = ref.getUniqueAlias(); if (uniqueTableAliasSet.contains(uniqueAlias)) { ErrorReport.reportAnalysisException(ErrorCode.ERR_NONUNIQ_TABLE, uniqueAlias); @@ -677,7 +673,7 @@ public TupleDescriptor registerTableRef(TableRef ref, boolean fromChild) throws for (String alias : aliases) { // TODO(zc) // aliasMap_.put(alias, result); - tupleByAlias.put(alias, Pair.of(fromChild, result)); + tupleByAlias.put(alias, result); } tableRefMap.put(result.getId(), ref); @@ -721,7 +717,7 @@ public TupleDescriptor registerOlapTable(Table table, TableName tableName, List< globalState.descTbl.computeStatAndMemLayout(); tableRefMap.put(result.getId(), ref); for (String alias : tableRef.getAliases()) { - tupleByAlias.put(alias, Pair.of(false, result)); + tupleByAlias.put(alias, result); } return result; } @@ -835,7 +831,7 @@ public ExprRewriter getMVExprRewriter() { * @return null if not registered. */ public Collection getDescriptor(TableName name) { - return tupleByAlias.get(name.toString()).stream().map(p -> p.second).collect(Collectors.toList()); + return tupleByAlias.get(name.toString()); } public TupleDescriptor getTupleDesc(TupleId id) { @@ -899,12 +895,12 @@ public SlotDescriptor registerColumnRef(TableName tblName, String colName) throw * This column could not be resolved because doris can only resolved the parent column instead of grandpa. * The exception to this query like that: Unknown column 'k1' in 'a' */ - if (d == null && hasAncestors() && isSubquery) { + if (d == null && hasAncestors() && isSubquery && isFirstScopeInSubquery) { // analyzer father for subquery if (newTblName == null) { - d = getParentAnalyzer().resolveColumnRef(colName, true); + d = getParentAnalyzer().resolveColumnRef(colName); } else { - d = getParentAnalyzer().resolveColumnRef(newTblName, colName, true); + d = getParentAnalyzer().resolveColumnRef(newTblName, colName); } } if (d == null) { @@ -961,24 +957,15 @@ public SlotDescriptor registerVirtualColumnRef(String colName, Type type, TupleD 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, - boolean requestByChild) throws AnalysisException { + private TupleDescriptor resolveColumnRef(TableName tblName, String colName) throws AnalysisException { TupleDescriptor result = null; // find table all name - for (Pair p : tupleByAlias.get(tblName.toString())) { - if (p.first && requestByChild) { - continue; - } - TupleDescriptor desc = p.second; + for (TupleDescriptor desc : tupleByAlias.get(tblName.toString())) { //result = desc; if (!colName.equalsIgnoreCase(Column.DELETE_SIGN) && !isVisible(desc.getId())) { ErrorReport.reportAnalysisException(ErrorCode.ERR_ILLEGAL_COLUMN_REFERENCE_ERROR, @@ -997,16 +984,8 @@ private TupleDescriptor resolveColumnRef(TableName tblName, String colName, } private TupleDescriptor resolveColumnRef(String colName) throws AnalysisException { - return resolveColumnRef(colName, false); - } - - private TupleDescriptor resolveColumnRef(String colName, boolean requestFromChild) throws AnalysisException { TupleDescriptor result = null; - for (Pair p : tupleByAlias.values()) { - if (p.first && requestFromChild) { - continue; - } - TupleDescriptor desc = p.second; + for (TupleDescriptor desc : tupleByAlias.values()) { 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 b16130c237a532..cf7b459005b1b2 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 void analyze(Analyzer analyzer) throws AnalysisException, UserException { } //TODO(chenhao16): fix TableName in Db.Table style // name.analyze(analyzer); - desc = analyzer.registerTableRef(this, true); + desc = analyzer.registerTableRef(this); 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 d294be5505d7cb..09208b307a6ead 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,7 +18,6 @@ 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; @@ -47,10 +46,10 @@ public void setUp() throws AnalysisException { try { Field f = analyzer.getClass().getDeclaredField("tupleByAlias"); f.setAccessible(true); - Multimap> tupleByAlias = ArrayListMultimap.create(); + Multimap tupleByAlias = ArrayListMultimap.create(); TupleDescriptor td = new TupleDescriptor(new TupleId(0)); td.setTable(analyzerBase.getTableOrAnalysisException(new TableName(internalCtl, "testdb", "t"))); - tupleByAlias.put("testdb.t", Pair.of(false, td)); + tupleByAlias.put("testdb.t", 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 c03006700292c8..b42216c62d7a9f 100644 --- a/regression-test/data/query_p0/subquery/test_subquery.out +++ b/regression-test/data/query_p0/subquery/test_subquery.out @@ -36,8 +36,3 @@ true 9 1991 -2147483647 11011902 -654.654 true 1991-08-11 1989-03-21T13:11 wangj -- !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 d59f0aef760935..d22aada105cb54 100644 --- a/regression-test/suites/query_p0/subquery/test_subquery.groovy +++ b/regression-test/suites/query_p0/subquery/test_subquery.groovy @@ -54,28 +54,4 @@ 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 - """ } \ No newline at end of file