executor: Support read operation for local temporary table #26353

lcwangchao · 2021-07-19T11:01:17Z

What problem does this PR solve?

Issue Number: close #25920

Problem Summary:

Support read operation for local temporary table. PointGet/BatchPointGet are already supported in previous pr #26053 and #2629. So this pr finished last read operation for table scan.

What is changed and how it works?

For local temporary table, we force to use UnionScanExec to scan table for all data are in memory. We also introduced UnionIter to merge two MemBuff iters from session and transaction into one.

Because UnionScanExec does not support index merge, so it is forbidden on local temporary table.

Check List

Tests

Unit test

Side effects

N/A

Documentation

N/A

Release note

No release note

…nto temp_table_scan_reader

tiancaiamao · 2021-07-20T06:14:21Z

executor/union_iter.go

+)
+
+// UnionIter implements kv.Iterator
+type UnionIter struct {


UnionIter is moved to the client-go repo and is now internal
https://github.com/tikv/client-go/blob/master/internal/unionstore/union_iter.go

djshow832

Rest LGTM

djshow832 · 2021-07-20T08:43:12Z

planner/core/logical_plan_builder.go

@@ -4085,7 +4085,7 @@ func (b *PlanBuilder) buildDataSource(ctx context.Context, tn *ast.TableName, as

 	var result LogicalPlan = ds
 	dirty := tableHasDirtyContent(b.ctx, tableInfo)
-	if dirty {
+	if dirty || tableInfo.TempTableType == model.TempTableLocal {


You may need to update some comments. Such as LogicalUnionScan: "LogicalUnionScan is only used in non read-only txn.".

djshow832 · 2021-07-20T08:59:58Z

planner/core/stats.go

@@ -327,7 +328,7 @@ func (ds *DataSource) DeriveStats(childStats []*property.StatsInfo, selfSchema *
 			}
 		}
 	}
-	if isPossibleIdxMerge && sessionAndStmtPermission && needConsiderIndexMerge && isReadOnlyTxn {
+	if isPossibleIdxMerge && sessionAndStmtPermission && needConsiderIndexMerge && isReadOnlyTxn && ds.tableInfo.TempTableType != model.TempTableLocal {


Why skip index merge?

UnionScan for index merge is just a TODO, so we must skip is because local temporary table always use UnionScan.

tidb/planner/core/stats.go

Lines 308 to 320 in cca097d

// TODO: implement UnionScan + IndexMerge

isReadOnlyTxn := true

txn, err := ds.ctx.Txn(false)

if err != nil {

return nil, err

}

if txn.Valid() && !txn.IsReadOnly() {

isReadOnlyTxn = false

}

// Consider the IndexMergePath. Now, we just generate `IndexMergePath` in DNF case.

isPossibleIdxMerge := len(ds.pushedDownConds) > 0 && len(ds.possibleAccessPaths) > 1

sessionAndStmtPermission := (ds.ctx.GetSessionVars().GetEnableIndexMerge() || len(ds.indexMergeHints) > 0) && !ds.ctx.GetSessionVars().StmtCtx.NoIndexMergeHint

// If there is an index path, we current do not consider `IndexMergePath`.

ti-chi-bot · 2021-07-20T09:42:25Z

[REVIEW NOTIFICATION]

This pull request has been approved by:

djshow832
tiancaiamao

To complete the pull request process, please ask the reviewers in the list to review by filling /cc @reviewer in the comment.
After your PR has acquired the required number of LGTMs, you can assign this pull request to the committer in the list by filling /assign @committer in the comment to help you merge this pull request.

The full list of commands accepted by this bot can be found here.

Reviewer can indicate their review by submitting an approval review.
Reviewer can cancel approval by submitting a request changes review.

djshow832 · 2021-07-20T09:42:34Z

/merge

ti-chi-bot · 2021-07-20T09:42:37Z

This pull request has been accepted and is ready to merge.

Commit hash: e569e42

…nto temp_table_scan_reader

lcwangchao · 2021-07-20T10:07:26Z

/run-check_dev_2

djshow832 · 2021-07-20T10:23:35Z

/merge

ti-chi-bot · 2021-07-20T10:23:38Z

This pull request has been accepted and is ready to merge.

Commit hash: 1954958

executor: Add union scan support for local temporary table

e804944

ti-chi-bot added the size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. label Jul 19, 2021

Merge branch 'master' into temp_table_scan_reader

1e77d38

lcwangchao requested review from djshow832 and tiancaiamao July 19, 2021 11:02

lcwangchao added 2 commits July 19, 2021 19:03

modify some code

4f1100f

Merge branch 'temp_table_scan_reader' of github.com:lcwangchao/tidb i…

521f5e4

…nto temp_table_scan_reader

github-actions bot added sig/execution SIG execution sig/sql-infra SIG: SQL Infra labels Jul 19, 2021

lcwangchao added 3 commits July 19, 2021 19:11

remove unused method

7c90163

Merge branch 'master' into temp_table_scan_reader

3967043

fix some code

b08f1c1

tiancaiamao approved these changes Jul 20, 2021

View reviewed changes

ti-chi-bot added the status/LGT1 Indicates that a PR has LGTM 1. label Jul 20, 2021

djshow832 reviewed Jul 20, 2021

View reviewed changes

lcwangchao added 2 commits July 20, 2021 17:35

add comments

733aa2f

Merge branch 'master' into temp_table_scan_reader

e569e42

djshow832 approved these changes Jul 20, 2021

View reviewed changes

ti-chi-bot added status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Jul 20, 2021

ti-chi-bot added the status/can-merge Indicates a PR has been approved by a committer. label Jul 20, 2021

ti-chi-bot and others added 3 commits July 20, 2021 17:42

Merge branch 'master' into temp_table_scan_reader

180c731

modify comments

a658c0c

Merge branch 'temp_table_scan_reader' of github.com:lcwangchao/tidb i…

1954958

…nto temp_table_scan_reader

ti-chi-bot removed the status/can-merge Indicates a PR has been approved by a committer. label Jul 20, 2021

lcwangchao added the status/can-merge Indicates a PR has been approved by a committer. label Jul 20, 2021

ti-chi-bot removed the status/can-merge Indicates a PR has been approved by a committer. label Jul 20, 2021

ti-chi-bot added the status/can-merge Indicates a PR has been approved by a committer. label Jul 20, 2021

Merge branch 'master' into temp_table_scan_reader

eb73e47

ti-chi-bot merged commit add2df6 into pingcap:master Jul 20, 2021

lcwangchao deleted the temp_table_scan_reader branch July 20, 2021 10:39

lcwangchao mentioned this pull request Sep 7, 2021

txn: allow transaction set custom retrievers #27847

Merged

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

executor: Support read operation for local temporary table #26353

executor: Support read operation for local temporary table #26353

lcwangchao commented Jul 19, 2021 •

edited

Loading

tiancaiamao Jul 20, 2021

djshow832 left a comment

djshow832 Jul 20, 2021

lcwangchao Jul 20, 2021

djshow832 Jul 20, 2021

lcwangchao Jul 20, 2021 •

edited

Loading

ti-chi-bot commented Jul 20, 2021

djshow832 commented Jul 20, 2021

ti-chi-bot commented Jul 20, 2021

lcwangchao commented Jul 20, 2021

djshow832 commented Jul 20, 2021

ti-chi-bot commented Jul 20, 2021

	// TODO: implement UnionScan + IndexMerge
	isReadOnlyTxn := true
	txn, err := ds.ctx.Txn(false)
	if err != nil {
	return nil, err
	}
	if txn.Valid() && !txn.IsReadOnly() {
	isReadOnlyTxn = false
	}
	// Consider the IndexMergePath. Now, we just generate `IndexMergePath` in DNF case.
	isPossibleIdxMerge := len(ds.pushedDownConds) > 0 && len(ds.possibleAccessPaths) > 1
	sessionAndStmtPermission := (ds.ctx.GetSessionVars().GetEnableIndexMerge() \|\| len(ds.indexMergeHints) > 0) && !ds.ctx.GetSessionVars().StmtCtx.NoIndexMergeHint
	// If there is an index path, we current do not consider `IndexMergePath`.

executor: Support read operation for local temporary table #26353

executor: Support read operation for local temporary table #26353

Conversation

lcwangchao commented Jul 19, 2021 • edited Loading

What problem does this PR solve?

What is changed and how it works?

Check List

Release note

tiancaiamao Jul 20, 2021

Choose a reason for hiding this comment

djshow832 left a comment

Choose a reason for hiding this comment

djshow832 Jul 20, 2021

Choose a reason for hiding this comment

lcwangchao Jul 20, 2021

Choose a reason for hiding this comment

djshow832 Jul 20, 2021

Choose a reason for hiding this comment

lcwangchao Jul 20, 2021 • edited Loading

Choose a reason for hiding this comment

ti-chi-bot commented Jul 20, 2021

djshow832 commented Jul 20, 2021

ti-chi-bot commented Jul 20, 2021

lcwangchao commented Jul 20, 2021

djshow832 commented Jul 20, 2021

ti-chi-bot commented Jul 20, 2021

lcwangchao commented Jul 19, 2021 •

edited

Loading

lcwangchao Jul 20, 2021 •

edited

Loading