Support non-correlated subquery in having clause #3150

EmmyMiao87 · 2020-03-19T12:06:57Z

This commit support the non-correlated subquery in having clause.
For example:
select k1, sum(k2) from table group by k1 having sum(k2) > (select avg(k1) from table)

Also the non-scalar subquery is supported in Doris.
For example:
select k1, sum(k2) from table group by k1 having sum(k2) > (select avg(k1) from table group by k2)
Doris will check the result row numbers of subquery in executing.
If more then one row returned by subquery, the query will throw exception.

The implement method:
The entire outer query is regarded as inline view of new query.
The subquery in having clause is changed to the where predicate in this new query.

After this commit, tpc-ds 23,24,44 are supported.

This commit also support the subquery in ArithmeticExpr.
For example:
select k1 from table where k1=0.9*(select k1 from t);

EmmyMiao87 · 2020-03-19T12:11:49Z

#2848

yangzhg · 2020-03-20T08:58:24Z

fe/src/main/java/org/apache/doris/analysis/StmtRewriter.java

+        newTableRefList.add(inlineViewRef);
+        FromClause newFromClause = new FromClause(newTableRefList);
+        SelectStmt result = new SelectStmt(newSelectList, newFromClause, newWherePredicate, null, null, null,
+                LimitElement.NO_LIMIT);


what if the origin query is

select cs_item_sk, sum(cs_sales_price) from catalog_sales a group by cs_item_sk having sum(cs_sales_price) > (select min(cs_sales_price) from catalog_sales b where a.cs_item_sk = b.cs_item_sk) limit 1;

on limit after rewrite
what about order by ?

This commit support the non-correlated subquery in having clause. For example: select k1, sum(k2) from table group by k1 having sum(k2) > (select avg(k1) from table) Also the non-scalar subquery is supportted in Doris. For example: select k1, sum(k2) from table group by k1 having sum(k2) > (select avg(k1) from table group by k2) Doris will check the result row numbers of subquery in executing. If more then one row returned by subquery, the query will thrown exception. The implement method: The entire outer query is regarded as inline view of new query. The subquery in having clause is changed to the where predicate in this new query. After this commit, tpc-ds 23,24,44 are supported. This commit also support the subquery in ArithmeticExpr. For example: select k1 from table where k1=0.9*(select k1 from t);

fe/src/main/java/org/apache/doris/analysis/ArithmeticExpr.java

fe/src/main/java/org/apache/doris/analysis/BinaryPredicate.java

morningman · 2020-03-23T03:30:03Z

