[fix](lateral-view) fix bugs of lateral view with CTE or where clause (#7865)

fix bugs of lateral view with CTE or where clause.
The error case can be found in newly added tests in `TableFunctionPlanTest.java`
But there are still some bugs not being fixed, so the unit test is annotated with @Ignore

This PR contains the change is #7824 :

> Issue Number: close #7823
> 
> After the subquery is rewritten, the rewritten stmt needs to be reset
> (that is, the content of the first analyze semantic analysis is cleared),
> and then the rewritten stmt can be reAnalyzed.
> 
> The lateral view ref in the previous implementation forgot to implement the reset function.
> This caused him to keep the first error message in the second analyze.
> Eventually, two duplicate tupleIds appear in the new stmt and are marked with different tuple.
> From the explain string, the following syntax will have an additional wrong join predicate.
> ```
> Query: explain select k1 from test_explode lateral view explode_split(k2, ",") tmp as e1  where k1 in (select k3 from tbl1);
> Error equal join conjunct: `k3` = `k3`
> ```
> 
> This pr mainly adds the reset function of the lateral view
> to avoid possible errors in the second analyze
> when the lateral view and subquery rewrite occur at the same time.
This commit is contained in:
Mingyu Chen
2022-01-28 22:24:23 +08:00
committed by GitHub
parent 22830ea498
commit f93ac89a67
4 changed files with 147 additions and 17 deletions

View File

@ -85,6 +85,11 @@ public class LateralViewRef extends TableRef {
isAnalyzed = true; // true now that we have assigned desc
}
@Override
public TableRef clone() {
return new LateralViewRef(this.expr.clone(), this.viewName, this.columnName);
}
private void analyzeFunctionExpr(Analyzer analyzer) throws AnalysisException {
fnExpr = (FunctionCallExpr) expr;
fnExpr.setTableFnCall(true);
@ -178,5 +183,29 @@ public class LateralViewRef extends TableRef {
throw new AnalysisException("Subquery is not allowed in lateral view");
}
}
@Override
public String toSql() {
return "lateral view " + fnExpr.toSql() + " " + viewName + " as " + columnName;
}
@Override
public String toString() {
return toSql();
}
@Override
public void reset() {
isAnalyzed = false;
expr.reset();
fnExpr = null;
originSlotRefList = Lists.newArrayList();
view = null;
explodeSlotRef = null;
// There is no need to call the reset function of @relatedTableRef here.
// The main reason is that @lateralViewRef itself is an attribute of @relatedTableRef
// The reset of @lateralViewRef happens in the reset() of @relatedTableRef.
}
}

View File

