fix rollup with having (#21834)

fix rollup with having

Approved by: @heni02, @ouyuanning, @aressu1985, @aunjgr

Author: iamlinjunhong

pkg/pb/plan/plan.pb.go

@@ -390,8 +390,9 @@ func (node *SelectClause) GetQueryType() string     { return QueryTypeDQL }
 
 // WHERE or HAVING clause.
 type Where struct {
-	Type string
-	Expr Expr
+	Type         string
+	RollupHaving bool
+	Expr         Expr
 }
 
 func (node *Where) Format(ctx *FmtCtx) {

pkg/sql/parsers/tree/select.go

@@ -127,13 +127,20 @@ func (builder *QueryBuilder) pushdownFilters(nodeID int32, filters []*plan.Expr,
 
 	case plan.Node_FILTER:
 		canPushdown = filters
-		for _, filter := range node.FilterList {
-			canPushdown = append(canPushdown, splitPlanConjunction(applyDistributivity(builder.GetContext(), filter))...)
+		if !node.RollupFilter {
+			for _, filter := range node.FilterList {
+				canPushdown = append(canPushdown, splitPlanConjunction(applyDistributivity(builder.GetContext(), filter))...)
+			}
 		}
 
 		childID, cantPushdownChild := builder.pushdownFilters(node.Children[0], canPushdown, separateNonEquiConds)
 
-		if len(cantPushdownChild) > 0 {
+		if node.RollupFilter {
+			if len(cantPushdownChild) > 0 {
+				node.Children[0] = childID
+				node.FilterList = append(node.FilterList, cantPushdownChild...)
+			}
+		} else if len(cantPushdownChild) > 0 {
 			node.Children[0] = childID
 			node.FilterList = cantPushdownChild
 		} else {

pkg/sql/plan/pushdown.go

@@ -2638,6 +2638,7 @@ func (builder *QueryBuilder) bindSelect(stmt *tree.Select, ctx *BindContext, isR
 	var notCacheable bool
 	var helpFunc *helpFunc
 	var boundTimeWindowGroupBy *plan.Expr
+	var rollupFilter bool
 
 	astOrderBy := stmt.OrderBy
 	astLimit := stmt.Limit
@@ -2731,6 +2732,9 @@ func (builder *QueryBuilder) bindSelect(stmt *tree.Select, ctx *BindContext, isR
 				selectStmts := make([]*tree.SelectClause, groupingCount)
 				if groupingCount > 1 {
 					for i, list := range selectClause.GroupBy.GroupByExprsList {
+						if selectClause.Having != nil {
+							selectClause.Having.RollupHaving = true
+						}
 						selectStmts[i] = &tree.SelectClause{
 							Distinct: selectClause.Distinct,
 							Exprs:    selectClause.Exprs,
@@ -2769,6 +2773,9 @@ func (builder *QueryBuilder) bindSelect(stmt *tree.Select, ctx *BindContext, isR
 		); err != nil {
 			return
 		}
+		if selectClause.Having != nil {
+			rollupFilter = selectClause.Having.RollupHaving
+		}
 	case *tree.UnionClause:
 		return builder.buildUnion(selectClause, astOrderBy, astLimit, ctx, isRoot)
 	case *tree.ValuesClause:
@@ -2874,7 +2881,7 @@ func (builder *QueryBuilder) bindSelect(stmt *tree.Select, ctx *BindContext, isR
 			return
 		}
 	} else if len(ctx.groups) > 0 || len(ctx.aggregates) > 0 {
-		if nodeID, err = builder.appendAggNode(ctx, nodeID, boundHavingList); err != nil {
+		if nodeID, err = builder.appendAggNode(ctx, nodeID, boundHavingList, rollupFilter); err != nil {
 			return
 		}
 	}
@@ -3545,6 +3552,7 @@ func (builder *QueryBuilder) appendAggNode(
 	ctx *BindContext,
 	nodeID int32,
 	boundHavingList []*plan.Expr,
+	rollupFilter bool,
 ) (newNodeID int32, err error) {
 	if ctx.bindingRecurStmt() {
 		err = moerr.NewInternalError(builder.GetContext(), "not support aggregate function recursive cte")
@@ -3579,9 +3587,10 @@ func (builder *QueryBuilder) appendAggNode(
 		}
 
 		nodeID = builder.appendNode(&plan.Node{
-			NodeType:   plan.Node_FILTER,
-			Children:   []int32{nodeID},
-			FilterList: newFilterList,
+			NodeType:     plan.Node_FILTER,
+			Children:     []int32{nodeID},
+			FilterList:   newFilterList,
+			RollupFilter: rollupFilter,
 		}, ctx)
 	}
 

pkg/sql/plan/query_builder.go

@@ -456,7 +456,7 @@ func TestQueryBuilder_appendAggNode(t *testing.T) {
 	boundHavingList, err := builder.bindHaving(bindCtx, selectClause.Having, NewHavingBinder(builder, bindCtx))
 	require.NoError(t, err)
 
-	nodeID, err = builder.appendAggNode(bindCtx, nodeID, boundHavingList)
+	nodeID, err = builder.appendAggNode(bindCtx, nodeID, boundHavingList, false)
 	require.NoError(t, err)
 	require.Equal(t, int32(2), nodeID)
 	require.Equal(t, 3, len(builder.qry.Nodes))

pkg/sql/plan/query_builder_test.go

@@ -853,6 +853,7 @@ message Node {
 	OnDuplicateAction on_duplicate_action = 66;
 	string dedup_col_name = 67;
 	repeated Type dedup_col_types = 68 [(gogoproto.nullable) = false];
+	bool rollup_filter = 69;
 }
 
 // Snapshot Represents a snapshot of the database

proto/plan.proto

@@ -151,7 +151,18 @@ product with rollup
 having
 (product is not null and quarter is not null) or
 (product is null and quarter is null);
-[unknown result because it is related to issue#19984]
+year    quarter    product    total_sales
+null    null    null    22000.00
+2022    null    null    7000.00
+2023    null    null    15000.00
+2022    Q1    Product A    1000.00
+2022    Q1    Product B    1500.00
+2022    Q2    Product A    2000.00
+2022    Q2    Product C    2500.00
+2023    Q1    Product A    3000.00
+2023    Q1    Product B    3500.00
+2023    Q2    Product B    4000.00
+2023    Q2    Product C    4500.00
 select
 year,
 quarter,
@@ -165,7 +176,13 @@ quarter,
 product with rollup
 having
 (product = 'Product A') or (product = 'Product B');
-[unknown result because it is related to issue#19984]
+year    quarter    product    total_sales
+2022    Q1    Product A    1000.00
+2022    Q1    Product B    1500.00
+2022    Q2    Product A    2000.00
+2023    Q1    Product A    3000.00
+2023    Q1    Product B    3500.00
+2023    Q2    Product B    4000.00
 drop table sales;
 drop table if exists orders;
 drop table if exists order_items;
@@ -367,7 +384,15 @@ group by
 j, i with rollup
 having
 (i is not null and j is not null);
-[unknown result because it is related to issue#19984]
+i    j    total
+18446744073709551615    2147483647    123214
+4294967295    2147483647    2
+18446744073709551615    1    2
+2147483647    23289483    123214
+13289392    2    2
+18446744073709551615    23289483    1
+3824    13289392    123215
+2438294    1    2
 drop table uint_64;
 drop table if exists sales;
 create table sales (

test/distributed/cases/window/rollup.result

@@ -84,7 +84,6 @@ group by
     product with rollup;
 -- @bvt:issue
 
--- @bvt:issue#19984
 select
     year,
     quarter,
@@ -113,7 +112,6 @@ group by
     product with rollup
 having
      (product = 'Product A') or (product = 'Product B');
--- @bvt:issue
 drop table sales;
 
 
@@ -301,7 +299,6 @@ group by
     j, i with rollup;
 -- @bvt:issue
 
--- @bvt:issue#19984
 select
     i, j, sum(k) as total
 from
@@ -310,7 +307,6 @@ group by
     j, i with rollup
 having
     (i is not null and j is not null);
--- @bvt:issue
 drop table uint_64;