Skip to content

Commit

Permalink
planner, CTE, view: Fix default inline CTE which contains orderby/lim…
Browse files Browse the repository at this point in the history 
…it/distinct and inside of view | tidb-test=e1d0c1e615f749e7139f5be95bc4a2b8cedb7380 (pingcap#56609) (pingcap#56666)

close pingcap#56582, close pingcap#56603
  • Loading branch information
@ti-chi-bot
ti-chi-bot authored Oct 16, 2024
1 parent 3635e94 commit 7972ff1
Show file tree
Hide file tree
Showing 8 changed files with 246 additions and 39 deletions.
33 changes: 27 additions & 6 deletions planner/core/logical_plan_builder.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,7 @@ func (b *PlanBuilder) buildAggregation(ctx context.Context, p LogicalPlan, aggFu

// flag it if cte contain aggregation
if b.buildingCTE {
b.outerCTEs[len(b.outerCTEs)-1].containAggOrWindow = true
b.outerCTEs[len(b.outerCTEs)-1].containRecursiveForbiddenOperator = true
}

plan4Agg := LogicalAggregation{AggFuncs: make([]*aggregation.AggFuncDesc, 0, len(aggFuncList))}.Init(b.ctx, b.getSelectOffset())
Expand Down Expand Up @@ -1658,6 +1658,10 @@ func (b *PlanBuilder) buildProjection(ctx context.Context, p LogicalPlan, fields
func (b *PlanBuilder) buildDistinct(child LogicalPlan, length int) (*LogicalAggregation, error) {
b.optFlag = b.optFlag | flagBuildKeyInfo
b.optFlag = b.optFlag | flagPushDownAgg
// flag it if cte contain distinct
if b.buildingCTE {
b.outerCTEs[len(b.outerCTEs)-1].containRecursiveForbiddenOperator = true
}
plan4Agg := LogicalAggregation{
AggFuncs: make([]*aggregation.AggFuncDesc, 0, child.Schema().Len()),
GroupByItems: expression.Column2Exprs(child.Schema().Clone().Columns[:length]),
Expand Down Expand Up @@ -2221,6 +2225,10 @@ func extractLimitCountOffset(ctx sessionctx.Context, limit *ast.Limit) (count ui

func (b *PlanBuilder) buildLimit(src LogicalPlan, limit *ast.Limit) (LogicalPlan, error) {
b.optFlag = b.optFlag | flagPushDownTopN
// flag it if cte contain limit
if b.buildingCTE {
b.outerCTEs[len(b.outerCTEs)-1].containRecursiveForbiddenOperator = true
}
var (
offset, count uint64
err error
Expand Down Expand Up @@ -4292,6 +4300,10 @@ func (b *PlanBuilder) buildSelect(ctx context.Context, sel *ast.SelectStmt) (p L
}

if sel.OrderBy != nil {
// flag it if cte contain order by
if b.buildingCTE {
b.outerCTEs[len(b.outerCTEs)-1].containRecursiveForbiddenOperator = true
}
// We need to keep the ORDER BY clause for the following cases:
// 1. The select is top level query, order should be honored
// 2. The query has LIMIT clause
Expand Down Expand Up @@ -4483,9 +4495,9 @@ func (b *PlanBuilder) tryBuildCTE(ctx context.Context, tn *ast.TableName, asName
prevSchema := cte.seedLP.Schema().Clone()
lp.SetSchema(getResultCTESchema(cte.seedLP.Schema(), b.ctx.GetSessionVars()))

// If current CTE query contain another CTE which 'containAggOrWindow' is true, current CTE 'containAggOrWindow' will be true
// If current CTE query contain another CTE which 'containRecursiveForbiddenOperator' is true, current CTE 'containRecursiveForbiddenOperator' will be true
if b.buildingCTE {
b.outerCTEs[len(b.outerCTEs)-1].containAggOrWindow = cte.containAggOrWindow || b.outerCTEs[len(b.outerCTEs)-1].containAggOrWindow
b.outerCTEs[len(b.outerCTEs)-1].containRecursiveForbiddenOperator = cte.containRecursiveForbiddenOperator || b.outerCTEs[len(b.outerCTEs)-1].containRecursiveForbiddenOperator
}
// Compute cte inline
b.computeCTEInlineFlag(cte)
Expand Down Expand Up @@ -4543,13 +4555,22 @@ func (b *PlanBuilder) computeCTEInlineFlag(cte *cteInfo) {
b.ctx.GetSessionVars().StmtCtx.AppendWarning(
ErrInternal.GenWithStack("Recursive CTE %s can not be inlined by merge() or tidb_opt_force_inline_cte.", cte.def.Name))
}
} else if cte.containAggOrWindow && b.buildingRecursivePartForCTE {
cte.isInline = false
} else if cte.containRecursiveForbiddenOperator && b.buildingRecursivePartForCTE {
if cte.forceInlineByHintOrVar {
b.ctx.GetSessionVars().StmtCtx.AppendWarning(ErrCTERecursiveForbidsAggregation.FastGenByArgs(cte.def.Name))
}
} else if cte.consumerCount > 1 {
cte.isInline = false
} else if cte.consumerCount != 1 {
// If hint or session variable is set, it can be inlined by user.
if cte.forceInlineByHintOrVar {
cte.isInline = true
} else {
// Consumer count > 1 or = 0, CTE can not be inlined by default.
// Case the consumer count = 0 (issue #56582)
// It means that CTE maybe inside of view and the UpdateCTEConsumerCount(preprocess phase) is skipped
// So all of CTE.consumerCount is not updated, and we can not use it to determine whether CTE can be inlined.
cte.isInline = false
}
} else {
cte.isInline = true
Expand Down Expand Up @@ -6605,7 +6626,7 @@ func sortWindowSpecs(groupedFuncs map[*ast.WindowSpec][]*ast.WindowFuncExpr, ord

func (b *PlanBuilder) buildWindowFunctions(ctx context.Context, p LogicalPlan, groupedFuncs map[*ast.WindowSpec][]*ast.WindowFuncExpr, orderedSpec []*ast.WindowSpec, aggMap map[*ast.AggregateFuncExpr]int) (LogicalPlan, map[*ast.WindowFuncExpr]int, error) {
if b.buildingCTE {
b.outerCTEs[len(b.outerCTEs)-1].containAggOrWindow = true
b.outerCTEs[len(b.outerCTEs)-1].containRecursiveForbiddenOperator = true
}
args := make([]ast.ExprNode, 0, 4)
windowMap := make(map[*ast.WindowFuncExpr]int)
Expand Down
3 changes: 2 additions & 1 deletion planner/core/physical_plan_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1289,6 +1289,7 @@ func TestSingleConsumerCTE(t *testing.T) {
tk.MustExec("create table t1 (c1 int primary key, c2 int, index c2 (c2));")
tk.MustExec("create table t2 (c1 int unique, c2 int);")
tk.MustExec("insert into t values (1), (5), (10), (15), (20), (30), (50);")
tk.MustExec("create table test(a int);")

var (
input []string
Expand All @@ -1305,7 +1306,7 @@ func TestSingleConsumerCTE(t *testing.T) {
testdata.OnRecord(func() {
output[i].SQL = ts
})
if strings.HasPrefix(ts, "set") {
if strings.HasPrefix(ts, "set") || strings.HasPrefix(ts, "create") {
tk.MustExec(ts)
continue
}
Expand Down
14 changes: 0 additions & 14 deletions planner/core/plan_stats_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -160,20 +160,6 @@ func TestPlanStatsLoad(t *testing.T) {
require.Greater(t, countFullStats(ptr.Stats().HistColl, tableInfo.Columns[2].ID), 0)
},
},
{ // CTE
sql: "with cte(x, y) as (select d + 1, b from t where c > 1) select * from cte where x < 3",
check: func(p plannercore.Plan, tableInfo *model.TableInfo) {
ps, ok := p.(*plannercore.PhysicalProjection)
require.True(t, ok)
pc, ok := ps.Children()[0].(*plannercore.PhysicalTableReader)
require.True(t, ok)
pp, ok := pc.GetTablePlan().(*plannercore.PhysicalSelection)
require.True(t, ok)
reader, ok := pp.Children()[0].(*plannercore.PhysicalTableScan)
require.True(t, ok)
require.Greater(t, countFullStats(reader.Stats().HistColl, tableInfo.Columns[2].ID), 0)
},
},
{ // recursive CTE
sql: "with recursive cte(x, y) as (select a, b from t where c > 1 union select x + 1, y from cte where x < 5) select * from cte",
check: func(p plannercore.Plan, tableInfo *model.TableInfo) {
Expand Down
4 changes: 2 additions & 2 deletions planner/core/planbuilder.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -492,8 +492,8 @@ type cteInfo struct {
isInline bool
// forceInlineByHintOrVar will be true when CTE is hint by merge() or session variable "tidb_opt_force_inline_cte=true"
forceInlineByHintOrVar bool
// If CTE contain aggregation or window function in query (Indirect references to other cte containing agg or window in the query are also counted.)
containAggOrWindow bool
// If CTE contain aggregation, window function, order by, distinct and limit in query (Indirect references to other cte containing those operator in the query are also counted.)
containRecursiveForbiddenOperator bool
// Compute in preprocess phase. Record how many consumers the current CTE has
consumerCount int
}
Expand Down
75 changes: 70 additions & 5 deletions planner/core/testdata/flat_plan_suite_out.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -212,8 +212,8 @@
{
"Depth": 2,
"Label": 0,
"IsRoot": false,
"StoreType": 0,
"IsRoot": true,
"StoreType": 2,
"ReqType": 0,
"IsPhysicalPlan": true,
"TextTreeIndent": "│ │ ",
Expand All @@ -232,15 +232,80 @@
{
"Depth": 2,
"Label": 0,
"IsRoot": false,
"StoreType": 0,
"IsRoot": true,
"StoreType": 2,
"ReqType": 0,
"IsPhysicalPlan": true,
"TextTreeIndent": " │ ",
"IsLastChild": true
}
],
"CTEs": null
"CTEs": [
[
{
"Depth": 0,
"Label": 0,
"IsRoot": true,
"StoreType": 2,
"ReqType": 0,
"IsPhysicalPlan": true,
"TextTreeIndent": "",
"IsLastChild": true
},
{
"Depth": 1,
"Label": 3,
"IsRoot": true,
"StoreType": 2,
"ReqType": 0,
"IsPhysicalPlan": true,
"TextTreeIndent": "│ ",
"IsLastChild": true
},
{
"Depth": 2,
"Label": 0,
"IsRoot": false,
"StoreType": 0,
"ReqType": 0,
"IsPhysicalPlan": true,
"TextTreeIndent": " │ ",
"IsLastChild": true
}
],
[
{
"Depth": 0,
"Label": 0,
"IsRoot": true,
"StoreType": 2,
"ReqType": 0,
"IsPhysicalPlan": true,
"TextTreeIndent": "",
"IsLastChild": true
},
{
"Depth": 1,
"Label": 3,
"IsRoot": true,
"StoreType": 2,
"ReqType": 0,
"IsPhysicalPlan": true,
"TextTreeIndent": "│ ",
"IsLastChild": true
},
{
"Depth": 2,
"Label": 0,
"IsRoot": false,
"StoreType": 0,
"ReqType": 0,
"IsPhysicalPlan": true,
"TextTreeIndent": " │ ",
"IsLastChild": true
}
]
]
},
{
"SQL": "WITH RECURSIVE cte (n) AS( SELECT 1 UNION ALL SELECT n + 1 FROM cte WHERE n < 5)SELECT * FROM cte;",
Expand Down
23 changes: 13 additions & 10 deletions planner/core/testdata/integration_suite_out.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6592,21 +6592,24 @@
{
"SQL": "explain format = 'brief' select /*+ qb_name(qb_v8, v8), merge(@qb_v8) */ * from v8;",
"Plan": [
"HashAgg 16000.00 root group by:Column#41, funcs:firstrow(Column#41)->Column#41",
"HashAgg 16000.00 root group by:Column#21, funcs:firstrow(Column#21)->Column#21",
"└─Union 1000000010000.00 root ",
" ├─HashJoin 1000000000000.00 root CARTESIAN inner join",
" │ ├─TableReader(Build) 10000.00 root data:TableFullScan",
" │ │ └─TableFullScan 10000.00 cop[tikv] table:t1 keep order:false, stats:pseudo",
" │ └─Projection(Probe) 100000000.00 root 1->Column#55",
" │ └─HashJoin 100000000.00 root CARTESIAN inner join",
" │ ├─Projection(Build) 10000.00 root 1->Column#54",
" │ │ └─IndexReader 10000.00 root index:IndexFullScan",
" │ │ └─IndexFullScan 10000.00 cop[tikv] table:t3, index:idx_a(a) keep order:false, stats:pseudo",
" │ └─Projection(Probe) 10000.00 root 1->Column#53",
" │ └─IndexReader 10000.00 root index:IndexFullScan",
" │ └─IndexFullScan 10000.00 cop[tikv] table:t2, index:idx_a(a) keep order:false, stats:pseudo",
" │ └─CTEFullScan(Probe) 100000000.00 root CTE:cte2 data:CTE_1",
" └─TableReader 10000.00 root data:TableFullScan",
" └─TableFullScan 10000.00 cop[tikv] table:t1 keep order:false, stats:pseudo"
" └─TableFullScan 10000.00 cop[tikv] table:t1 keep order:false, stats:pseudo",
"CTE_1 100000000.00 root Non-Recursive CTE",
"└─HashJoin(Seed Part) 100000000.00 root CARTESIAN inner join",
" ├─CTEFullScan(Build) 10000.00 root CTE:cte4 data:CTE_3",
" └─CTEFullScan(Probe) 10000.00 root CTE:cte3 data:CTE_2",
"CTE_3 10000.00 root Non-Recursive CTE",
"└─IndexReader(Seed Part) 10000.00 root index:IndexFullScan",
" └─IndexFullScan 10000.00 cop[tikv] table:t3, index:idx_a(a) keep order:false, stats:pseudo",
"CTE_2 10000.00 root Non-Recursive CTE",
"└─IndexReader(Seed Part) 10000.00 root index:IndexFullScan",
" └─IndexFullScan 10000.00 cop[tikv] table:t2, index:idx_a(a) keep order:false, stats:pseudo"
],
"Warn": null
},
Expand Down
11 changes: 10 additions & 1 deletion planner/core/testdata/plan_suite_in.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -650,7 +650,16 @@
"with cte as (select 1) select /*+ MERGE() */ * from cte union select * from cte; -- firstly inline cte, secondly cannot be inlined",
"with a as (select 8 as id from dual),maxa as (select max(id) as max_id from a),b as (with recursive temp as (select 1 as lvl from dual union all select lvl+1 from temp, maxa where lvl < max_id)select * from temp) select * from b; -- issue #47711, maxa cannot be inlined because it contains agg and in the recursive part of cte temp",
"with a as (select count(*) from t1), b as (select 2 as bb from a), c as (with recursive tmp as (select 1 as res from t1 union all select res+1 from tmp,b where res+1 < bb) select * from tmp) select * from c; -- inline a, cannot be inline b because b indirectly contains agg and in the recursive part of cte tmp",
"with a as (select count(*) from t1), b as (select 2 as bb from a), c as (with recursive tmp as (select bb as res from b union all select res+1 from tmp where res +1 < 10) select * from tmp) select * from c; -- inline a, b, cannot be inline tmp, c"
"with a as (select count(*) from t1), b as (select 2 as bb from a), c as (with recursive tmp as (select bb as res from b union all select res+1 from tmp where res +1 < 10) select * from tmp) select * from c; -- inline a, b, cannot be inline tmp, c",
"WITH RECURSIVE CTE (x) AS (SELECT 1 UNION ALL SELECT distinct a FROM test), CTE1 AS (SELECT x FROM CTE UNION ALL select CTE.x from CTE join CTE1 on CTE.x=CTE1.x) SELECT * FROM CTE1; -- CTE contain distinct and ref by CET1 recursive part cannot be inlined;",
"create view test_cte(a) as WITH RECURSIVE CTE (x) AS (SELECT 1 UNION ALL SELECT distinct a FROM test) , CTE1 AS (SELECT x FROM CTE UNION ALL select CTE.x from CTE join CTE1 on CTE.x=CTE1.x) SELECT * FROM CTE1;",
"select * from test_cte; -- CTE (inside of view) cannot be inlined by default;",
"create view test_inline_cte(a) as with CTE (x) as (select distinct a from test) select * from CTE;",
"select * from test_inline_cte; -- CTE (inside of view) cannot be inlined by default;",
"create view test_force_inline_cte(a) as with CTE (x) as (select /*+ merge() */ distinct a from test) select * from CTE;",
"select * from test_force_inline_cte; -- CTE (inside of view) can be inlined by force;" ,
"WITH RECURSIVE CTE (x) AS (SELECT a FROM test limit 1) , CTE1(x) AS (SELECT a FROM test UNION ALL select CTE.x from CTE join CTE1 on CTE.x=CTE1.x) SELECT * FROM CTE1; -- CTE contain limit and ref by CET1 recursive part cannot be inlined;",
"WITH RECURSIVE CTE (x) AS (SELECT a FROM test order by a) , CTE1(x) AS (SELECT a FROM test UNION ALL select CTE.x from CTE join CTE1 on CTE.x=CTE1.x) SELECT * FROM CTE1; -- CTE contain order by and ref by CET1 recursive part cannot be inlined;"
]
},
{
Expand Down
Loading

0 comments on commit 7972ff1

Please sign in to comment.