@ -438,7 +438,7 @@ public class SelectStmt extends QueryStmt {
if (groupByClause != null && groupByClause.isGroupByExtension()) {
for (SelectListItem item : selectList.getItems()) {
if (item.getExpr() instanceof FunctionCallExpr && item.getExpr().fn instanceof AggregateFunction) {
for (Expr expr: groupByClause.getGroupingExprs()) {
for (Expr expr : groupByClause.getGroupingExprs()) {
if (item.getExpr().contains(expr)) {
throw new AnalysisException("column: " + expr.toSql() + " cannot both in select list and "
+ "aggregate functions when using GROUPING SETS/CUBE/ROLLUP, please use union"
@ -877,6 +877,12 @@ public class SelectStmt extends QueryStmt {
expandStar(new TableName(tableRef.getAliasAsName().getDb(),
tableRef.getAliasAsName().getTbl()),
tableRef.getDesc());
if (tableRef.lateralViewRefs != null) {
for (LateralViewRef lateralViewRef : tableRef.lateralViewRefs) {
expandStar(lateralViewRef.getName(), lateralViewRef.getDesc());
}
}
}
}
@ -1049,7 +1055,7 @@ public class SelectStmt extends QueryStmt {
groupByClause.analyze(analyzer);
createAggInfo(groupByClause.getGroupingExprs(), aggExprs, analyzer);
} else {
createAggInfo( new ArrayList<>(), aggExprs, analyzer);
createAggInfo(new ArrayList<>(), aggExprs, analyzer);
}
// combine avg smap with the one that produces the final agg output
@ -1874,3 +1880,4 @@ public class SelectStmt extends QueryStmt {
return this.id.equals(((SelectStmt) obj).id);
}
}

View File

@ -99,9 +99,9 @@ public class TableRef implements ParseNode, Writable {
private boolean isForcePreAggOpened;
// ///////////////////////////////////////
// BEGIN: Members that need to be reset()
protected Expr onClause;
// the ref to the left of us, if we're part of a JOIN clause
protected TableRef leftTblRef;
@ -133,7 +133,7 @@ public class TableRef implements ParseNode, Writable {
// END: Members that need to be reset()
// ///////////////////////////////////////
public TableRef() {
// for persist
}
@ -161,6 +161,7 @@ public class TableRef implements ParseNode, Writable {
this.commonHints = commonHints;
isAnalyzed = false;
}
// Only used to clone
// this will reset all the 'analyzed' stuff
protected TableRef(TableRef other) {
@ -187,7 +188,13 @@ public class TableRef implements ParseNode, Writable {
allMaterializedTupleIds_ = Lists.newArrayList(other.allMaterializedTupleIds_);
correlatedTupleIds_ = Lists.newArrayList(other.correlatedTupleIds_);
desc = other.desc;
lateralViewRefs = other.lateralViewRefs;
lateralViewRefs = null;
if (other.lateralViewRefs != null) {
lateralViewRefs = Lists.newArrayList();
for (LateralViewRef viewRef : other.lateralViewRefs) {
lateralViewRefs.add((LateralViewRef) viewRef.clone());
}
}
}
public PartitionNames getPartitionNames() {
@ -279,13 +286,17 @@ public class TableRef implements ParseNode, Writable {
* Returns true if this table ref has a resolved path that is rooted at a registered
* tuple descriptor, false otherwise.
*/
public boolean isRelative() { return false; }
public boolean isRelative() {
return false;
}
/**
* Indicates if this TableRef directly or indirectly references another TableRef from
* an outer query block.
*/
public boolean isCorrelated() { return !correlatedTupleIds_.isEmpty(); }
public boolean isCorrelated() {
return !correlatedTupleIds_.isEmpty();
}
public Table getTable() {
return desc.getTable();
@ -417,7 +428,7 @@ public class TableRef implements ParseNode, Writable {
* The join clause can only be analyzed after the left table has been analyzed
* and the TupleDescriptor (desc) of this table has been created.
*/
public void analyzeJoin(Analyzer analyzer) throws AnalysisException {
public void analyzeJoin(Analyzer analyzer) throws AnalysisException {
Preconditions.checkState(leftTblRef == null || leftTblRef.isAnalyzed);
Preconditions.checkState(desc != null);
analyzeJoinHints();
@ -540,7 +551,7 @@ public class TableRef implements ParseNode, Writable {
// of the outer join; those can be evaluated directly when materializing tuples
// without violating outer join semantics.
analyzer.registerOnClauseConjuncts(conjuncts, this);
for (Expr e: conjuncts) {
for (Expr e : conjuncts) {
List<TupleId> tupleIds = Lists.newArrayList();
e.getIds(tupleIds, null);
onClauseTupleIds.addAll(tupleIds);
@ -611,7 +622,16 @@ public class TableRef implements ParseNode, Writable {
// if (resolvedPath_ != null) path = resolvedPath_.getFullyQualifiedRawPath();
// return ToSqlUtils.getPathSql(path) + ((aliasSql != null) ? " " + aliasSql : "");
return name.toSql() + ((aliasSql != null) ? " " + aliasSql : "");
// tbl1
// tbl1 alias_tbl1
// tbl1 alias_tbl1 lateral view explode_split(k1, ",") tmp1 as e1
String tblName = name.toSql() + ((aliasSql != null) ? " " + aliasSql : "");
if (lateralViewRefs != null) {
for (LateralViewRef viewRef : lateralViewRefs) {
tblName += " " + viewRef.toSql();
}
}
return tblName;
}
@Override
@ -652,21 +672,27 @@ public class TableRef implements ParseNode, Writable {
/**
* Returns all legal aliases of this table ref.
*/
public String[] getAliases() { return aliases_; }
public String[] getAliases() {
return aliases_;
}
/**
* Returns the explicit alias or the fully-qualified implicit alias. The returned alias
* is guaranteed to be unique (i.e., column/field references against the alias cannot
* be ambiguous).
*/
public String getUniqueAlias() { return aliases_[0]; }
public String getUniqueAlias() {
return aliases_[0];
}
/**
* Returns true if this table ref has an explicit alias.
* Note that getAliases().length() == 1 does not imply an explicit alias because
* nested collection refs have only a single implicit alias.
*/
public boolean hasExplicitAlias() { return hasExplicitAlias_; }
public boolean hasExplicitAlias() {
return hasExplicitAlias_;
}
/**
* Returns the explicit alias if this table ref has one, null otherwise.
@ -676,7 +702,10 @@ public class TableRef implements ParseNode, Writable {
return null;
}
public boolean isAnalyzed() { return isAnalyzed; }
public boolean isAnalyzed() {
return isAnalyzed;
}
public boolean isResolved() {
return !getClass().equals(TableRef.class);
}
@ -722,6 +751,11 @@ public class TableRef implements ParseNode, Writable {
allMaterializedTupleIds_.clear();
correlatedTupleIds_.clear();
desc = null;
if (lateralViewRefs != null) {
for (LateralViewRef lateralViewRef : lateralViewRefs) {
lateralViewRef.reset();
}
}
}
/**
@ -755,7 +789,7 @@ public class TableRef implements ParseNode, Writable {
out.writeBoolean(true);
partitionNames.write(out);
}
if (hasExplicitAlias()) {
out.writeBoolean(true);
Text.writeString(out, getExplicitAlias());
@ -783,7 +817,8 @@ public class TableRef implements ParseNode, Writable {
if (in.readBoolean()) {
String alias = Text.readString(in);
aliases_ = new String[] { alias };
aliases_ = new String[]{alias};
}
}
}

View File

@ -19,6 +19,7 @@ package org.apache.doris.planner;
import org.apache.doris.analysis.CreateDbStmt;
import org.apache.doris.analysis.CreateTableStmt;
import org.apache.doris.analysis.CreateViewStmt;
import org.apache.doris.analysis.FunctionCallExpr;
import org.apache.doris.catalog.Catalog;
import org.apache.doris.qe.ConnectContext;
@ -28,6 +29,7 @@ import org.apache.commons.io.FileUtils;
import org.junit.After;
import org.junit.Assert;
import org.junit.BeforeClass;
import org.junit.Ignore;
import org.junit.Test;
import java.io.File;
@ -60,6 +62,11 @@ public class TableFunctionPlanTest {
+ "distributed by hash(k1) buckets 3 properties('replication_num' = '1');";
createTableStmt = (CreateTableStmt) UtFrameUtils.parseAndAnalyzeStmt(createTblStmtStr, ctx);
Catalog.getCurrentCatalog().createTable(createTableStmt);
createTblStmtStr = "create table db1.table_for_view (k1 int, k2 int, k3 varchar(100)) distributed by hash(k1)" +
"properties('replication_num' = '1');";
createTableStmt = (CreateTableStmt) UtFrameUtils.parseAndAnalyzeStmt(createTblStmtStr, ctx);
Catalog.getCurrentCatalog().createTable(createTableStmt);
}
// test planner
@ -441,6 +448,7 @@ public class TableFunctionPlanTest {
Assert.assertTrue(explainString.contains("table function: explode_json_array_double('[1.1, 2.2, 3.3]')"));
Assert.assertTrue(explainString.contains("output slot id: 0 1"));
}
/*
Case4 agg and order column in the same stmt with lateral view
select min(c1) from (select k1 as c1, min(k2) as c2 from tbl1 group by k1) tmp1
@ -470,4 +478,55 @@ public class TableFunctionPlanTest {
Assert.assertTrue(explainString.contains("output slot id: 2"));
Assert.assertTrue(explainString.contains("tuple ids: 1 3"));
}
@Test
public void testLaterViewWithView() throws Exception {
// test 1
String createViewStr = "create view db1.v1 (k1,e1) as select k1,e1 from db1.table_for_view lateral view explode_split(k3,',') tmp as e1;";
CreateViewStmt createViewStmt = (CreateViewStmt) UtFrameUtils.parseAndAnalyzeStmt(createViewStr, ctx);
Catalog.getCurrentCatalog().createView(createViewStmt);
String sql = "select * from db1.v1;";
String explainString = UtFrameUtils.getSQLPlanOrErrorMsg(ctx, sql, true);
Assert.assertTrue(explainString.contains("output slot id: 1 2"));
// query again to see if it has error
explainString = UtFrameUtils.getSQLPlanOrErrorMsg(ctx, sql, true);
Assert.assertTrue(explainString.contains("output slot id: 1 2"));
}
@Test
public void testLaterViewWithWhere() throws Exception {
String sql = "select k1,e1 from db1.table_for_view lateral view explode_split(k3,',') tmp as e1 where k1 in (select k2 from db1.table_for_view);";
String explainString = UtFrameUtils.getSQLPlanOrErrorMsg(ctx, sql, true);
Assert.assertTrue(explainString.contains("join op: LEFT SEMI JOIN (BROADCAST)"));
Assert.assertTrue(explainString.contains("equal join conjunct: `k1` = `k2`"));
Assert.assertTrue(!explainString.contains("equal join conjunct: `k2` = `k2`"));
}
@Test
public void testLaterViewWithCTE() throws Exception {
String sql = "with tmp as (select k1,e1 from db1.table_for_view lateral view explode_split(k3,',') tmp2 as e1) select * from tmp;";
String explainString = UtFrameUtils.getSQLPlanOrErrorMsg(ctx, sql, true);
Assert.assertTrue(explainString.contains("table function: explode_split(`default_cluster:db1`.`table_for_view`.`k3`, ',') "));
}
@Ignore
// errCode = 2, detailMessage = Unknown column 'e1' in 'table list'
public void testLaterViewWithCTEBug() throws Exception {
String sql = "with tmp as (select * from db1.table_for_view where k2=1) select k1,e1 from tmp lateral view explode_split(k3,',') tmp2 as e1;";
String explainString = UtFrameUtils.getSQLPlanOrErrorMsg(ctx, sql, true);
Assert.assertTrue(!explainString.contains("Unknown column 'e1' in 'table list'"));
}
@Ignore
// errCode = 2, detailMessage = Unknown column 'e1' in 'table list'
public void testLaterViewUnknowColumnBug() throws Exception {
// test2
String createViewStr = "create view db1.v2 (k1,k3) as select k1,k3 from db1.table_for_view;";
CreateViewStmt createViewStmt = (CreateViewStmt) UtFrameUtils.parseAndAnalyzeStmt(createViewStr, ctx);
Catalog.getCurrentCatalog().createView(createViewStmt);
String sql = "select k1,e1 from db1.v2 lateral view explode_split(k3,',') tmp as e1;";
String explainString = UtFrameUtils.getSQLPlanOrErrorMsg(ctx, sql, true);
Assert.assertTrue(!explainString.contains("Unknown column 'e1' in 'table list'"));
}
}