Closed henrybw closed 2 weeks ago
Hi @henrybw. Thanks for your PR.
PRs from untrusted users cannot be marked as trusted with /ok-to-test
in this repo meaning untrusted PR authors can never trigger tests themselves. Collaborators can still trigger tests on the PR using /test all
.
I understand the commands that are listed here.
Attention: Patch coverage is 83.33333%
with 4 lines
in your changes are missing coverage. Please review.
Project coverage is 73.9689%. Comparing base (
acdb6f5
) to head (d32550b
). Report is 23 commits behind head on master.
/cc @hawkingrei
/retest
@henrybw: Cannot trigger testing until a trusted user reviews the PR and leaves an /ok-to-test
message.
[APPROVALNOTIFIER] This PR is APPROVED
This pull-request has been approved by: D3Hunter, hawkingrei, tangenta
The full list of commands accepted by this bot can be found here.
The pull request process is described here
/retest
What problem does this PR solve?
Issue Number: close #52432
Problem Summary:
The
LOCK IN SHARE MODE
clause forSELECT
statements is currently a noop, controlled bytidb_enable_noop_functions
: whentidb_enable_noop_functions=OFF
, using a noop likeLOCK IN SHARE MODE
should cause an error. However, point-get queries use a special "fast path" that accidentally bypasses this noop check, meaning that a query likeselect * from t where a=1 lock in share mode;
(wherea
is a key column) will run even whentidb_enable_noop_functions=OFF
.What changed and how does it work?
SELECT
statements (bothLOCK IN SHARE MODE
andSQL_CALC_FOUND_ROWS
) out ofbuildLogicalPlan
and intoPreprocess
, which runs before theTryFastPlan
fast path for point-get queries.noop_functions
test case tointegrationtest
, testing the noop function checks for bothLOCK IN SHARE MODE
andSQL_CALC_FOUND_ROWS
based on the existingTestNoopFunctions
"unit test" inpkg/expression/integration_test/integration_test.go
.Check List
Tests
Side effects
Documentation
Release note
Please refer to Release Notes Language Style Guide to write a quality release note.