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.

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: support push part of order property down to the partition table #36108

Merged
merged 31 commits into from
Nov 29, 2022
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
7c9e327
planner: support push part of order property down to the partition table
winoros Jul 7, 2022
3993e00
planner: support push part of order property to partition table
winoros Jul 11, 2022
3106166
fix fmt
winoros Jul 11, 2022
2c08dde
add tests
winoros Jul 13, 2022
39bca97
Merge branch 'master' into part-table-topn
hawkingrei Jul 16, 2022
896a4e0
Merge branch 'master' into part-table-topn
winoros Jul 18, 2022
9f57b9c
address comments
winoros Jul 18, 2022
6ccdc30
Merge remote-tracking branch 'yiding/part-table-topn' into part-table…
winoros Jul 19, 2022
423f599
add tests
winoros Jul 19, 2022
f940c71
make sure that the request is sent first by partition then by region
winoros Jul 20, 2022
2bc49ff
fix make check
winoros Jul 20, 2022
af0cf54
Merge branch 'master' into part-table-topn
winoros Aug 26, 2022
0575b4c
fix lint
winoros Aug 26, 2022
5006949
Merge branch 'master' into part-table-topn
winoros Aug 26, 2022
027eca6
fix tests
winoros Aug 28, 2022
4b00f0a
Merge remote-tracking branch 'yiding/part-table-topn' into part-table…
winoros Aug 28, 2022
174254c
Merge branch 'master' into part-table-topn
winoros Aug 28, 2022
27324cb
fix test
winoros Aug 28, 2022
bdf9854
Merge branch 'master' into part-table-topn
winoros Oct 28, 2022
51853c7
fix
winoros Oct 28, 2022
e9da563
Merge branch 'master' into part-table-topn
winoros Nov 23, 2022
921e702
wrap the ranges
winoros Nov 23, 2022
7a60b86
rename and cleanup
winoros Nov 23, 2022
09fb930
pass static check
winoros Nov 26, 2022
e1b4211
fix tiny error
winoros Nov 26, 2022
e285406
Merge branch 'master' into part-table-topn
winoros Nov 26, 2022
953273f
address comments
winoros Nov 28, 2022
6b0699f
Merge branch 'master' into part-table-topn
ti-chi-bot Nov 29, 2022
2794d3e
Merge branch 'master' into part-table-topn
ti-chi-bot Nov 29, 2022
1d38fcc
Merge branch 'master' into part-table-topn
ti-chi-bot Nov 29, 2022
743265c
Merge branch 'master' into part-table-topn
ti-chi-bot Nov 29, 2022
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
3 changes: 0 additions & 3 deletions executor/distsql.go
Original file line number Diff line number Diff line change
Expand Up @@ -446,9 +446,6 @@ func (e *IndexLookUpExecutor) Open(ctx context.Context) error {
func (e *IndexLookUpExecutor) buildTableKeyRanges() (err error) {
sc := e.ctx.GetSessionVars().StmtCtx
if e.partitionTableMode {
if e.keepOrder { // this case should be prevented by the optimizer
return errors.New("invalid execution plan: cannot keep order when accessing a partition table by IndexLookUpReader")
}
e.feedback.Invalidate() // feedback for partition tables is not ready
e.partitionKVRanges = make([][]kv.KeyRange, 0, len(e.prunedPartitions))
for _, p := range e.prunedPartitions {
Expand Down
1 change: 1 addition & 0 deletions planner/core/find_best_task.go
Original file line number Diff line number Diff line change
Expand Up @@ -2281,6 +2281,7 @@ func (ds *DataSource) getOriginalPhysicalIndexScan(prop *property.PhysicalProper
physicalTableID: ds.physicalTableID,
tblColHists: ds.TblColHists,
pkIsHandleCol: ds.getPKIsHandleCol(),
constColsByCond: path.ConstCols,
}.Init(ds.ctx, ds.blockOffset)
statsTbl := ds.statisticTable
if statsTbl.Indices[idx.ID] != nil {
Expand Down
4 changes: 4 additions & 0 deletions planner/core/physical_plans.go
Original file line number Diff line number Diff line change
Expand Up @@ -442,6 +442,10 @@ type PhysicalIndexScan struct {
// tblColHists contains all columns before pruning, which are used to calculate row-size
tblColHists *statistics.HistColl
pkIsHandleCol *expression.Column

// constColsByCond records the constant part of the index columns caused by the access conds.
// e.g. the index is (a, b, c) and there's filter a = 1 and b = 2, then the column a and b are const part.
constColsByCond []bool
}

// Clone implements PhysicalPlan interface.
Expand Down
156 changes: 156 additions & 0 deletions planner/core/task.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import (
"github.com/pingcap/tidb/planner/util"
"github.com/pingcap/tidb/sessionctx"
"github.com/pingcap/tidb/statistics"
"github.com/pingcap/tidb/table/tables"
"github.com/pingcap/tidb/types"
"github.com/pingcap/tidb/util/chunk"
"github.com/pingcap/tidb/util/collate"
Expand Down Expand Up @@ -1010,6 +1011,10 @@ func (p *PhysicalTopN) attach2Task(tasks ...task) task {
}
needPushDown := len(cols) > 0
if copTask, ok := t.(*copTask); ok && needPushDown && p.canPushDown(copTask.getStoreType()) && len(copTask.rootTaskConds) == 0 {
newTask, changed := p.pushTopNDownToDynamicPartition(copTask)
if changed {
return newTask
}
// If all columns in topN are from index plan, we push it to index plan, otherwise we finish the index plan and
// push it to table plan.
var pushedDownTopN *PhysicalTopN
Expand All @@ -1034,6 +1039,157 @@ func (p *PhysicalTopN) attach2Task(tasks ...task) task {
return attachPlan2Task(p, rootTask)
}

// pushTopNDownToDynamicPartition is a temp solution for partition table. It actually does the same thing as DataSource's isMatchProp.
func (p *PhysicalTopN) pushTopNDownToDynamicPartition(copTsk *copTask) (task, bool) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we need more detailed comments since it's a tricky way to modify the plan.
I think we should clarify what the resulting plan would be like and why we have to use this method as a temporary solution.

var err error
copTsk = copTsk.copy().(*copTask)
if err != nil {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the err isn't assigned anything before

return nil, false
}
if len(copTsk.rootTaskConds) > 0 {
return nil, false
}
colsProp, ok := GetPropByOrderByItems(p.ByItems)
if !ok {
return nil, false
}
allSameOrder, isDesc := colsProp.AllSameOrder()
if !allSameOrder {
return nil, false
}
checkIndexMatchProp := func(idxCols []*expression.Column, idxColLens []int, constColsByCond []bool, colsProp *property.PhysicalProperty) bool {
// If the number of the by-items is bigger than the index columns. We cannot push down since it must not keep order.
if len(idxCols) < len(colsProp.SortItems) {
return false
}
idxPos := 0
for _, byItem := range colsProp.SortItems {
found := false
for ; idxPos < len(idxCols); idxPos++ {
if idxColLens[idxPos] == types.UnspecifiedLength && idxCols[idxPos].Equal(p.SCtx(), byItem.Col) {
found = true
idxPos++
break
}
if len(constColsByCond) == 0 || idxPos > len(constColsByCond) || !constColsByCond[idxPos] {
found = false
break
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

missed set found = true at the end? (indicating that we are sure that this col is constant, then just go to outer's continued column)

In:
order by (a,b,c), once index(a,c) exists and b=1, we can still access this path right?
reader -> limit -> selection -> indexScan

order by (a,c), once index(a,b,c) exists and b=1 is what actually it already does.

}
if !found {
return false
}
}
return true
}
var (
idxScan *PhysicalIndexScan
tblScan *PhysicalTableScan
)
if copTsk.indexPlan != nil {
copTsk.indexPlan, err = copTsk.indexPlan.Clone()
if err != nil {
return nil, false
}
finalIdxScanPlan := copTsk.indexPlan
for len(finalIdxScanPlan.Children()) > 0 && finalIdxScanPlan.Children()[0] != nil {
finalIdxScanPlan = finalIdxScanPlan.Children()[0]
}
idxScan = finalIdxScanPlan.(*PhysicalIndexScan)
}
if copTsk.tablePlan != nil {
copTsk.tablePlan, err = copTsk.tablePlan.Clone()
if err != nil {
return nil, false
}
finalTblScanPlan := copTsk.tablePlan
if len(finalTblScanPlan.Children()) > 0 {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/ if / for?

finalTblScanPlan = finalTblScanPlan.Children()[0]
}
tblScan = finalTblScanPlan.(*PhysicalTableScan)
}
if !copTsk.indexPlanFinished {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We didn't consider that idxScan might be nil in the logic below.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When indexPlanFinished is not true, there should be a index scan to be finished later.

// If indexPlan side isn't finished, there's no selection on the table side.

if idxScan.Table.GetPartitionInfo() == nil {
return nil, false
}

propMatched := checkIndexMatchProp(idxScan.IdxCols, idxScan.IdxColLens, idxScan.constColsByCond, colsProp)
logutil.BgLogger().Warn("push down top-n", zap.Bool("prop is matched", propMatched))
if !propMatched {
return nil, false
}

// If we reach here, the index matches the order property of the top-n, we could push a limit down.
copTsk.keepOrder = true
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems like we don't need to set copTask.keepOrder to true.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for partitions maybe

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

func buildIndexLookUpTask(ctx sessionctx.Context, t *copTask) *rootTask {
	newTask := &rootTask{cst: t.cst}
	p := PhysicalIndexLookUpReader{
		tablePlan:        t.tablePlan,
		indexPlan:        t.indexPlan,
		ExtraHandleCol:   t.extraHandleCol,
		CommonHandleCols: t.commonHandleCols,
		expectedCnt:      t.expectCnt,
		keepOrder:        t.keepOrder,
	}.Init(ctx, t.tablePlan.SelectBlockOffset())

This func uses it.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh actually we don't need the keep order, remove it.

idxScan.KeepOrder = true
idxScan.Desc = isDesc
childProfile := copTsk.plan().statsInfo()
newCount := p.Offset + p.Count
stats := deriveLimitStats(childProfile, float64(newCount))
pushedLimit := PhysicalLimit{
Count: newCount,
}.Init(p.SCtx(), stats, p.SelectBlockOffset())
pushedLimit.SetSchema(copTsk.indexPlan.Schema())
copTsk = attachPlan2Task(pushedLimit, copTsk).(*copTask)
pushedLimit.cost = copTsk.cost()
if copTsk.tablePlan != nil && !idxScan.Table.IsCommonHandle && copTsk.extraHandleCol == nil {
// The clustered index would always add handle, so we don't need to consider that case.
copTsk.extraHandleCol = &expression.Column{
RetType: types.NewFieldType(mysql.TypeLonglong),
ID: model.ExtraHandleID,
UniqueID: idxScan.ctx.GetSessionVars().AllocPlanColumnID(),
}
tblScan.Schema().Append(copTsk.extraHandleCol)
tblScan.Columns = append(tblScan.Columns, model.NewExtraHandleColInfo())
copTsk.needExtraProj = true
copTsk.originSchema = idxScan.dataSourceSchema
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We missed the case that the int primary key is not _tidb_rowid but a column specified by the user.
I think we can modify the (*PhysicalTableScan).appendExtraHandleCol a bit and reuse that.

} else if copTsk.indexPlan == nil {

if tblScan.Table.GetPartitionInfo() == nil {
return nil, false
}

if tblScan.HandleCols == nil {
return nil, false
}

if tblScan.HandleCols.IsInt() {
pk := tblScan.HandleCols.GetCol(0)
if len(colsProp.SortItems) != 1 || !colsProp.SortItems[0].Col.Equal(p.SCtx(), pk) {
Comment on lines +1090 to +1092
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is different from (*DataSource).isMatchProp. According to isMatchProp, tiflash doesn't support desc scan.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change only affects tikv part.

return nil, false
}
} else {
idxCols, idxColLens := expression.IndexInfo2PrefixCols(tblScan.Columns, tblScan.Schema().Columns, tables.FindPrimaryIndex(tblScan.Table))
matched := checkIndexMatchProp(idxCols, idxColLens, nil, colsProp)
if !matched {
return nil, false
}
}
copTsk.keepOrder = true
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto.

tblScan.KeepOrder = true
tblScan.Desc = isDesc
childProfile := copTsk.plan().statsInfo()
newCount := p.Offset + p.Count
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we consider there's a selection on the table side? the count here may won't pass the selection

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In such case, the plan should be Limit->Selection->TableScan. The selection should not matter

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

get!make sense

stats := deriveLimitStats(childProfile, float64(newCount))
pushedLimit := PhysicalLimit{
Count: newCount,
}.Init(p.SCtx(), stats, p.SelectBlockOffset())
pushedLimit.SetSchema(copTsk.tablePlan.Schema())
copTsk = attachPlan2Task(pushedLimit, copTsk).(*copTask)
pushedLimit.cost = copTsk.cost()
} else {
return nil, false
}

rootTask := copTsk.convertToRootTask(p.ctx)
rootTask.addCost(p.GetCost(rootTask.count(), true))
p.cost = rootTask.cost()
return attachPlan2Task(p, rootTask), true
}

func (p *PhysicalProjection) attach2Task(tasks ...task) task {
t := tasks[0].copy()
if cop, ok := t.(*copTask); ok {
Expand Down