txn: support multi-table join in nt-dml #39139
Conversation
|
[REVIEW NOTIFICATION] This pull request has been approved by:
To complete the pull request process, please ask the reviewers in the list to review by filling The full list of commands accepted by this bot can be found here. Reviewer can indicate their review by submitting an approval review. |
Signed-off-by: ekexium <eke@fastmail.com>
| @@ -197,11 +205,11 @@ func checkConstraint(stmt *ast.NonTransactionalDMLStmt, se Session) error { | |||
| return nil | |||
| } | |||
|
|
|||
| func checkTableRef(t *ast.TableRefsClause) error { | |||
| func checkTableRef(t *ast.TableRefsClause, allowMultipleTables bool) error { | |||
There was a problem hiding this comment.
I see allowMultipleTables is always true now, is this check need to fail with multi tables in some future conditions?
There was a problem hiding this comment.
Maybe. I'm not sure about Updates and Deletes. If we find them error-prone in tests we could disable multiple table joins for them.
Signed-off-by: ekexium <eke@fastmail.com>
Checking these constraints is kinda complex. I'll leave them to a following PR. |
| // if the clustered index contains multiple columns, we cannot automatically choose a column as the shard column | ||
| return false, nil, errors.New("Non-transactional DML, the clustered index contains multiple columns. Please specify a shard column") |
There was a problem hiding this comment.
Should this be a TODO? Multi-column clustered index seems doable.
There was a problem hiding this comment.
Do you mean by selecting the first column? That column might be highly skewed. So I'd rather let user manually specify it.
Is there any test for selecting sharding key automatically with clustered index?
It's in TestNonTransactionalWithAutoDetectShardColumn
There was a problem hiding this comment.
Single column can be highly skewed too. I don't get why multi-column is special.
I thought you didn't do it just because of implementation...
There was a problem hiding this comment.
Ah, actually I don't mean using first column only. To support multiple columns, we need to scan batches of keys containing all columns in the clustered index.
|
/run-all-tests |
Signed-off-by: ekexium <eke@fastmail.com>
Signed-off-by: ekexium <eke@fastmail.com>
|
@sticnarf Could you help merge it? |
|
/merge |
|
This pull request has been accepted and is ready to merge. Commit hash: 8340eb0
|
TiDB MergeCI notify🔴 Bad News! [4] CI still failing after this pr merged.
|
Signed-off-by: ekexium eke@fastmail.com
What problem does this PR solve?
Issue Number: ref #33485
Problem Summary:
This PR adds support for multi-join in nontransactional DML.
The remaining problems
What is changed and how it works?
Check List
Tests
Side effects
Documentation
Release note
Please refer to Release Notes Language Style Guide to write a quality release note.