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: fix possible inconsistent output cols among union's children (#48775) #48895

Merged
merged 5 commits into from
Dec 8, 2023
Merged

planner: fix possible inconsistent output cols among union's children (#48775) #48895

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
bc7df18
planner: fix possible inconsistent output cols among union's children…
winoros Dec 6, 2023
59d4c26
Merge branch 'release-7.1' into cherry-pick-48775-to-release-7.1
winoros Dec 8, 2023
43247f2
Update planner_issue_test.go
winoros Dec 8, 2023
7e954ed
Update BUILD.bazel
winoros Dec 8, 2023
f908008
fix tests
winoros Dec 8, 2023
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
49 changes: 27 additions & 22 deletions executor/index_merge_reader_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -568,22 +568,26 @@ func TestPessimisticLockOnPartitionForIndexMerge(t *testing.T) {
" ├─IndexReader(Build) 3.00 root index:IndexFullScan",
" │ └─IndexFullScan 3.00 cop[tikv] table:t2, index:c_datetime(c_datetime) keep order:false",
" └─PartitionUnion(Probe) 5545.21 root ",
" ├─IndexMerge 5542.21 root type: union",
" │ ├─IndexRangeScan(Build) 3323.33 cop[tikv] table:t1, partition:p0, index:c1(c1) range:[-inf,10), keep order:false, stats:pseudo",
" │ ├─IndexRangeScan(Build) 3323.33 cop[tikv] table:t1, partition:p0, index:c2(c2) range:[-inf,10), keep order:false, stats:pseudo",
" │ └─TableRowIDScan(Probe) 5542.21 cop[tikv] table:t1, partition:p0 keep order:false, stats:pseudo",
" ├─IndexMerge 1.00 root type: union",
" │ ├─IndexRangeScan(Build) 1.00 cop[tikv] table:t1, partition:p1, index:c1(c1) range:[-inf,10), keep order:false",
" │ ├─IndexRangeScan(Build) 1.00 cop[tikv] table:t1, partition:p1, index:c2(c2) range:[-inf,10), keep order:false",
" │ └─TableRowIDScan(Probe) 1.00 cop[tikv] table:t1, partition:p1 keep order:false",
" ├─IndexMerge 1.00 root type: union",
" │ ├─IndexRangeScan(Build) 1.00 cop[tikv] table:t1, partition:p2, index:c1(c1) range:[-inf,10), keep order:false",
" │ ├─IndexRangeScan(Build) 1.00 cop[tikv] table:t1, partition:p2, index:c2(c2) range:[-inf,10), keep order:false",
" │ └─TableRowIDScan(Probe) 1.00 cop[tikv] table:t1, partition:p2 keep order:false",
" └─IndexMerge 1.00 root type: union",
" ├─IndexRangeScan(Build) 1.00 cop[tikv] table:t1, partition:p3, index:c1(c1) range:[-inf,10), keep order:false",
" ├─IndexRangeScan(Build) 1.00 cop[tikv] table:t1, partition:p3, index:c2(c2) range:[-inf,10), keep order:false",
" └─TableRowIDScan(Probe) 1.00 cop[tikv] table:t1, partition:p3 keep order:false",
" ├─Projection 5542.21 root test.t1.c_datetime, test.t1.c1, test.t1._tidb_rowid, test.t1._tidb_tid",
" │ └─IndexMerge 5542.21 root type: union",
" │ ├─IndexRangeScan(Build) 3323.33 cop[tikv] table:t1, partition:p0, index:c1(c1) range:[-inf,10), keep order:false, stats:pseudo",
" │ ├─IndexRangeScan(Build) 3323.33 cop[tikv] table:t1, partition:p0, index:c2(c2) range:[-inf,10), keep order:false, stats:pseudo",
" │ └─TableRowIDScan(Probe) 5542.21 cop[tikv] table:t1, partition:p0 keep order:false, stats:pseudo",
" ├─Projection 1.00 root test.t1.c_datetime, test.t1.c1, test.t1._tidb_rowid, test.t1._tidb_tid",
" │ └─IndexMerge 1.00 root type: union",
" │ ├─IndexRangeScan(Build) 1.00 cop[tikv] table:t1, partition:p1, index:c1(c1) range:[-inf,10), keep order:false",
" │ ├─IndexRangeScan(Build) 1.00 cop[tikv] table:t1, partition:p1, index:c2(c2) range:[-inf,10), keep order:false",
" │ └─TableRowIDScan(Probe) 1.00 cop[tikv] table:t1, partition:p1 keep order:false",
" ├─Projection 1.00 root test.t1.c_datetime, test.t1.c1, test.t1._tidb_rowid, test.t1._tidb_tid",
" │ └─IndexMerge 1.00 root type: union",
" │ ├─IndexRangeScan(Build) 1.00 cop[tikv] table:t1, partition:p2, index:c1(c1) range:[-inf,10), keep order:false",
" │ ├─IndexRangeScan(Build) 1.00 cop[tikv] table:t1, partition:p2, index:c2(c2) range:[-inf,10), keep order:false",
" │ └─TableRowIDScan(Probe) 1.00 cop[tikv] table:t1, partition:p2 keep order:false",
" └─Projection 1.00 root test.t1.c_datetime, test.t1.c1, test.t1._tidb_rowid, test.t1._tidb_tid",
" └─IndexMerge 1.00 root type: union",
" ├─IndexRangeScan(Build) 1.00 cop[tikv] table:t1, partition:p3, index:c1(c1) range:[-inf,10), keep order:false",
" ├─IndexRangeScan(Build) 1.00 cop[tikv] table:t1, partition:p3, index:c2(c2) range:[-inf,10), keep order:false",
" └─TableRowIDScan(Probe) 1.00 cop[tikv] table:t1, partition:p3 keep order:false",
))
tk.MustQuery(`select /*+ use_index_merge(t1) */ c1 from t1 join t2
on t1.c_datetime >= t2.c_datetime
Expand Down Expand Up @@ -710,20 +714,21 @@ func TestIntersectionWithDifferentConcurrency(t *testing.T) {
for _, concurrency := range execCon {
tk.MustExec(fmt.Sprintf("set tidb_executor_concurrency = %d", concurrency))
for i := 0; i < 2; i++ {
sql := "select /*+ use_index_merge(t1, primary, c2, c3) */ c1 from t1 where c2 < 1024 and c3 > 1024"
if i == 0 {
// Dynamic mode.
tk.MustExec("set tidb_partition_prune_mode = 'dynamic'")
res := tk.MustQuery("explain select /*+ use_index_merge(t1, primary, c2, c3) */ c1 from t1 where c2 < 1024 and c3 > 1024")
require.Contains(t, res.Rows()[1][0], "IndexMerge")
require.True(t, tk.HasPlan(sql, "IndexMerge"))
require.True(t, tk.HasNoPlan(sql, "PartitionUnion"))
} else {
tk.MustExec("set tidb_partition_prune_mode = 'static'")
res := tk.MustQuery("explain select /*+ use_index_merge(t1, primary, c2, c3) */ c1 from t1 where c2 < 1024 and c3 > 1024")
if tblIdx == 0 {
// partition table
require.Contains(t, res.Rows()[1][0], "PartitionUnion")
require.Contains(t, res.Rows()[2][0], "IndexMerge")
require.True(t, tk.HasPlan(sql, "IndexMerge"))
require.True(t, tk.HasPlan(sql, "PartitionUnion"))
} else {
require.Contains(t, res.Rows()[1][0], "IndexMerge")
require.True(t, tk.HasPlan(sql, "IndexMerge"))
require.True(t, tk.HasNoPlan(sql, "PartitionUnion"))
}
}
for i := 0; i < queryCnt; i++ {
Expand Down
2 changes: 1 addition & 1 deletion planner/core/issuetest/BUILD.bazel
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,6 @@ go_test(
srcs = ["planner_issue_test.go"],
flaky = True,
race = "on",
shard_count = 7,
shard_count = 8,
deps = ["//testkit"],
)
27 changes: 27 additions & 0 deletions planner/core/issuetest/planner_issue_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@
package issuetest

import (
"sort"
"strconv"
"testing"

"github.com/pingcap/tidb/testkit"
Expand Down Expand Up @@ -116,6 +118,31 @@ func TestIssue46083(t *testing.T) {
tk.MustExec("INSERT INTO v0 WITH ta2 AS (TABLE v0) TABLE ta2 FOR UPDATE OF ta2;")
}

func TestIssue48755(t *testing.T) {
store := testkit.CreateMockStore(t)
tk := testkit.NewTestKit(t, store)
tk.MustExec("use test")
tk.MustExec("set @@tidb_max_chunk_size=32")
tk.MustExec("create table t(a int, b int);")
tk.MustExec("insert into t values(1, 1);")
tk.MustExec("insert into t select a+1, a+1 from t;")
tk.MustExec("insert into t select a+2, a+2 from t;")
tk.MustExec("insert into t select a+4, a+4 from t;")
tk.MustExec("insert into t select a+8, a+8 from t;")
tk.MustExec("insert into t select a+16, a+16 from t;")
tk.MustExec("insert into t select a+32, a+32 from t;")
rs := tk.MustQuery("select a from (select 100 as a, 100 as b union all select * from t) t where b != 0;")
expectedResult := make([]string, 0, 65)
for i := 1; i < 65; i++ {
expectedResult = append(expectedResult, strconv.FormatInt(int64(i), 10))
}
expectedResult = append(expectedResult, "100")
sort.Slice(expectedResult, func(i, j int) bool {
return expectedResult[i] < expectedResult[j]
})
rs.Sort().Check(testkit.Rows(expectedResult...))
}

func TestIssue47881(t *testing.T) {
store := testkit.CreateMockStore(t)
tk := testkit.NewTestKit(t, store)
Expand Down
18 changes: 10 additions & 8 deletions planner/core/rule_column_pruning.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -247,6 +247,9 @@ func (p *LogicalUnionAll) PruneColumns(parentUsedCols []*expression.Column, opt
if !hasBeenUsed {
parentUsedCols = make([]*expression.Column, len(p.schema.Columns))
copy(parentUsedCols, p.schema.Columns)
for i := range used {
used[i] = true
}
}
for _, child := range p.Children() {
err := child.PruneColumns(parentUsedCols, opt)
Expand All @@ -256,16 +259,15 @@ func (p *LogicalUnionAll) PruneColumns(parentUsedCols []*expression.Column, opt
}

prunedColumns := make([]*expression.Column, 0)
for i := len(used) - 1; i >= 0; i-- {
if !used[i] {
prunedColumns = append(prunedColumns, p.schema.Columns[i])
p.schema.Columns = append(p.schema.Columns[:i], p.schema.Columns[i+1:]...)
}
}
appendColumnPruneTraceStep(p, prunedColumns, opt)
if hasBeenUsed {
// keep the schema of LogicalUnionAll same as its children's
used := expression.GetUsedList(p.children[0].Schema().Columns, p.schema)
for i := len(used) - 1; i >= 0; i-- {
if !used[i] {
prunedColumns = append(prunedColumns, p.schema.Columns[i])
p.schema.Columns = append(p.schema.Columns[:i], p.schema.Columns[i+1:]...)
}
}
appendColumnPruneTraceStep(p, prunedColumns, opt)
// It's possible that the child operator adds extra columns to the schema.
// Currently, (*LogicalAggregation).PruneColumns() might do this.
// But we don't need such columns, so we add an extra Projection to prune this column when this happened.
Expand Down
11 changes: 11 additions & 0 deletions testkit/testkit.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -236,6 +236,17 @@ func (tk *TestKit) HasPlan(sql string, plan string, args ...interface{}) bool {
return false
}

// HasNoPlan checks if the result execution plan doesn't contain specific plan.
func (tk *TestKit) HasNoPlan(sql string, plan string, args ...interface{}) bool {
rs := tk.MustQuery("explain "+sql, args...)
for i := range rs.rows {
if strings.Contains(rs.rows[i][0], plan) {
return false
}
}
return true
}

// HasTiFlashPlan checks if the result execution plan contains TiFlash plan.
func (tk *TestKit) HasTiFlashPlan(sql string, args ...interface{}) bool {
rs := tk.MustQuery("explain "+sql, args...)
Expand Down