You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@doris.apache.org by GitBox <gi...@apache.org> on 2020/03/19 12:06:58 UTC

[GitHub] [incubator-doris] EmmyMiao87 opened a new pull request #3150: Support non-correlated subquery in having clause

EmmyMiao87 opened a new pull request #3150: Support non-correlated subquery in having clause
URL: https://github.com/apache/incubator-doris/pull/3150
 
 
   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);

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


[GitHub] [incubator-doris] morningman commented on a change in pull request #3150: Support non-correlated subquery in having clause

Posted by GitBox <gi...@apache.org>.
morningman commented on a change in pull request #3150: Support non-correlated subquery in having clause
URL: https://github.com/apache/incubator-doris/pull/3150#discussion_r396200869
 
 

 ##########
 File path: fe/src/main/java/org/apache/doris/analysis/StmtRewriter.java
 ##########
 @@ -97,8 +104,142 @@ private static void rewriteSelectStatement(SelectStmt stmt, Analyzer analyzer)
             }
             rewriteWhereClauseSubqueries(stmt, analyzer);
         }
+        // Rewrite all subquery in the having clause
+        if (stmt.getHavingPred() != null && stmt.getHavingPred().getSubquery() != null) {
+            stmt = rewriteHavingClauseSubqueries(stmt, analyzer);
+        }
         stmt.sqlString_ = null;
         if (LOG.isDebugEnabled()) LOG.debug("rewritten stmt: " + stmt.toSql());
