planner: support the safe-update mode #20184
Conversation
|
We should adjust the doc together? |
|
|
||
| safeUpdatesErr := func(err error) { | ||
| c.Assert(err, Not(IsNil)) | ||
| c.Assert(err.Error(), Equals, "[planner:1175]You are using safe update mode and you tried to update a table without a WHERE that uses a KEY column") |
There was a problem hiding this comment.
Could we just compare with ErrUpdateWithoutKeyInSafeMode?
| @@ -759,9 +772,29 @@ func (ds *DataSource) findBestTask(prop *property.PhysicalProperty, planCounter | |||
| } | |||
| } | |||
|
|
|||
| if safeUpdateMode && numRangeScanPlans == 0 { | |||
There was a problem hiding this comment.
Should we just check the best plan instead of every plan? The range scan plan may not be chosen as the chosen one.
There was a problem hiding this comment.
If the optimizer chooses a full-scan as the best plan, but actually some other range-scans can be used, we shouldn't return the error in this case.
sessionctx/variable/tidb_vars.go
Outdated
| @@ -447,6 +447,9 @@ const ( | |||
|
|
|||
| // TiDBEnableAmendPessimisticTxn indicates if amend pessimistic transactions is enabled. | |||
| TiDBEnableAmendPessimisticTxn = "tidb_enable_amend_pessimistic_txn" | |||
|
|
|||
| // TiDBEnableSafeUpdates indicates if the safe-update mode is enabled. | |||
| TiDBEnableSafeUpdates = "tidb_enable_safe_updates" | |||
There was a problem hiding this comment.
Why don't we just use sql_safe_updates?
qw4990
force-pushed
the
safe-update-mode
branch
from
350db21 to
17acd1e
Compare
|
@qw4990: PR needs rebase. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
ti-chi-bot
added
the
needs-rebase
What problem does this PR solve?
Issue Number: close #20177
Problem Summary: support the safe-update mode
What is changed and how it works?
Please see #20177 for more details.
Check List
Tests
Release note