Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

planner, CTE, view: Fix default inline CTE which contains orderby/limit/distinct and inside of view | tidb-test=e1d0c1e615f749e7139f5be95bc4a2b8cedb7380 (#56609) #56666

Merged
merged 2 commits into from
Oct 16, 2024
Merged

planner, CTE, view: Fix default inline CTE which contains orderby/limit/distinct and inside of view | tidb-test=e1d0c1e615f749e7139f5be95bc4a2b8cedb7380 (#56609) #56666

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
481f4c2
This is an automated cherry-pick of #56609
elsa0520 Oct 15, 2024
abc02e6
fix conflick
elsa0520 Oct 16, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 27 additions & 6 deletions planner/core/logical_plan_builder.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,7 @@

// 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) 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 (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 @@
}

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 @@
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 @@
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 (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

Check warning on line 6629 in planner/core/logical_plan_builder.go

View check run for this annotation

Codecov / codecov/patch

planner/core/logical_plan_builder.go#L6629 

Added line #L6629 was not covered by tests
}
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Loading