+        return stmt;
+    }
+
+    /**
+     * Rewrite having subquery.
+     * Step1: rewrite having subquery to where subquery
+     * Step2: rewrite where subquery
+     * <p>
+     * For example:
+     * 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);
+     * <p>
+     * Step1: rewrite having subquery to where subquery
+     * Outer query is changed to inline view in rewritten query
+     * Inline view of outer query:
+     *     from (select cs_item_sk, sum(cs_sales_price) sum_cs_sales_price from catalog_sales group by cs_item_sk) a
+     * Rewritten subquery of expr:
+     *     where a.sum_cs_sales_price >
+     *           (select min(cs_sales_price) from catalog_sales b where a.cs_item_sk = b.cs_item_sk)
+     * Rewritten query:
+     *     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
+     *     where a.sum_cs_sales_price >
+     *           (select min(cs_sales_price) from catalog_sales b where a.cs_item_sk = b.cs_item_sk)
+     * <p>
+     * Step2: rewrite where subquery
+     * 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)
+     *
+     * @param stmt
+     * @param analyzer
+     */
+    private static SelectStmt rewriteHavingClauseSubqueries(SelectStmt stmt, Analyzer analyzer) throws AnalysisException {
+        // extract having predicate
+        Expr havingPredicate = stmt.getHavingPred();
+        Preconditions.checkState(havingPredicate != null);
+        Preconditions.checkState(havingPredicate.getSubquery() != null);
+        // extract limit
+        long limit = stmt.getLimit();
+        // extract order by element
+        ArrayList<OrderByElement> orderByElements = stmt.getOrderByElements();
+        // extract result of stmt
+        List<Expr> leftExprList = stmt.getResultExprs();
+        // extract table alias generator
+        TableAliasGenerator tableAliasGenerator = stmt.getTableAliasGenerator();
+
+        /*
+         * The outer query is changed to inline view without having predicate
+         * 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
+         */
+        SelectStmt inlineViewQuery = (SelectStmt) stmt.clone();
+        inlineViewQuery.reset();
+        // the having, order by and limit should be move to outer query
+        inlineViewQuery.removeHavingClause();
+        inlineViewQuery.removeOrderByElements();
+        inlineViewQuery.removeLimitElement();
+        // add a new alias for all of columns in subquery
+        List<String> colAliasOfInlineView = Lists.newArrayList();
+        for (int i = 0; i < inlineViewQuery.getSelectList().getItems().size(); ++i) {
+            colAliasOfInlineView.add(inlineViewQuery.getColumnAliasGenerator().getNextAlias());
+        }
+        InlineViewRef inlineViewRef = new InlineViewRef(tableAliasGenerator.getNextAlias(), inlineViewQuery,
+                colAliasOfInlineView);
+        try {
+            inlineViewRef.analyze(analyzer);
+        } catch (UserException e) {
+            throw new AnalysisException(e.getMessage());
+        }
+        LOG.debug("Outer query is changed to " + inlineViewRef.tableRefToSql());
+
+        /*
+         * Columns which belong to outer query can substitute for output columns of inline view
+         * For example:
+         * Having predicate: sum(cs_sales_price) >
+         *                   (select min(cs_sales_price) from catalog_sales b where a.cs_item_sk = b.cs_item_sk);
+         * Order by: sum(cs_sales_price), a.cs_item_sk
+         * Columns which belong to outer query: sum(cs_sales_price), a.cs_item_sk
+         * SMap: <cs_item_sk $ColumnA> <sum(cs_sales_price) $ColumnB>
+         * After substitute: $ColumnB >
+         *                   (select min(cs_sales_price) from catalog_sales b where $ColumnA = b.cs_item_sk)
+         * Order by: $ColumnB, $ColumnA
+         */
+        /*
+         * Prepare select list of new query.
+         * Generate a new select item for each original columns in select list
+         */
+        ExprSubstitutionMap smap = new ExprSubstitutionMap();
+        SelectList newSelectList = new SelectList();
+        for (int i = 0; i < inlineViewQuery.getSelectList().getItems().size(); i++) {
+            Expr leftExpr = leftExprList.get(i);
+            Expr rightExpr = new SlotRef(inlineViewRef.getAliasAsName(), colAliasOfInlineView.get(i));
+            rightExpr.analyze(analyzer);
+            smap.put(leftExpr, rightExpr);
+            // construct outer query select list
+            SelectListItem selectListItem = new SelectListItem(rightExpr, stmt.getColLabels().get(i));
+            newSelectList.addItem(selectListItem);
+        }
+        havingPredicate.reset();
+        Expr newWherePredicate = havingPredicate.substitute(smap, analyzer,false);
+        LOG.debug("Having predicate is changed to " + newWherePredicate.toSql());
+        ArrayList<OrderByElement> newOrderByElements = null;
+        if (orderByElements != null) {
+            newOrderByElements = OrderByElement.substitute(orderByElements, smap, analyzer);
+            LOG.debug("Order by is changed to " + Joiner.on(",").join(newOrderByElements));
+        }
+
+        // construct rewritten query
+        List<TableRef> newTableRefList = Lists.newArrayList();
+        newTableRefList.add(inlineViewRef);
+        FromClause newFromClause = new FromClause(newTableRefList);
+        SelectStmt result = new SelectStmt(newSelectList, newFromClause, newWherePredicate, null, null,
+                newOrderByElements, new LimitElement(limit));
+        result.setTableAliasGenerator(tableAliasGenerator);
+        try {
+            result.analyze(analyzer);
+        } catch (UserException e) {
+            throw new AnalysisException(e.getMessage());
+        }
+        LOG.info("New stmt {} is constructed after rewritten subquery of having clause.", result.toSql());
+
+        // rewrite where subquery
+        result = rewriteSelectStatement(result, analyzer);
+        LOG.info("The final stmt is " + result.toSql());
 
 Review comment:
   ```suggestion
           LOG.debug("The final stmt is " + result.toSql());
   ```

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


[GitHub] [incubator-doris] morningman commented on a change in pull request #3150: Support non-correlated subquery in having clause

Posted by GitBox <gi...@apache.org>.
morningman commented on a change in pull request #3150: Support non-correlated subquery in having clause
URL: https://github.com/apache/incubator-doris/pull/3150#discussion_r396195732
 
 

 ##########
 File path: fe/src/main/java/org/apache/doris/analysis/BinaryPredicate.java
 ##########
 @@ -313,14 +314,30 @@ public void analyzeImpl(Analyzer analyzer) throws AnalysisException {
         super.analyzeImpl(analyzer);
 
         for (Expr expr : children) {
-            if (expr instanceof Subquery && !((Subquery) expr).returnsScalarColumn()) {
-                String msg = "Subquery of binary predicate must return a single column: " + expr.toSql();
-                throw new AnalysisException(msg);
+            if (expr instanceof Subquery) {
+                Subquery subquery = (Subquery) expr;
+                if (!subquery.returnsScalarColumn()) {
+                    String msg = "Subquery of binary predicate must return a single column: " + expr.toSql();
+                    throw new AnalysisException(msg);
+                }
+                /**
+                 * Situation: The expr is a binary predicate and the type of subquery is not scalar type.
+                 * Add assert: The stmt of subquery is added an assert condition (return error if row count > 1).
+                 * Input params:
+                 *     expr: k1=(select k1 from t2)
+                 *     subquery stmt: select k1 from t2
+                 * Output params:
+                 *     new expr: k1 = (select k1 from t2 (assert row count: return error if row count > 1 ))
+                 *     subquery stmt: select k1 from t2 (assert row count: return error if row count > 1 )
+                 */
+                if (!subquery.getType().isScalarType()) {
+                    subquery.getStatement().setAssertNumRowsElement(1);
+                }
             }
         }
 
         // if children has subquery, it will be written and reanalyzed in the future.
 
 Review comment:
   ```suggestion
           // if children has subquery, it will be rewritten and reanalyzed in the future.
   ```

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


[GitHub] [incubator-doris] wutiangan commented on a change in pull request #3150: Support non-correlated subquery in having clause

Posted by GitBox <gi...@apache.org>.
wutiangan commented on a change in pull request #3150: Support non-correlated subquery in having clause
URL: https://github.com/apache/incubator-doris/pull/3150#discussion_r397250060
 
 

 ##########
 File path: fe/src/main/java/org/apache/doris/analysis/SelectStmt.java
 ##########
 @@ -839,19 +861,32 @@ private Expr rewriteCountDistinctForBitmapOrHLL(Expr expr, Analyzer analyzer) {
     private void analyzeAggregation(Analyzer analyzer) throws AnalysisException {
         // check having clause
         if (havingClause != null) {
-            if (havingClause.contains(Predicates.instanceOf(Subquery.class))) {
-                throw new AnalysisException(
-                        "Subqueries are not supported in the HAVING clause.");
-            }
             Expr ambiguousAlias = getFirstAmbiguousAlias(havingClause);
             if (ambiguousAlias != null) {
                 ErrorReport.reportAnalysisException(ErrorCode.ERR_NON_UNIQ_ERROR, ambiguousAlias.toColumnLabel());
             }
-            // substitute aliases in place (ordinals not allowed in having clause)
-            havingPred = havingClause.substitute(aliasSMap, analyzer, false);
-            havingPred.checkReturnsBool("HAVING clause", true);
+            /*
+             * The having clause need to be substitute by aliasSMap.
+             * And it is analyzed after substitute.
+             * For example:
+             * Query: select k1 a, sum(k2) b from table group by k1 having a > 1;
+             * Having clause: a > 1
+             * aliasSMap: <a, table.k1> <b, sum(table.k2)>
+             * After substitute: a > 1 changed to table.k1 > 1
+             * Analyzer: check column and other subquery in having clause
+             * having predicate: table.k1 > 1
+             */
+            /*
+             * TODO(ml): support substitute outer column in subquery
 
 Review comment:
   ```suggestion
                * TODO(ml): support substitute outer column in correlated subquery
   ```

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


[GitHub] [incubator-doris] morningman merged pull request #3150: Support non-correlated subquery in having clause

Posted by GitBox <gi...@apache.org>.
morningman merged pull request #3150: Support non-correlated subquery in having clause
URL: https://github.com/apache/incubator-doris/pull/3150
 
 
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


[GitHub] [incubator-doris] wutiangan commented on a change in pull request #3150: Support non-correlated subquery in having clause

Posted by GitBox <gi...@apache.org>.
wutiangan commented on a change in pull request #3150: Support non-correlated subquery in having clause
URL: https://github.com/apache/incubator-doris/pull/3150#discussion_r397263620
 
 

 ##########
 File path: fe/src/main/java/org/apache/doris/analysis/StmtRewriter.java
 ##########
 @@ -43,62 +44,260 @@
      * Rewrite the statement of an analysis result. The unanalyzed rewritten
      * statement is returned.
      */
-    public static void rewrite(Analyzer analyzer, StatementBase parsedStmt)
+    public static StatementBase rewrite(Analyzer analyzer, StatementBase parsedStmt)
             throws AnalysisException {
         if (parsedStmt instanceof QueryStmt) {
             QueryStmt analyzedStmt = (QueryStmt) parsedStmt;
             Preconditions.checkNotNull(analyzedStmt.analyzer);
-            rewriteQueryStatement(analyzedStmt, analyzer);
+            return rewriteQueryStatement(analyzedStmt, analyzer);
         } else if (parsedStmt instanceof InsertStmt) {
             final InsertStmt insertStmt = (InsertStmt)parsedStmt;
             final QueryStmt analyzedStmt = (QueryStmt)insertStmt.getQueryStmt();
             Preconditions.checkNotNull(analyzedStmt.analyzer);
-            rewriteQueryStatement(analyzedStmt, analyzer);
+            QueryStmt rewrittenQueryStmt = rewriteQueryStatement(analyzedStmt, analyzer);
+            insertStmt.setQueryStmt(rewrittenQueryStmt);
         } else {
             throw new AnalysisException("Unsupported statement containing subqueries: "
                     + parsedStmt.toSql());
         }
+        return parsedStmt;
     }
 
   /**
    *  Calls the appropriate rewrite method based on the specific type of query stmt. See
    *  rewriteSelectStatement() and rewriteUnionStatement() documentation.
    */
-    public static void rewriteQueryStatement(QueryStmt stmt, Analyzer analyzer)
+    public static QueryStmt rewriteQueryStatement(QueryStmt stmt, Analyzer analyzer)
             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);
 
 Review comment:
   what‘s the difference between 70 line and 74 line ? why 70 line add return, and 74 line not return?

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


[GitHub] [incubator-doris] EmmyMiao87 commented on a change in pull request #3150: Support non-correlated subquery in having clause

Posted by GitBox <gi...@apache.org>.
EmmyMiao87 commented on a change in pull request #3150: Support non-correlated subquery in having clause
URL: https://github.com/apache/incubator-doris/pull/3150#discussion_r397575808
 
 

 ##########
 File path: fe/src/main/java/org/apache/doris/analysis/SelectStmt.java
 ##########
 @@ -974,16 +1009,24 @@ private void analyzeAggregation(Analyzer analyzer) throws AnalysisException {
             LOG.debug("desctbl: " + analyzer.getDescTbl().debugString());
             LOG.debug("resultexprs: " + Expr.debugString(resultExprs));
         }
+        /*
+         * 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)))
 
 Review comment:
   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);```

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


[GitHub] [incubator-doris] wutiangan commented on a change in pull request #3150: Support non-correlated subquery in having clause

Posted by GitBox <gi...@apache.org>.
wutiangan commented on a change in pull request #3150: Support non-correlated subquery in having clause
URL: https://github.com/apache/incubator-doris/pull/3150#discussion_r397245174
 
 

 ##########
 File path: 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
 
 Review comment:
   ```suggestion
       // this parameter:  sum(t.k2) > 1
   ```

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


[GitHub] [incubator-doris] EmmyMiao87 closed pull request #3150: Support non-correlated subquery in having clause

Posted by GitBox <gi...@apache.org>.
EmmyMiao87 closed pull request #3150: Support non-correlated subquery in having clause
URL: https://github.com/apache/incubator-doris/pull/3150
 
 
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


[GitHub] [incubator-doris] morningman commented on a change in pull request #3150: Support non-correlated subquery in having clause

Posted by GitBox <gi...@apache.org>.
morningman commented on a change in pull request #3150: Support non-correlated subquery in having clause
URL: https://github.com/apache/incubator-doris/pull/3150#discussion_r396195211
 
 

 ##########
 File path: fe/src/main/java/org/apache/doris/analysis/ArithmeticExpr.java
 ##########
 @@ -231,6 +231,12 @@ public void analyzeImpl(Analyzer analyzer) throws AnalysisException {
             return;
         }
 
+        analyzeSubqueryInChildren();
+        // if children has subquery, it will be written and reanalyzed in the future.
 
 Review comment:
   ```suggestion
           // if children has subquery, it will be rewritten and reanalyzed in the future.
   ```

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


[GitHub] [incubator-doris] morningman commented on a change in pull request #3150: Support non-correlated subquery in having clause

Posted by GitBox <gi...@apache.org>.
morningman commented on a change in pull request #3150: Support non-correlated subquery in having clause
URL: https://github.com/apache/incubator-doris/pull/3150#discussion_r396198048
 
 

 ##########
 File path: fe/src/main/java/org/apache/doris/analysis/StmtRewriter.java
 ##########
 @@ -97,8 +104,142 @@ private static void rewriteSelectStatement(SelectStmt stmt, Analyzer analyzer)
             }
             rewriteWhereClauseSubqueries(stmt, analyzer);
         }
+        // Rewrite all subquery in the having clause
+        if (stmt.getHavingPred() != null && stmt.getHavingPred().getSubquery() != null) {
+            stmt = rewriteHavingClauseSubqueries(stmt, analyzer);
+        }
         stmt.sqlString_ = null;
         if (LOG.isDebugEnabled()) LOG.debug("rewritten stmt: " + stmt.toSql());
+        return stmt;
 
 Review comment:
   Not returning the `parameter`, use local variable instead.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


[GitHub] [incubator-doris] wutiangan commented on a change in pull request #3150: Support non-correlated subquery in having clause

Posted by GitBox <gi...@apache.org>.
wutiangan commented on a change in pull request #3150: Support non-correlated subquery in having clause
URL: https://github.com/apache/incubator-doris/pull/3150#discussion_r397274017
 
 

 ##########
 File path: fe/src/main/java/org/apache/doris/analysis/StmtRewriter.java
 ##########
 @@ -43,62 +44,260 @@
      * Rewrite the statement of an analysis result. The unanalyzed rewritten
      * statement is returned.
      */
-    public static void rewrite(Analyzer analyzer, StatementBase parsedStmt)
+    public static StatementBase rewrite(Analyzer analyzer, StatementBase parsedStmt)
             throws AnalysisException {
         if (parsedStmt instanceof QueryStmt) {
             QueryStmt analyzedStmt = (QueryStmt) parsedStmt;
             Preconditions.checkNotNull(analyzedStmt.analyzer);
-            rewriteQueryStatement(analyzedStmt, analyzer);
+            return rewriteQueryStatement(analyzedStmt, analyzer);
         } else if (parsedStmt instanceof InsertStmt) {
             final InsertStmt insertStmt = (InsertStmt)parsedStmt;
             final QueryStmt analyzedStmt = (QueryStmt)insertStmt.getQueryStmt();
             Preconditions.checkNotNull(analyzedStmt.analyzer);
-            rewriteQueryStatement(analyzedStmt, analyzer);
+            QueryStmt rewrittenQueryStmt = rewriteQueryStatement(analyzedStmt, analyzer);
+            insertStmt.setQueryStmt(rewrittenQueryStmt);
         } else {
             throw new AnalysisException("Unsupported statement containing subqueries: "
                     + parsedStmt.toSql());
         }
+        return parsedStmt;
     }
 
   /**
    *  Calls the appropriate rewrite method based on the specific type of query stmt. See
    *  rewriteSelectStatement() and rewriteUnionStatement() documentation.
    */
-    public static void rewriteQueryStatement(QueryStmt stmt, Analyzer analyzer)
+    public static QueryStmt rewriteQueryStatement(QueryStmt stmt, Analyzer analyzer)
             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);
         } else {
             throw new AnalysisException("Subqueries not supported for "
                     + stmt.getClass().getSimpleName() + " statements");
         }
+        return stmt;
     }
 
-    private static void rewriteSelectStatement(SelectStmt stmt, Analyzer analyzer)
+    private static SelectStmt rewriteSelectStatement(SelectStmt stmt, Analyzer analyzer)
             throws AnalysisException {
+        SelectStmt result = stmt;
         // Rewrite all the subqueries in the FROM clause.
-        for (TableRef tblRef: stmt.fromClause_) {
+        for (TableRef tblRef: result.fromClause_) {
             if (!(tblRef instanceof InlineViewRef)) continue;
             InlineViewRef inlineViewRef = (InlineViewRef)tblRef;
-            rewriteQueryStatement(inlineViewRef.getViewStmt(), inlineViewRef.getAnalyzer());
+            QueryStmt rewrittenQueryStmt = rewriteQueryStatement(inlineViewRef.getViewStmt(),
+                    inlineViewRef.getAnalyzer());
+            inlineViewRef.setViewStmt(rewrittenQueryStmt);
         }
         // Rewrite all the subqueries in the WHERE clause.
-        if (stmt.hasWhereClause()) {
+        if (result.hasWhereClause()) {
             // Push negation to leaf operands.
-            stmt.whereClause = Expr.pushNegationToOperands(stmt.whereClause);
+            result.whereClause = Expr.pushNegationToOperands(result.whereClause);
             // Check if we can rewrite the subqueries in the WHERE clause. OR predicates with
             // subqueries are not supported.
-            if (hasSubqueryInDisjunction(stmt.whereClause)) {
+            if (hasSubqueryInDisjunction(result.whereClause)) {
                 throw new AnalysisException("Subqueries in OR predicates are not supported: "
-                        + stmt.whereClause.toSql());
+                        + result.whereClause.toSql());
             }
-            rewriteWhereClauseSubqueries(stmt, analyzer);
+            rewriteWhereClauseSubqueries(result, analyzer);
         }
-        stmt.sqlString_ = null;
-        if (LOG.isDebugEnabled()) LOG.debug("rewritten stmt: " + stmt.toSql());
+        // Rewrite all subquery in the having clause
+        if (result.getHavingPred() != null && result.getHavingPred().getSubquery() != null) {
+            result = rewriteHavingClauseSubqueries(result, analyzer);
+        }
+        result.sqlString_ = null;
+        if (LOG.isDebugEnabled()) LOG.debug("rewritten stmt: " + result.toSql());
+        return result;
+    }
+
+    /**
+     * Rewrite having subquery.
+     * Step1: rewrite having subquery to where subquery
+     * Step2: rewrite where subquery
+     * <p>
+     * For example:
+     * 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);
+     * <p>
+     * Step1: rewrite having subquery to where subquery
+     * Outer query is changed to inline view in rewritten query
+     * Inline view of outer query:
+     *     from (select cs_item_sk, sum(cs_sales_price) sum_cs_sales_price from catalog_sales group by cs_item_sk) a
+     * Rewritten subquery of expr:
+     *     where a.sum_cs_sales_price >
+     *           (select min(cs_sales_price) from catalog_sales b where a.cs_item_sk = b.cs_item_sk)
+     * Rewritten query:
+     *     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
+     *     where a.sum_cs_sales_price >
+     *           (select min(cs_sales_price) from catalog_sales b where a.cs_item_sk = b.cs_item_sk)
+     * <p>
+     * Step2: rewrite where subquery
+     * 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)
+     * The final stmt:
+     * select a.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(b.cs_sales_price) min_cs_sales_price from catalog_sales b group by b.cs_item_sk) c
+     * where c.cs_item_sk = a.cs_item_sk and a.sum_cs_sales_price > c.min_cs_sales_price;
+     *
+     * @param stmt
+     * @param analyzer
+     */
+    private static SelectStmt rewriteHavingClauseSubqueries(SelectStmt stmt, Analyzer analyzer) throws AnalysisException {
+        // prepare parameters
+        SelectList selectList = stmt.getSelectList();
+        List<String> columnLables = stmt.getColLabels();
+        Expr havingClause = stmt.getHavingClauseAfterAnaylzed();
+        List<FunctionCallExpr> aggregateExprs = stmt.getAggInfo().getAggregateExprs();
+        Preconditions.checkState(havingClause != null);
+        Preconditions.checkState(havingClause.getSubquery() != null);
+        List<OrderByElement> orderByElements = stmt.getOrderByElementsAfterAnalyzed();
+        long limit = stmt.getLimit();
+        TableAliasGenerator tableAliasGenerator = stmt.getTableAliasGenerator();
+
+        /*
+         * The outer query is changed to inline view without having predicate
+         * 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
 
 Review comment:
   Inline view does not need to write the ‘from’ keyword

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


[GitHub] [incubator-doris] wutiangan commented on a change in pull request #3150: Support non-correlated subquery in having clause

Posted by GitBox <gi...@apache.org>.
wutiangan commented on a change in pull request #3150: Support non-correlated subquery in having clause
URL: https://github.com/apache/incubator-doris/pull/3150#discussion_r397235776
 
 

 ##########
 File path: fe/src/main/java/org/apache/doris/analysis/Expr.java
 ##########
 @@ -1462,6 +1462,18 @@ public Subquery getSubquery() {
         return subqueries.get(0);
     }
 
+    public boolean isCorrelatedPredicate(List<TupleId> tupleIdList) {
+        if ((this instanceof BinaryPredicate || this instanceof SlotRef) && !this.isBoundByTupleIds(tupleIdList)) {
+            return true;
+        }
 
 Review comment:
   why you add “this instanceof BinaryPredicate ”?
   only slotref has override isBoundByTupleIds function.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


[GitHub] [incubator-doris] EmmyMiao87 commented on a change in pull request #3150: Support non-correlated subquery in having clause

Posted by GitBox <gi...@apache.org>.
EmmyMiao87 commented on a change in pull request #3150: Support non-correlated subquery in having clause
URL: https://github.com/apache/incubator-doris/pull/3150#discussion_r397576967
 
 

 ##########
 File path: fe/src/main/java/org/apache/doris/analysis/SelectStmt.java
 ##########
 @@ -1438,6 +1476,22 @@ public boolean returnsSingleRow() {
         return false;
     }
 
+    @Override
+    public boolean containsCorrelatedPredicate() {
+        // check inline view
+        for (TableRef tableRef : fromClause_.getTableRefs()) {
+            if (tableRef instanceof InlineViewRef) {
+                if (((InlineViewRef) tableRef).getViewStmt().containsCorrelatedPredicate()) {
+                    return true;
+                }
+            }
+        }
+        if (whereClause == null) {
+            return false;
+        }
+        return whereClause.isCorrelatedPredicate(getTableRefIds());
 
 Review comment:
   This function has been removed.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


[GitHub] [incubator-doris] morningman commented on a change in pull request #3150: Support non-correlated subquery in having clause

Posted by GitBox <gi...@apache.org>.
morningman commented on a change in pull request #3150: Support non-correlated subquery in having clause
URL: https://github.com/apache/incubator-doris/pull/3150#discussion_r396200125
 
 

 ##########
 File path: fe/src/main/java/org/apache/doris/analysis/StmtRewriter.java
 ##########
 @@ -97,8 +104,142 @@ private static void rewriteSelectStatement(SelectStmt stmt, Analyzer analyzer)
             }
             rewriteWhereClauseSubqueries(stmt, analyzer);
         }
+        // Rewrite all subquery in the having clause
+        if (stmt.getHavingPred() != null && stmt.getHavingPred().getSubquery() != null) {
+            stmt = rewriteHavingClauseSubqueries(stmt, analyzer);
+        }
         stmt.sqlString_ = null;
         if (LOG.isDebugEnabled()) LOG.debug("rewritten stmt: " + stmt.toSql());
+        return stmt;
+    }
+
+    /**
+     * Rewrite having subquery.
+     * Step1: rewrite having subquery to where subquery
+     * Step2: rewrite where subquery
+     * <p>
+     * For example:
+     * 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);
+     * <p>
+     * Step1: rewrite having subquery to where subquery
+     * Outer query is changed to inline view in rewritten query
+     * Inline view of outer query:
+     *     from (select cs_item_sk, sum(cs_sales_price) sum_cs_sales_price from catalog_sales group by cs_item_sk) a
+     * Rewritten subquery of expr:
+     *     where a.sum_cs_sales_price >
+     *           (select min(cs_sales_price) from catalog_sales b where a.cs_item_sk = b.cs_item_sk)
+     * Rewritten query:
+     *     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
+     *     where a.sum_cs_sales_price >
+     *           (select min(cs_sales_price) from catalog_sales b where a.cs_item_sk = b.cs_item_sk)
+     * <p>
+     * Step2: rewrite where subquery
+     * 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)
 
 Review comment:
   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);
   ```
   
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


[GitHub] [incubator-doris] morningman commented on a change in pull request #3150: Support non-correlated subquery in having clause

Posted by GitBox <gi...@apache.org>.
morningman commented on a change in pull request #3150: Support non-correlated subquery in having clause
URL: https://github.com/apache/incubator-doris/pull/3150#discussion_r396201257
 
 

 ##########
 File path: fe/src/test/java/org/apache/doris/analysis/StmtRewriterTest.java
 ##########
 @@ -0,0 +1,63 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package org.apache.doris.analysis;
+
+import org.apache.doris.common.FeConstants;
+import org.apache.doris.utframe.DorisAssert;
+import org.apache.doris.utframe.UtFrameUtils;
+
+import org.junit.AfterClass;
+import org.junit.BeforeClass;
+import org.junit.Test;
+
+import java.util.UUID;
+
+public class StmtRewriterTest {
+
+    private static String baseDir = "fe";
+    private static String runningDir = baseDir + "/mocked/StmtRewriterTest/"
+            + UUID.randomUUID().toString() + "/";
+    private static final String TABLE_NAME = "table1";
+    private static final String DB_NAME = "db1";
+    private static DorisAssert dorisAssert;
+
+    @BeforeClass
+    public static void beforeClass() throws Exception{
+        FeConstants.runningUnitTest = true;
+        UtFrameUtils.createMinDorisCluster(runningDir);
+        dorisAssert = new DorisAssert();
+        dorisAssert.withDatabase(DB_NAME).useDatabase(DB_NAME);
+        String createTableSQL = "create table " + DB_NAME + "." + TABLE_NAME + " (empid int, name varchar, " +
+                "deptno int, salary int, commission int) "
+                + "distributed by hash(empid) buckets 3 properties('replication_num' = '1');";
+        dorisAssert.withTable(createTableSQL);
+    }
+
+    @Test
+    public void testRewriteHavingClauseSubqueries() throws Exception {
 
 Review comment:
   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);
   ```

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


[GitHub] [incubator-doris] wutiangan commented on a change in pull request #3150: Support non-correlated subquery in having clause

Posted by GitBox <gi...@apache.org>.
wutiangan commented on a change in pull request #3150: Support non-correlated subquery in having clause
URL: https://github.com/apache/incubator-doris/pull/3150#discussion_r397255334
 
 

 ##########
 File path: fe/src/main/java/org/apache/doris/analysis/SelectStmt.java
 ##########
 @@ -974,16 +1009,24 @@ private void analyzeAggregation(Analyzer analyzer) throws AnalysisException {
             LOG.debug("desctbl: " + analyzer.getDescTbl().debugString());
             LOG.debug("resultexprs: " + Expr.debugString(resultExprs));
         }
+        /*
+         * 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)))
 
 Review comment:
   why do you add FunctionCall to decorate sum(SlotRef(k2))?And There is no function at below “After rewritten” 。

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


[GitHub] [incubator-doris] EmmyMiao87 commented on a change in pull request #3150: Support non-correlated subquery in having clause

Posted by GitBox <gi...@apache.org>.
EmmyMiao87 commented on a change in pull request #3150: Support non-correlated subquery in having clause
URL: https://github.com/apache/incubator-doris/pull/3150#discussion_r397577244
 
 

 ##########
 File path: fe/src/main/java/org/apache/doris/analysis/StmtRewriter.java
 ##########
 @@ -43,62 +44,260 @@
      * Rewrite the statement of an analysis result. The unanalyzed rewritten
      * statement is returned.
      */
-    public static void rewrite(Analyzer analyzer, StatementBase parsedStmt)
+    public static StatementBase rewrite(Analyzer analyzer, StatementBase parsedStmt)
             throws AnalysisException {
         if (parsedStmt instanceof QueryStmt) {
             QueryStmt analyzedStmt = (QueryStmt) parsedStmt;
             Preconditions.checkNotNull(analyzedStmt.analyzer);
-            rewriteQueryStatement(analyzedStmt, analyzer);
+            return rewriteQueryStatement(analyzedStmt, analyzer);
         } else if (parsedStmt instanceof InsertStmt) {
             final InsertStmt insertStmt = (InsertStmt)parsedStmt;
             final QueryStmt analyzedStmt = (QueryStmt)insertStmt.getQueryStmt();
             Preconditions.checkNotNull(analyzedStmt.analyzer);
-            rewriteQueryStatement(analyzedStmt, analyzer);
+            QueryStmt rewrittenQueryStmt = rewriteQueryStatement(analyzedStmt, analyzer);
+            insertStmt.setQueryStmt(rewrittenQueryStmt);
         } else {
             throw new AnalysisException("Unsupported statement containing subqueries: "
                     + parsedStmt.toSql());
         }
+        return parsedStmt;
     }
 
   /**
    *  Calls the appropriate rewrite method based on the specific type of query stmt. See
    *  rewriteSelectStatement() and rewriteUnionStatement() documentation.
    */
-    public static void rewriteQueryStatement(QueryStmt stmt, Analyzer analyzer)
+    public static QueryStmt rewriteQueryStatement(QueryStmt stmt, Analyzer analyzer)
             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);
 
 Review comment:
   The rewriteUnionStatement is a function without return parameters.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


[GitHub] [incubator-doris] EmmyMiao87 commented on a change in pull request #3150: Support non-correlated subquery in having clause

Posted by GitBox <gi...@apache.org>.
EmmyMiao87 commented on a change in pull request #3150: Support non-correlated subquery in having clause
URL: https://github.com/apache/incubator-doris/pull/3150#discussion_r397577884
 
 

 ##########
 File path: fe/src/main/java/org/apache/doris/analysis/StmtRewriter.java
 ##########
 @@ -43,62 +44,260 @@
      * Rewrite the statement of an analysis result. The unanalyzed rewritten
      * statement is returned.
      */
-    public static void rewrite(Analyzer analyzer, StatementBase parsedStmt)
+    public static StatementBase rewrite(Analyzer analyzer, StatementBase parsedStmt)
             throws AnalysisException {
         if (parsedStmt instanceof QueryStmt) {
             QueryStmt analyzedStmt = (QueryStmt) parsedStmt;
             Preconditions.checkNotNull(analyzedStmt.analyzer);
-            rewriteQueryStatement(analyzedStmt, analyzer);
+            return rewriteQueryStatement(analyzedStmt, analyzer);
         } else if (parsedStmt instanceof InsertStmt) {
             final InsertStmt insertStmt = (InsertStmt)parsedStmt;
             final QueryStmt analyzedStmt = (QueryStmt)insertStmt.getQueryStmt();
             Preconditions.checkNotNull(analyzedStmt.analyzer);
-            rewriteQueryStatement(analyzedStmt, analyzer);
+            QueryStmt rewrittenQueryStmt = rewriteQueryStatement(analyzedStmt, analyzer);
+            insertStmt.setQueryStmt(rewrittenQueryStmt);
         } else {
             throw new AnalysisException("Unsupported statement containing subqueries: "
                     + parsedStmt.toSql());
         }
+        return parsedStmt;
     }
 
   /**
    *  Calls the appropriate rewrite method based on the specific type of query stmt. See
    *  rewriteSelectStatement() and rewriteUnionStatement() documentation.
    */
-    public static void rewriteQueryStatement(QueryStmt stmt, Analyzer analyzer)
+    public static QueryStmt rewriteQueryStatement(QueryStmt stmt, Analyzer analyzer)
             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);
         } else {
             throw new AnalysisException("Subqueries not supported for "
                     + stmt.getClass().getSimpleName() + " statements");
         }
+        return stmt;
     }
 
-    private static void rewriteSelectStatement(SelectStmt stmt, Analyzer analyzer)
+    private static SelectStmt rewriteSelectStatement(SelectStmt stmt, Analyzer analyzer)
             throws AnalysisException {
+        SelectStmt result = stmt;
         // Rewrite all the subqueries in the FROM clause.
-        for (TableRef tblRef: stmt.fromClause_) {
+        for (TableRef tblRef: result.fromClause_) {
             if (!(tblRef instanceof InlineViewRef)) continue;
             InlineViewRef inlineViewRef = (InlineViewRef)tblRef;
-            rewriteQueryStatement(inlineViewRef.getViewStmt(), inlineViewRef.getAnalyzer());
+            QueryStmt rewrittenQueryStmt = rewriteQueryStatement(inlineViewRef.getViewStmt(),
+                    inlineViewRef.getAnalyzer());
+            inlineViewRef.setViewStmt(rewrittenQueryStmt);
         }
         // Rewrite all the subqueries in the WHERE clause.
-        if (stmt.hasWhereClause()) {
+        if (result.hasWhereClause()) {
             // Push negation to leaf operands.
-            stmt.whereClause = Expr.pushNegationToOperands(stmt.whereClause);
+            result.whereClause = Expr.pushNegationToOperands(result.whereClause);
             // Check if we can rewrite the subqueries in the WHERE clause. OR predicates with
             // subqueries are not supported.
-            if (hasSubqueryInDisjunction(stmt.whereClause)) {
+            if (hasSubqueryInDisjunction(result.whereClause)) {
                 throw new AnalysisException("Subqueries in OR predicates are not supported: "
-                        + stmt.whereClause.toSql());
+                        + result.whereClause.toSql());
             }
-            rewriteWhereClauseSubqueries(stmt, analyzer);
+            rewriteWhereClauseSubqueries(result, analyzer);
         }
-        stmt.sqlString_ = null;
-        if (LOG.isDebugEnabled()) LOG.debug("rewritten stmt: " + stmt.toSql());
+        // Rewrite all subquery in the having clause
+        if (result.getHavingPred() != null && result.getHavingPred().getSubquery() != null) {
+            result = rewriteHavingClauseSubqueries(result, analyzer);
+        }
+        result.sqlString_ = null;
+        if (LOG.isDebugEnabled()) LOG.debug("rewritten stmt: " + result.toSql());
+        return result;
+    }
+
+    /**
+     * Rewrite having subquery.
+     * Step1: rewrite having subquery to where subquery
+     * Step2: rewrite where subquery
+     * <p>
+     * For example:
+     * 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);
+     * <p>
+     * Step1: rewrite having subquery to where subquery
+     * Outer query is changed to inline view in rewritten query
+     * Inline view of outer query:
+     *     from (select cs_item_sk, sum(cs_sales_price) sum_cs_sales_price from catalog_sales group by cs_item_sk) a
+     * Rewritten subquery of expr:
+     *     where a.sum_cs_sales_price >
+     *           (select min(cs_sales_price) from catalog_sales b where a.cs_item_sk = b.cs_item_sk)
+     * Rewritten query:
+     *     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
+     *     where a.sum_cs_sales_price >
+     *           (select min(cs_sales_price) from catalog_sales b where a.cs_item_sk = b.cs_item_sk)
+     * <p>
+     * Step2: rewrite where subquery
+     * 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)
+     * The final stmt:
+     * select a.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(b.cs_sales_price) min_cs_sales_price from catalog_sales b group by b.cs_item_sk) c
+     * where c.cs_item_sk = a.cs_item_sk and a.sum_cs_sales_price > c.min_cs_sales_price;
+     *
+     * @param stmt
+     * @param analyzer
+     */
+    private static SelectStmt rewriteHavingClauseSubqueries(SelectStmt stmt, Analyzer analyzer) throws AnalysisException {
+        // prepare parameters
+        SelectList selectList = stmt.getSelectList();
+        List<String> columnLables = stmt.getColLabels();
+        Expr havingClause = stmt.getHavingClauseAfterAnaylzed();
+        List<FunctionCallExpr> aggregateExprs = stmt.getAggInfo().getAggregateExprs();
+        Preconditions.checkState(havingClause != null);
+        Preconditions.checkState(havingClause.getSubquery() != null);
+        List<OrderByElement> orderByElements = stmt.getOrderByElementsAfterAnalyzed();
+        long limit = stmt.getLimit();
+        TableAliasGenerator tableAliasGenerator = stmt.getTableAliasGenerator();
+
+        /*
+         * The outer query is changed to inline view without having predicate
+         * 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
 
 Review comment:
   I am afraid of missing understanding.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


[GitHub] [incubator-doris] wutiangan commented on a change in pull request #3150: Support non-correlated subquery in having clause

Posted by GitBox <gi...@apache.org>.
wutiangan commented on a change in pull request #3150: Support non-correlated subquery in having clause
URL: https://github.com/apache/incubator-doris/pull/3150#discussion_r397273194
 
 

 ##########
 File path: fe/src/main/java/org/apache/doris/analysis/StmtRewriter.java
 ##########
 @@ -43,62 +44,260 @@
      * Rewrite the statement of an analysis result. The unanalyzed rewritten
      * statement is returned.
      */
-    public static void rewrite(Analyzer analyzer, StatementBase parsedStmt)
+    public static StatementBase rewrite(Analyzer analyzer, StatementBase parsedStmt)
             throws AnalysisException {
         if (parsedStmt instanceof QueryStmt) {
             QueryStmt analyzedStmt = (QueryStmt) parsedStmt;
             Preconditions.checkNotNull(analyzedStmt.analyzer);
-            rewriteQueryStatement(analyzedStmt, analyzer);
+            return rewriteQueryStatement(analyzedStmt, analyzer);
         } else if (parsedStmt instanceof InsertStmt) {
             final InsertStmt insertStmt = (InsertStmt)parsedStmt;
             final QueryStmt analyzedStmt = (QueryStmt)insertStmt.getQueryStmt();
             Preconditions.checkNotNull(analyzedStmt.analyzer);
-            rewriteQueryStatement(analyzedStmt, analyzer);
+            QueryStmt rewrittenQueryStmt = rewriteQueryStatement(analyzedStmt, analyzer);
+            insertStmt.setQueryStmt(rewrittenQueryStmt);
         } else {
             throw new AnalysisException("Unsupported statement containing subqueries: "
                     + parsedStmt.toSql());
         }
+        return parsedStmt;
     }
 
   /**
    *  Calls the appropriate rewrite method based on the specific type of query stmt. See
    *  rewriteSelectStatement() and rewriteUnionStatement() documentation.
    */
-    public static void rewriteQueryStatement(QueryStmt stmt, Analyzer analyzer)
+    public static QueryStmt rewriteQueryStatement(QueryStmt stmt, Analyzer analyzer)
             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);
         } else {
             throw new AnalysisException("Subqueries not supported for "
                     + stmt.getClass().getSimpleName() + " statements");
         }
+        return stmt;
     }
 
-    private static void rewriteSelectStatement(SelectStmt stmt, Analyzer analyzer)
+    private static SelectStmt rewriteSelectStatement(SelectStmt stmt, Analyzer analyzer)
             throws AnalysisException {
+        SelectStmt result = stmt;
         // Rewrite all the subqueries in the FROM clause.
-        for (TableRef tblRef: stmt.fromClause_) {
+        for (TableRef tblRef: result.fromClause_) {
             if (!(tblRef instanceof InlineViewRef)) continue;
             InlineViewRef inlineViewRef = (InlineViewRef)tblRef;
-            rewriteQueryStatement(inlineViewRef.getViewStmt(), inlineViewRef.getAnalyzer());
+            QueryStmt rewrittenQueryStmt = rewriteQueryStatement(inlineViewRef.getViewStmt(),
+                    inlineViewRef.getAnalyzer());
+            inlineViewRef.setViewStmt(rewrittenQueryStmt);
         }
         // Rewrite all the subqueries in the WHERE clause.
-        if (stmt.hasWhereClause()) {
+        if (result.hasWhereClause()) {
             // Push negation to leaf operands.
-            stmt.whereClause = Expr.pushNegationToOperands(stmt.whereClause);
+            result.whereClause = Expr.pushNegationToOperands(result.whereClause);
             // Check if we can rewrite the subqueries in the WHERE clause. OR predicates with
             // subqueries are not supported.
-            if (hasSubqueryInDisjunction(stmt.whereClause)) {
+            if (hasSubqueryInDisjunction(result.whereClause)) {
                 throw new AnalysisException("Subqueries in OR predicates are not supported: "
-                        + stmt.whereClause.toSql());
+                        + result.whereClause.toSql());
             }
-            rewriteWhereClauseSubqueries(stmt, analyzer);
+            rewriteWhereClauseSubqueries(result, analyzer);
         }
-        stmt.sqlString_ = null;
-        if (LOG.isDebugEnabled()) LOG.debug("rewritten stmt: " + stmt.toSql());
+        // Rewrite all subquery in the having clause
+        if (result.getHavingPred() != null && result.getHavingPred().getSubquery() != null) {
+            result = rewriteHavingClauseSubqueries(result, analyzer);
+        }
+        result.sqlString_ = null;
+        if (LOG.isDebugEnabled()) LOG.debug("rewritten stmt: " + result.toSql());
+        return result;
+    }
+
+    /**
 
 Review comment:
   you’d better add “corelated having subqueries are not supported” here。

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


[GitHub] [incubator-doris] morningman commented on a change in pull request #3150: Support non-correlated subquery in having clause

Posted by GitBox <gi...@apache.org>.
morningman commented on a change in pull request #3150: Support non-correlated subquery in having clause
URL: https://github.com/apache/incubator-doris/pull/3150#discussion_r396197288
 
 

 ##########
 File path: fe/src/main/java/org/apache/doris/analysis/StmtRewriter.java
 ##########
 @@ -43,47 +45,52 @@
      * Rewrite the statement of an analysis result. The unanalyzed rewritten
      * statement is returned.
      */
-    public static void rewrite(Analyzer analyzer, StatementBase parsedStmt)
+    public static StatementBase rewrite(Analyzer analyzer, StatementBase parsedStmt)
             throws AnalysisException {
         if (parsedStmt instanceof QueryStmt) {
             QueryStmt analyzedStmt = (QueryStmt) parsedStmt;
             Preconditions.checkNotNull(analyzedStmt.analyzer);
-            rewriteQueryStatement(analyzedStmt, analyzer);
+            parsedStmt = rewriteQueryStatement(analyzedStmt, analyzer);
 
 Review comment:
   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);
   ```

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


[GitHub] [incubator-doris] yangzhg commented on a change in pull request #3150: Support non-correlated subquery in having clause

Posted by GitBox <gi...@apache.org>.
yangzhg commented on a change in pull request #3150: Support non-correlated subquery in having clause
URL: https://github.com/apache/incubator-doris/pull/3150#discussion_r395505446
 
 

 ##########
 File path: fe/src/main/java/org/apache/doris/analysis/StmtRewriter.java
 ##########
 @@ -97,8 +103,128 @@ private static void rewriteSelectStatement(SelectStmt stmt, Analyzer analyzer)
             }
             rewriteWhereClauseSubqueries(stmt, analyzer);
         }
+        // Rewrite all subquery in the having clause
+        if (stmt.getHavingPred() != null && stmt.getHavingPred().getSubquery() != null) {
+            stmt = rewriteHavingClauseSubqueries(stmt, analyzer);
+        }
         stmt.sqlString_ = null;
         if (LOG.isDebugEnabled()) LOG.debug("rewritten stmt: " + stmt.toSql());
+        return stmt;
+    }
+
+    /**
+     * Rewrite having subquery.
+     * Step1: rewrite having subquery to where subquery
+     * Step2: rewrite where subquery
+     * <p>
+     * For example:
+     * 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);
+     * <p>
+     * Step1: rewrite having subquery to where subquery
+     * Outer query is changed to inline view in rewritten query
+     * Inline view of outer query:
+     *     from (select cs_item_sk, sum(cs_sales_price) sum_cs_sales_price from catalog_sales group by cs_item_sk) a
+     * Rewritten subquery of expr:
+     *     where a.sum_cs_sales_price >
+     *           (select min(cs_sales_price) from catalog_sales b where a.cs_item_sk = b.cs_item_sk)
+     * Rewritten query:
+     *     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
+     *     where a.sum_cs_sales_price >
+     *           (select min(cs_sales_price) from catalog_sales b where a.cs_item_sk = b.cs_item_sk)
+     * <p>
+     * Step2: rewrite where subquery
+     * 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)
+     *
+     * @param stmt
+     * @param analyzer
+     */
+    private static SelectStmt rewriteHavingClauseSubqueries(SelectStmt stmt, Analyzer analyzer) throws AnalysisException {
+        // extract having predicate
+        Expr havingPredicate = stmt.getHavingPred();
+        Preconditions.checkState(havingPredicate != null);
+        Preconditions.checkState(havingPredicate.getSubquery() != null);
+        // extract result of stmt
+        List<Expr> leftExprList = stmt.getResultExprs();
+        // extract table alias generator
+        TableAliasGenerator tableAliasGenerator = stmt.getTableAliasGenerator();
+
+        /*
+         * The outer query is changed to inline view without having predicate
+         * 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
+         */
+        SelectStmt inlineViewQuery = (SelectStmt) stmt.clone();
+        inlineViewQuery.reset();
+        inlineViewQuery.removeHavingClause();
+        // add a new alias for all of columns in subquery
+        List<String> colAliasOfInlineView = Lists.newArrayList();
+        for (int i = 0; i < inlineViewQuery.getSelectList().getItems().size(); ++i) {
+            colAliasOfInlineView.add(inlineViewQuery.getColumnAliasGenerator().getNextAlias());
+        }
+        InlineViewRef inlineViewRef = new InlineViewRef(tableAliasGenerator.getNextAlias(), inlineViewQuery,
+                colAliasOfInlineView);
+        try {
+            inlineViewRef.analyze(analyzer);
+        } catch (UserException e) {
+            throw new AnalysisException(e.getMessage());
+        }
+        LOG.debug("Outer query is changed to " + inlineViewRef.tableRefToSql());
+
+        /*
+         * Columns which belong to outer query can substitute for output columns of inline view
+         * For example:
+         * Having predicate: sum(cs_sales_price) >
+         *                   (select min(cs_sales_price) from catalog_sales b where a.cs_item_sk = b.cs_item_sk);
+         * Columns which belong to outer query: sum(cs_sales_price), a.cs_item_sk
+         * SMap: <cs_item_sk $ColumnA> <sum(cs_sales_price) $ColumnB>
+         * After substitute: $ColumnB >
+         *                   (select min(cs_sales_price) from catalog_sales b where $ColumnA = b.cs_item_sk)
+         */
+        /*
+         * Prepare select list of new query.
+         * Generate a new select item for each original columns in select list
+         */
+        havingPredicate.reset();
+        ExprSubstitutionMap smap = new ExprSubstitutionMap();
+        SelectList newSelectList = new SelectList();
+        for (int i = 0; i < inlineViewQuery.getSelectList().getItems().size(); i++) {
+            Expr leftExpr = leftExprList.get(i);
+            Expr rightExpr = new SlotRef(inlineViewRef.getAliasAsName(), colAliasOfInlineView.get(i));
+            rightExpr.analyze(analyzer);
+            smap.put(leftExpr, rightExpr);
+            // construct outer query select list
+            SelectListItem selectListItem = new SelectListItem(rightExpr, stmt.getColLabels().get(i));
+            newSelectList.addItem(selectListItem);
+        }
+        Expr newWherePredicate = havingPredicate.substitute(smap, analyzer,false);
+        LOG.debug("Having predicate is changed to " + newWherePredicate.toSql());
+
+        // construct rewritten query
+        List<TableRef> newTableRefList = Lists.newArrayList();
+        newTableRefList.add(inlineViewRef);
+        FromClause newFromClause = new FromClause(newTableRefList);
+        SelectStmt result = new SelectStmt(newSelectList, newFromClause, newWherePredicate, null, null, null,
+                LimitElement.NO_LIMIT);
 
 Review comment:
   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 is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


[GitHub] [incubator-doris] EmmyMiao87 opened a new pull request #3150: Support non-correlated subquery in having clause

Posted by GitBox <gi...@apache.org>.
EmmyMiao87 opened a new pull request #3150: Support non-correlated subquery in having clause
URL: https://github.com/apache/incubator-doris/pull/3150
 
 
   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);

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


[GitHub] [incubator-doris] EmmyMiao87 commented on a change in pull request #3150: Support non-correlated subquery in having clause

Posted by GitBox <gi...@apache.org>.
EmmyMiao87 commented on a change in pull request #3150: Support non-correlated subquery in having clause
URL: https://github.com/apache/incubator-doris/pull/3150#discussion_r397577244
 
 

 ##########
 File path: fe/src/main/java/org/apache/doris/analysis/StmtRewriter.java
 ##########
 @@ -43,62 +44,260 @@
      * Rewrite the statement of an analysis result. The unanalyzed rewritten
      * statement is returned.
      */
-    public static void rewrite(Analyzer analyzer, StatementBase parsedStmt)
+    public static StatementBase rewrite(Analyzer analyzer, StatementBase parsedStmt)
             throws AnalysisException {
         if (parsedStmt instanceof QueryStmt) {
             QueryStmt analyzedStmt = (QueryStmt) parsedStmt;
             Preconditions.checkNotNull(analyzedStmt.analyzer);
-            rewriteQueryStatement(analyzedStmt, analyzer);
+            return rewriteQueryStatement(analyzedStmt, analyzer);
         } else if (parsedStmt instanceof InsertStmt) {
             final InsertStmt insertStmt = (InsertStmt)parsedStmt;
             final QueryStmt analyzedStmt = (QueryStmt)insertStmt.getQueryStmt();
             Preconditions.checkNotNull(analyzedStmt.analyzer);
-            rewriteQueryStatement(analyzedStmt, analyzer);
+            QueryStmt rewrittenQueryStmt = rewriteQueryStatement(analyzedStmt, analyzer);
+            insertStmt.setQueryStmt(rewrittenQueryStmt);
         } else {
             throw new AnalysisException("Unsupported statement containing subqueries: "
                     + parsedStmt.toSql());
         }
+        return parsedStmt;
     }
 
   /**
    *  Calls the appropriate rewrite method based on the specific type of query stmt. See
    *  rewriteSelectStatement() and rewriteUnionStatement() documentation.
    */
-    public static void rewriteQueryStatement(QueryStmt stmt, Analyzer analyzer)
+    public static QueryStmt rewriteQueryStatement(QueryStmt stmt, Analyzer analyzer)
             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);
 
 Review comment:
   The rewriteUnionStatement is a funtion without return parameters.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


[GitHub] [incubator-doris] morningman commented on a change in pull request #3150: Support non-correlated subquery in having clause

Posted by GitBox <gi...@apache.org>.
morningman commented on a change in pull request #3150: Support non-correlated subquery in having clause
URL: https://github.com/apache/incubator-doris/pull/3150#discussion_r396197525
 
 

 ##########
 File path: fe/src/main/java/org/apache/doris/analysis/StmtRewriter.java
 ##########
 @@ -43,47 +45,52 @@
      * Rewrite the statement of an analysis result. The unanalyzed rewritten
      * statement is returned.
      */
-    public static void rewrite(Analyzer analyzer, StatementBase parsedStmt)
+    public static StatementBase rewrite(Analyzer analyzer, StatementBase parsedStmt)
             throws AnalysisException {
         if (parsedStmt instanceof QueryStmt) {
             QueryStmt analyzedStmt = (QueryStmt) parsedStmt;
             Preconditions.checkNotNull(analyzedStmt.analyzer);
-            rewriteQueryStatement(analyzedStmt, analyzer);
+            parsedStmt = rewriteQueryStatement(analyzedStmt, analyzer);
         } else if (parsedStmt instanceof InsertStmt) {
             final InsertStmt insertStmt = (InsertStmt)parsedStmt;
             final QueryStmt analyzedStmt = (QueryStmt)insertStmt.getQueryStmt();
             Preconditions.checkNotNull(analyzedStmt.analyzer);
-            rewriteQueryStatement(analyzedStmt, analyzer);
+            QueryStmt rewrittenQueryStmt = rewriteQueryStatement(analyzedStmt, analyzer);
+            insertStmt.setQueryStmt(rewrittenQueryStmt);
         } else {
             throw new AnalysisException("Unsupported statement containing subqueries: "
                     + parsedStmt.toSql());
         }
+        return parsedStmt;
     }
 
   /**
    *  Calls the appropriate rewrite method based on the specific type of query stmt. See
    *  rewriteSelectStatement() and rewriteUnionStatement() documentation.
    */
-    public static void rewriteQueryStatement(QueryStmt stmt, Analyzer analyzer)
+    public static QueryStmt rewriteQueryStatement(QueryStmt stmt, Analyzer analyzer)
             throws AnalysisException {
         Preconditions.checkNotNull(stmt);
         if (stmt instanceof SelectStmt) {
-            rewriteSelectStatement((SelectStmt) stmt, analyzer);
+            stmt = rewriteSelectStatement((SelectStmt) stmt, analyzer);
 
 Review comment:
   ```suggestion
               return rewriteSelectStatement((SelectStmt) stmt, analyzer);
   ```

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


[GitHub] [incubator-doris] EmmyMiao87 closed pull request #3150: Support non-correlated subquery in having clause

Posted by GitBox <gi...@apache.org>.
EmmyMiao87 closed pull request #3150: Support non-correlated subquery in having clause
URL: https://github.com/apache/incubator-doris/pull/3150
 
 
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


[GitHub] [incubator-doris] EmmyMiao87 commented on issue #3150: Support non-correlated subquery in having clause

Posted by GitBox <gi...@apache.org>.
EmmyMiao87 commented on issue #3150: Support non-correlated subquery in having clause
URL: https://github.com/apache/incubator-doris/pull/3150#issuecomment-601144289
 
 
   #2848 

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


[GitHub] [incubator-doris] EmmyMiao87 opened a new pull request #3150: Support non-correlated subquery in having clause

Posted by GitBox <gi...@apache.org>.
EmmyMiao87 opened a new pull request #3150: Support non-correlated subquery in having clause
URL: https://github.com/apache/incubator-doris/pull/3150
 
 
   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);

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


[GitHub] [incubator-doris] wutiangan commented on a change in pull request #3150: Support non-correlated subquery in having clause

Posted by GitBox <gi...@apache.org>.
wutiangan commented on a change in pull request #3150: Support non-correlated subquery in having clause
URL: https://github.com/apache/incubator-doris/pull/3150#discussion_r397259275
 
 

 ##########
 File path: fe/src/main/java/org/apache/doris/analysis/SelectStmt.java
 ##########
 @@ -1438,6 +1476,22 @@ public boolean returnsSingleRow() {
         return false;
     }
 
+    @Override
+    public boolean containsCorrelatedPredicate() {
+        // check inline view
+        for (TableRef tableRef : fromClause_.getTableRefs()) {
+            if (tableRef instanceof InlineViewRef) {
+                if (((InlineViewRef) tableRef).getViewStmt().containsCorrelatedPredicate()) {
+                    return true;
+                }
+            }
+        }
+        if (whereClause == null) {
+            return false;
+        }
+        return whereClause.isCorrelatedPredicate(getTableRefIds());
 
 Review comment:
   why do you only analyze where clause? what other clasue?for example having clasue,selectList。

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org