fe/src/main/java/org/apache/doris/analysis/StmtRewriter.java

            throws AnalysisException {
        if (parsedStmt instanceof QueryStmt) {
            QueryStmt analyzedStmt = (QueryStmt) parsedStmt;
            Preconditions.checkNotNull(analyzedStmt.analyzer);
-            rewriteQueryStatement(analyzedStmt, analyzer);
+            parsedStmt = rewriteQueryStatement(analyzedStmt, analyzer);


Better to use a new local variable and return it. Not to return a parameter variable.
And I think you can just return here.

return rewriteQueryStatement(analyzedStmt, analyzer);

morningman · 2020-03-23T03:31:01Z

fe/src/main/java/org/apache/doris/analysis/StmtRewriter.java

            throws AnalysisException {
        Preconditions.checkNotNull(stmt);
        if (stmt instanceof SelectStmt) {
-            rewriteSelectStatement((SelectStmt) stmt, analyzer);
+            stmt = rewriteSelectStatement((SelectStmt) stmt, analyzer);


Suggested change

stmt = rewriteSelectStatement((SelectStmt) stmt, analyzer);

return rewriteSelectStatement((SelectStmt) stmt, analyzer);

morningman · 2020-03-23T03:33:43Z

fe/src/main/java/org/apache/doris/analysis/StmtRewriter.java

        stmt.sqlString_ = null;
        if (LOG.isDebugEnabled()) LOG.debug("rewritten stmt: " + stmt.toSql());
+        return stmt;


Not returning the parameter, use local variable instead.

morningman · 2020-03-23T03:45:14Z

fe/src/main/java/org/apache/doris/analysis/StmtRewriter.java

+     * Inline view of subquery:
+     *     from (select b.cs_item_sk, min(cs_sales_price) from catalog_sales b group by cs_item_sk) c
+     * Rewritten correlated predicate:
+     *     where c.cs_item_sk = a.cs_item_sk and a.sum_cs_sales_price > c.min(cs_sales_price)


How about adding a final stmt? Is that

select cs_item_sk, a.sum_cs_sales_price from (select cs_item_sk, sum(cs_sales_price) sum_cs_sales_price from catalog_sales group by cs_item_sk) a join (select b.cs_item_sk, min(cs_sales_price) from catalog_sales b group by cs_item_sk) c where c.cs_item_sk = a.cs_item_sk and a.sum_cs_sales_price > c.min(cs_sales_price);

morningman · 2020-03-23T03:49:06Z

fe/src/main/java/org/apache/doris/analysis/StmtRewriter.java

+
+        // rewrite where subquery
+        result = rewriteSelectStatement(result, analyzer);
+        LOG.info("The final stmt is " + result.toSql());


Suggested change

LOG.info("The final stmt is " + result.toSql());

LOG.debug("The final stmt is " + result.toSql());

morningman · 2020-03-23T03:51:19Z

fe/src/test/java/org/apache/doris/analysis/StmtRewriterTest.java

+    }
+
+    @Test
+    public void testRewriteHavingClauseSubqueries() throws Exception {


Add this case to the queryPlanTest to reduce the running time of FE ut.
And how about adding more tests to cover this case? such as query with limit ?

And could you add the final rewritten stmt in comment:

select empid, x from (select empid, sum(salary) x from tbl group by empid) v where v.x > (select avg(salary) from tbl);

Co-Authored-By: Mingyu Chen <morningman.cmy@gmail.com>

…cubator-doris into subquery_in_having

wutiangan · 2020-03-24T15:19:38Z

fe/src/main/java/org/apache/doris/analysis/Expr.java

+    public boolean isCorrelatedPredicate(List<TupleId> tupleIdList) {
+        if ((this instanceof BinaryPredicate || this instanceof SlotRef) && !this.isBoundByTupleIds(tupleIdList)) {
+            return true;
+        }


why you add “this instanceof BinaryPredicate ”？
only slotref has override isBoundByTupleIds function.

wutiangan · 2020-03-24T15:31:33Z

fe/src/main/java/org/apache/doris/analysis/SelectStmt.java

@@ -95,13 +95,21 @@
    // if we have grouping extensions like cube or rollup or grouping sets
    private GroupingInfo groupingInfo;

+    // having clause which has been analyzed
+    // For example: select k1, sum(k2) a from t group by k1 having a>1;
+    // this parameter: having sum(t.k2) > 1


Suggested change

// this parameter: having sum(t.k2) > 1

// this parameter: sum(t.k2) > 1

wutiangan · 2020-03-24T15:37:44Z

fe/src/main/java/org/apache/doris/analysis/SelectStmt.java

+             * having predicate: table.k1 > 1
+             */
+            /*
+             * TODO(ml): support substitute outer column in subquery


Suggested change

* TODO(ml): support substitute outer column in subquery

* TODO(ml): support substitute outer column in correlated subquery

wutiangan · 2020-03-24T15:44:21Z

fe/src/main/java/org/apache/doris/analysis/SelectStmt.java

+        /*
+         * All of columns of result and having clause are replaced by new slot ref which is bound by top tuple of agg info.
+         * For example:
+         * ResultExprs: SlotRef(k1), FunctionCall(sum(SlotRef(k2)))


why do you add FunctionCall to decorate sum(SlotRef(k2))？And There is no function at below “After rewritten” 。

Em...The "after rewritten" is mean that after Expr.substitueList as following. This commit is used to analyze what happen with this line resultExprs = Expr.substituteList(resultExprs, combinedSmap, analyzer, false);

wutiangan · 2020-03-24T15:49:11Z

fe/src/main/java/org/apache/doris/analysis/SelectStmt.java

+        if (whereClause == null) {
+            return false;
+        }
+        return whereClause.isCorrelatedPredicate(getTableRefIds());


why do you only analyze where clause？ what other clasue？for example having clasue，selectList。

This function has been removed.

wutiangan · 2020-03-24T15:54:37Z

fe/src/main/java/org/apache/doris/analysis/StmtRewriter.java

            throws AnalysisException {
        Preconditions.checkNotNull(stmt);
        if (stmt instanceof SelectStmt) {
-            rewriteSelectStatement((SelectStmt) stmt, analyzer);
+            return rewriteSelectStatement((SelectStmt) stmt, analyzer);
        } else if (stmt instanceof SetOperationStmt) {
            rewriteUnionStatement((SetOperationStmt) stmt, analyzer);


what‘s the difference between 70 line and 74 line ？ why 70 line add return， and 74 line not return？

The rewriteUnionStatement is a function without return parameters.

wutiangan · 2020-03-24T16:06:34Z

fe/src/main/java/org/apache/doris/analysis/StmtRewriter.java

+        return result;
+    }
+
+    /**


you’d better add “corelated having subqueries are not supported” here。

wutiangan · 2020-03-24T16:07:42Z

fe/src/main/java/org/apache/doris/analysis/StmtRewriter.java

+         * For example:
+         * Query: select cs_item_sk, sum(cs_sales_price) from catalog_sales a group by cs_item_sk having ...;
+         * Inline view:
+         *     from (select cs_item_sk $ColumnA, sum(cs_sales_price) $ColumnB from catalog_sales a group by cs_item_sk) $TableA


Inline view does not need to write the ‘from’ keyword

I am afraid of missing understanding.

morningman

LGTM

EmmyMiao87 added the area/sql/compatibility Issues or PRs related to the SQL compatibililty label Mar 19, 2020

EmmyMiao87 added this to In progress in TPC-DS via automation Mar 19, 2020

EmmyMiao87 linked an issue Mar 19, 2020 that may be closed by this pull request

Support subquery in having clause #2848

Closed

yangzhg reviewed Mar 20, 2020

View reviewed changes

EmmyMiao87 added 4 commits March 20, 2020 17:39

Add assert number of rows element in binary predicates

f40c631

Move order by and limit into outer query

13c8799

Fix ut

1581b3d

EmmyMiao87 force-pushed the subquery_in_having branch from 7e24bec to 1581b3d Compare March 20, 2020 10:15

Fix ut

bf7c45a

morningman reviewed Mar 23, 2020

View reviewed changes

EmmyMiao87 and others added 4 commits March 23, 2020 18:56

Update fe/src/main/java/org/apache/doris/analysis/BinaryPredicate.java

665f69e

Co-Authored-By: Mingyu Chen <morningman.cmy@gmail.com>

Update fe/src/main/java/org/apache/doris/analysis/ArithmeticExpr.java

ea1c2b0

Co-Authored-By: Mingyu Chen <morningman.cmy@gmail.com>

Add commit of ut

be86912

Merge branch 'subquery_in_having' of https://github.com/EmmyMiao87/in…

745a1f6

…cubator-doris into subquery_in_having

EmmyMiao87 closed this Mar 24, 2020

TPC-DS automation moved this from In progress to Done Mar 24, 2020

EmmyMiao87 added 5 commits March 24, 2020 17:47

Substitute order by

866e075

Fix error

d069b5c

Fix error

469662c

Fix error

7efe375

Fix error

b31a9ee

EmmyMiao87 reopened this Mar 24, 2020

TPC-DS automation moved this from Done to In progress Mar 24, 2020

wutiangan reviewed Mar 24, 2020

View reviewed changes

Remove is correlated predicate function in query stmt

726c078

EmmyMiao87 closed this Mar 25, 2020

TPC-DS automation moved this from In progress to Done Mar 25, 2020

EmmyMiao87 reopened this Mar 25, 2020

TPC-DS automation moved this from Done to In progress Mar 25, 2020

morningman approved these changes Mar 25, 2020

View reviewed changes

morningman merged commit b2518fc into apache:master Mar 25, 2020

TPC-DS automation moved this from In progress to Done Mar 25, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support non-correlated subquery in having clause #3150

Support non-correlated subquery in having clause #3150

EmmyMiao87 commented Mar 19, 2020 •

edited

EmmyMiao87 commented Mar 19, 2020

yangzhg Mar 20, 2020

morningman Mar 23, 2020

morningman Mar 23, 2020

morningman Mar 23, 2020

morningman Mar 23, 2020

morningman Mar 23, 2020

morningman Mar 23, 2020

wutiangan Mar 24, 2020

wutiangan Mar 24, 2020

wutiangan Mar 24, 2020

wutiangan Mar 24, 2020

EmmyMiao87 Mar 25, 2020

wutiangan Mar 24, 2020

EmmyMiao87 Mar 25, 2020

wutiangan Mar 24, 2020

EmmyMiao87 Mar 25, 2020 •

edited

wutiangan Mar 24, 2020

wutiangan Mar 24, 2020

EmmyMiao87 Mar 25, 2020

morningman left a comment

	stmt = rewriteSelectStatement((SelectStmt) stmt, analyzer);
	return rewriteSelectStatement((SelectStmt) stmt, analyzer);

	LOG.info("The final stmt is " + result.toSql());
	LOG.debug("The final stmt is " + result.toSql());

	// this parameter: having sum(t.k2) > 1
	// this parameter: sum(t.k2) > 1

	* TODO(ml): support substitute outer column in subquery
	* TODO(ml): support substitute outer column in correlated subquery

Support non-correlated subquery in having clause #3150

Support non-correlated subquery in having clause #3150

Conversation

EmmyMiao87 commented Mar 19, 2020 • edited

EmmyMiao87 commented Mar 19, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

EmmyMiao87 Mar 25, 2020 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

morningman left a comment

Choose a reason for hiding this comment

EmmyMiao87 commented Mar 19, 2020 •

edited

EmmyMiao87 Mar 25, 2020 •

edited