From d1e2138d23810f8c34e874bd492b1bb48101d497 Mon Sep 17 00:00:00 2001 From: HappenLee Date: Thu, 12 Nov 2020 19:16:24 +0800 Subject: [PATCH] [Bug] Fix bug #4886 and #4586 by refactoring code of method 'getDbs' --- .../org/apache/doris/analysis/InsertStmt.java | 4 +-- .../org/apache/doris/analysis/QueryStmt.java | 12 ++++++-- .../org/apache/doris/analysis/SelectStmt.java | 28 ++++++++++++------- .../doris/analysis/SetOperationStmt.java | 7 +++-- .../org/apache/doris/analysis/WithClause.java | 6 ++-- .../org/apache/doris/qe/StmtExecutor.java | 7 +++-- .../apache/doris/analysis/SelectStmtTest.java | 6 ++++ .../org/apache/doris/qe/StmtExecutorTest.java | 3 +- 8 files changed, 50 insertions(+), 23 deletions(-) diff --git a/fe/fe-core/src/main/java/org/apache/doris/analysis/InsertStmt.java b/fe/fe-core/src/main/java/org/apache/doris/analysis/InsertStmt.java index 8f28b0a246f479..c2845b828d7aa4 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/analysis/InsertStmt.java +++ b/fe/fe-core/src/main/java/org/apache/doris/analysis/InsertStmt.java @@ -180,9 +180,9 @@ public String getDb() { } // TODO(zc): used to get all dbs for lock - public void getDbs(Analyzer analyzer, Map dbs) throws AnalysisException { + public void getDbs(Analyzer analyzer, Map dbs, Set parentViewNameSet) throws AnalysisException { // get dbs of statement - queryStmt.getDbs(analyzer, dbs); + queryStmt.getDbs(analyzer, dbs, parentViewNameSet); // get db of target table tblName.analyze(analyzer); String dbName = tblName.getDb(); diff --git a/fe/fe-core/src/main/java/org/apache/doris/analysis/QueryStmt.java b/fe/fe-core/src/main/java/org/apache/doris/analysis/QueryStmt.java index ed70bb05c6d88d..73e53583f9eec8 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/analysis/QueryStmt.java +++ b/fe/fe-core/src/main/java/org/apache/doris/analysis/QueryStmt.java @@ -427,14 +427,20 @@ private Expr trySubstituteOrdinal(Expr expr, String errorPrefix, return resultExprs.get((int) pos - 1).clone(); } - public void getWithClauseDbs(Analyzer analyzer, Map dbs) throws AnalysisException { + public void getWithClauseDbs(Analyzer analyzer, Map dbs, Set parentViewNameSet) throws AnalysisException { if (withClause_ != null) { - withClause_.getDbs(analyzer, dbs); + withClause_.getDbs(analyzer, dbs, parentViewNameSet); } } // get database used by this query. - public abstract void getDbs(Analyzer analyzer, Map dbs) throws AnalysisException; + // Set parentViewNameSet contain parent stmt view name + // to make sure query like "with tmp as (select * from db1.table1) " + + // "select a.siteid, b.citycode, a.siteid from (select siteid, citycode from tmp) a " + + // "left join (select siteid, citycode from tmp) b on a.siteid = b.siteid;"; + // tmp in child stmt "(select siteid, citycode from tmp)" do not contain with_Clause + // so need to check is view name by parentViewNameSet. issue link: https://github.com/apache/incubator-doris/issues/4598 + public abstract void getDbs(Analyzer analyzer, Map dbs, Set parentViewNameSet) throws AnalysisException; /** * UnionStmt and SelectStmt have different implementations. diff --git a/fe/fe-core/src/main/java/org/apache/doris/analysis/SelectStmt.java b/fe/fe-core/src/main/java/org/apache/doris/analysis/SelectStmt.java index 13044741db820c..5560f7b77f00c4 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/analysis/SelectStmt.java +++ b/fe/fe-core/src/main/java/org/apache/doris/analysis/SelectStmt.java @@ -276,14 +276,13 @@ public ExprSubstitutionMap getBaseTblSmap() { } @Override - public void getDbs(Analyzer analyzer, Map dbs) throws AnalysisException { - getWithClauseDbs(analyzer, dbs); + public void getDbs(Analyzer analyzer, Map dbs, Set parentViewNameSet) throws AnalysisException { + getWithClauseDbs(analyzer, dbs, parentViewNameSet); for (TableRef tblRef : fromClause_) { if (tblRef instanceof InlineViewRef) { // Inline view reference QueryStmt inlineStmt = ((InlineViewRef) tblRef).getViewStmt(); - inlineStmt.withClause_ = this.withClause_; - inlineStmt.getDbs(analyzer, dbs); + inlineStmt.getDbs(analyzer, dbs, parentViewNameSet); } else { String dbName = tblRef.getName().getDb(); if (Strings.isNullOrEmpty(dbName)) { @@ -291,7 +290,7 @@ public void getDbs(Analyzer analyzer, Map dbs) throws Analysis } else { dbName = ClusterNamespace.getFullName(analyzer.getClusterName(), tblRef.getName().getDb()); } - if(withClause_ != null && isViewTableRef(tblRef)){ + if (isViewTableRef(tblRef.getName().toString(), parentViewNameSet)) { continue; } if (Strings.isNullOrEmpty(dbName)) { @@ -318,13 +317,22 @@ public void getDbs(Analyzer analyzer, Map dbs) throws Analysis } } - private boolean isViewTableRef(TableRef tblRef) { - List views = withClause_.getViews(); - for(View view : views){ - if(view.getName().equals(tblRef.getName().toString())){ - return true; + // if tableName in parentViewNameSetor tableName in withClause views + // means this tableref is inlineview, no need check dbname again + private boolean isViewTableRef(String tblName, Set parentViewNameSet) { + if (parentViewNameSet.contains(tblName)) { + return true; + } + + if (withClause_ != null) { + List views = withClause_.getViews(); + for (View view : views) { + if (view.getName().equals(tblName)) { + return true; + } } } + return false; } diff --git a/fe/fe-core/src/main/java/org/apache/doris/analysis/SetOperationStmt.java b/fe/fe-core/src/main/java/org/apache/doris/analysis/SetOperationStmt.java index 3a7ac703e5193e..583d647c8640da 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/analysis/SetOperationStmt.java +++ b/fe/fe-core/src/main/java/org/apache/doris/analysis/SetOperationStmt.java @@ -32,6 +32,7 @@ import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Set; /** * Representation of a set ops with its list of operands, and optional order by and limit. @@ -171,10 +172,10 @@ public void removeAllOperands() { public List getSetOpsResultExprs() { return setOpsResultExprs_; } @Override - public void getDbs(Analyzer analyzer, Map dbs) throws AnalysisException { - getWithClauseDbs(analyzer, dbs); + public void getDbs(Analyzer analyzer, Map dbs, Set parentViewNameSet) throws AnalysisException { + getWithClauseDbs(analyzer, dbs, parentViewNameSet); for (SetOperand op : operands) { - op.getQueryStmt().getDbs(analyzer, dbs); + op.getQueryStmt().getDbs(analyzer, dbs, parentViewNameSet); } } diff --git a/fe/fe-core/src/main/java/org/apache/doris/analysis/WithClause.java b/fe/fe-core/src/main/java/org/apache/doris/analysis/WithClause.java index 089a8bfcb2ea60..416490ce47513d 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/analysis/WithClause.java +++ b/fe/fe-core/src/main/java/org/apache/doris/analysis/WithClause.java @@ -20,6 +20,7 @@ import java.util.ArrayList; import java.util.List; import java.util.Map; +import java.util.Set; import org.apache.doris.catalog.Database; import org.apache.doris.catalog.View; @@ -105,10 +106,11 @@ public void reset() { for (View view: views_) view.getQueryStmt().reset(); } - public void getDbs(Analyzer analyzer, Map dbs) throws AnalysisException { + public void getDbs(Analyzer analyzer, Map dbs, Set parentViewNameSet) throws AnalysisException { for (View view : views_) { QueryStmt stmt = view.getQueryStmt(); - stmt.getDbs(analyzer, dbs); + parentViewNameSet.add(view.getName()); + stmt.getDbs(analyzer, dbs, parentViewNameSet); } } diff --git a/fe/fe-core/src/main/java/org/apache/doris/qe/StmtExecutor.java b/fe/fe-core/src/main/java/org/apache/doris/qe/StmtExecutor.java index 6cd135faa009ed..a2db0bfe4cd2e8 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/qe/StmtExecutor.java +++ b/fe/fe-core/src/main/java/org/apache/doris/qe/StmtExecutor.java @@ -90,12 +90,14 @@ import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; +import org.glassfish.jersey.internal.guava.Sets; import java.io.IOException; import java.io.StringReader; import java.nio.ByteBuffer; import java.util.List; import java.util.Map; +import java.util.Set; import java.util.UUID; import java.util.concurrent.atomic.AtomicLong; @@ -446,9 +448,10 @@ public void analyze(TQueryOptions tQueryOptions) throws UserException { || parsedStmt instanceof CreateTableAsSelectStmt) { Map dbs = Maps.newTreeMap(); QueryStmt queryStmt; + Set parentViewNameSet = Sets.newHashSet(); if (parsedStmt instanceof QueryStmt) { queryStmt = (QueryStmt) parsedStmt; - queryStmt.getDbs(analyzer, dbs); + queryStmt.getDbs(analyzer, dbs, parentViewNameSet); } else { InsertStmt insertStmt; if (parsedStmt instanceof InsertStmt) { @@ -456,7 +459,7 @@ public void analyze(TQueryOptions tQueryOptions) throws UserException { } else { insertStmt = ((CreateTableAsSelectStmt) parsedStmt).getInsertStmt(); } - insertStmt.getDbs(analyzer, dbs); + insertStmt.getDbs(analyzer, dbs, parentViewNameSet); } lock(dbs); diff --git a/fe/fe-core/src/test/java/org/apache/doris/analysis/SelectStmtTest.java b/fe/fe-core/src/test/java/org/apache/doris/analysis/SelectStmtTest.java index bb2e2880710b93..53cdde89226418 100755 --- a/fe/fe-core/src/test/java/org/apache/doris/analysis/SelectStmtTest.java +++ b/fe/fe-core/src/test/java/org/apache/doris/analysis/SelectStmtTest.java @@ -545,4 +545,10 @@ public void testWithWithoutDatabase() throws Exception { dorisAssert.withoutUseDatabase(); dorisAssert.query(sql).explainQuery(); } + + @Test + public void testWithInNestedQueryStmt() throws Exception { + String sql = "select 1 from (with w as (select 1 from db1.table1) select 1 from w) as tt"; + dorisAssert.query(sql).explainQuery(); + } } diff --git a/fe/fe-core/src/test/java/org/apache/doris/qe/StmtExecutorTest.java b/fe/fe-core/src/test/java/org/apache/doris/qe/StmtExecutorTest.java index c87bbf6cbc3aa7..26191d7bcc7ba9 100644 --- a/fe/fe-core/src/test/java/org/apache/doris/qe/StmtExecutorTest.java +++ b/fe/fe-core/src/test/java/org/apache/doris/qe/StmtExecutorTest.java @@ -44,6 +44,7 @@ import com.google.common.collect.Lists; +import org.glassfish.jersey.internal.guava.Sets; import org.junit.Assert; import org.junit.Before; import org.junit.BeforeClass; @@ -195,7 +196,7 @@ public void testSelect(@Mocked QueryStmt queryStmt, minTimes = 0; result = false; - queryStmt.getDbs((Analyzer) any, (SortedMap) any); + queryStmt.getDbs((Analyzer) any, (SortedMap) any, Sets.newHashSet()); minTimes = 0; queryStmt.getRedirectStatus();