-
Notifications
You must be signed in to change notification settings - Fork 2.3k
Fix SET and START TRANSACTION in create procedure statements
#18279
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
Merged
harshit-gangal
merged 1 commit into
vitessio:main
from
planetscale:fix-stored-procedure-normalization
May 22, 2025
Merged
Fix SET and START TRANSACTION in create procedure statements
#18279
harshit-gangal
merged 1 commit into
vitessio:main
from
planetscale:fix-stored-procedure-normalization
May 22, 2025
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…atements Signed-off-by: Manan Gupta <[email protected]>
Contributor
Review ChecklistHello reviewers! 👋 Please follow this checklist when reviewing this Pull Request. General
Tests
Documentation
New flags
If a workflow is added or modified:
Backward compatibility
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #18279 +/- ##
==========================================
- Coverage 67.46% 67.46% -0.01%
==========================================
Files 1602 1602
Lines 262245 262245
==========================================
- Hits 176930 176926 -4
- Misses 85315 85319 +4 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
systay
approved these changes
May 21, 2025
harshit-gangal
approved these changes
May 22, 2025
vitess-bot
pushed a commit
that referenced
this pull request
May 22, 2025
) Signed-off-by: Manan Gupta <[email protected]>
GuptaManan100
pushed a commit
that referenced
this pull request
May 23, 2025
…statements (#18279) (#18293) Signed-off-by: Manan Gupta <[email protected]> Co-authored-by: vitess-bot[bot] <108069721+vitess-bot[bot]@users.noreply.github.com>
timvaillancourt
added a commit
to slackhq/vitess
that referenced
this pull request
Jun 18, 2025
* [release-22.0] Bump to `v22.0.1-SNAPSHOT` after the `v22.0.0` release (vitessio#18225) Signed-off-by: Florent Poinsard <[email protected]> Co-authored-by: Florent Poinsard <[email protected]> * [release-22.0] fix: Preserve multi-column TupleExpr in tuple simplifier (vitessio#18216) (vitessio#18220) Signed-off-by: Harshit Gangal <[email protected]> Co-authored-by: vitess-bot[bot] <108069721+vitess-bot[bot]@users.noreply.github.com> Co-authored-by: Harshit Gangal <[email protected]> * [release-22.0] Properly handle grpc dial errors in the throttler metric aggregation (vitessio#18073) (vitessio#18231) Signed-off-by: Arthur Schreiber <[email protected]> Signed-off-by: Mohamed Hamza <[email protected]> Co-authored-by: vitess-bot[bot] <108069721+vitess-bot[bot]@users.noreply.github.com> Co-authored-by: Mohamed Hamza <[email protected]> * [release-22.0] test: TestQueryTimeoutWithShardTargeting fix flaky test (vitessio#18242) (vitessio#18250) Signed-off-by: Harshit Gangal <[email protected]> Co-authored-by: vitess-bot[bot] <108069721+vitess-bot[bot]@users.noreply.github.com> * [release-22.0] make sure to give MEMBER OF the correct precedence (vitessio#18237) (vitessio#18245) Signed-off-by: Andres Taylor <[email protected]> Co-authored-by: Andrés Taylor <[email protected]> * [release-22.0] Fix evalengine crashes on unexpected types (vitessio#18254) (vitessio#18258) Co-authored-by: vitess-bot[bot] <108069721+vitess-bot[bot]@users.noreply.github.com> * [release-22.0] Fix subquery merging regression introduced in vitessio#11379 (vitessio#18260) (vitessio#18263) Signed-off-by: Arthur Schreiber <[email protected]> Co-authored-by: Andrés Taylor <[email protected]> * [release-22.0] json array insert test (vitessio#18284) (vitessio#18286) Signed-off-by: Harshit Gangal <[email protected]> Co-authored-by: vitess-bot[bot] <108069721+vitess-bot[bot]@users.noreply.github.com> Co-authored-by: Harshit Gangal <[email protected]> * [release-22.0] Fix `SET` and `START TRANSACTION` in create procedure statements (vitessio#18279) (vitessio#18293) Signed-off-by: Manan Gupta <[email protected]> Co-authored-by: vitess-bot[bot] <108069721+vitess-bot[bot]@users.noreply.github.com> * [release-22.0] Fix deadlock in semi-sync monitor (vitessio#18276) (vitessio#18290) Signed-off-by: Manan Gupta <[email protected]> Co-authored-by: vitess-bot[bot] <108069721+vitess-bot[bot]@users.noreply.github.com> * [release-22.0] Upgrade the Golang version to `go1.24.3` (vitessio#18239) Signed-off-by: GitHub <[email protected]> Signed-off-by: Andres Taylor <[email protected]> Co-authored-by: frouioui <[email protected]> Co-authored-by: Andres Taylor <[email protected]> * [release-22.0] Atomic Copy: Handle error that was ignored while streaming tables and log it (vitessio#18313) (vitessio#18316) Signed-off-by: Rohit Nayak <[email protected]> Co-authored-by: vitess-bot[bot] <108069721+vitess-bot[bot]@users.noreply.github.com> * [release-22.0] fix: handle dml query for None opcode (vitessio#18326) (vitessio#18345) Signed-off-by: Harshit Gangal <[email protected]> Co-authored-by: vitess-bot[bot] <108069721+vitess-bot[bot]@users.noreply.github.com> Co-authored-by: Harshit Gangal <[email protected]> * [release-22.0] fix: keep LIMIT/OFFSET even when merging UNION queries (vitessio#18361) (vitessio#18363) Signed-off-by: Andres Taylor <[email protected]> Co-authored-by: vitess-bot[bot] <108069721+vitess-bot[bot]@users.noreply.github.com> Co-authored-by: Andres Taylor <[email protected]> * [release-22.0] Fix: Deadlock in `Close` and `write` in semi-sync monitor. (vitessio#18359) (vitessio#18368) Signed-off-by: Manan Gupta <[email protected]> Co-authored-by: vitess-bot[bot] <108069721+vitess-bot[bot]@users.noreply.github.com> * [release-22.0] Upgrade the Golang version to `go1.24.4` (vitessio#18329) Signed-off-by: GitHub <[email protected]> Signed-off-by: Manan Gupta <[email protected]> Co-authored-by: frouioui <[email protected]> Co-authored-by: Manan Gupta <[email protected]> * [release-22.0] fix version issue when using --mysql-shell-speedup-restore=true (vitessio#18310) (vitessio#18356) Co-authored-by: vitess-bot[bot] <108069721+vitess-bot[bot]@users.noreply.github.com> * [release-22.0] Split workflow with flaky vdiff2 e2e test. Skip flaky Migrate test. (vitessio#18300) (vitessio#18334) Signed-off-by: Rohit Nayak <[email protected]> Co-authored-by: vitess-bot[bot] <108069721+vitess-bot[bot]@users.noreply.github.com> * [release-22.0] Throttler: keep watching topo even on error (vitessio#18223) (vitessio#18322) Signed-off-by: Shlomi Noach <[email protected]> Co-authored-by: vitess-bot[bot] <108069721+vitess-bot[bot]@users.noreply.github.com> * [release-22.0] Code Freeze for `v22.0.1` (vitessio#18374) Signed-off-by: Manan Gupta <[email protected]> * [release-22.0] Release of `v22.0.1` (vitessio#18375) Signed-off-by: Manan Gupta <[email protected]> * add private repo config to new CI file Signed-off-by: Tim Vaillancourt <[email protected]> * `make generate_ci_workflows` Signed-off-by: Tim Vaillancourt <[email protected]> --------- Signed-off-by: Florent Poinsard <[email protected]> Signed-off-by: Harshit Gangal <[email protected]> Signed-off-by: Arthur Schreiber <[email protected]> Signed-off-by: Mohamed Hamza <[email protected]> Signed-off-by: Andres Taylor <[email protected]> Signed-off-by: Manan Gupta <[email protected]> Signed-off-by: GitHub <[email protected]> Signed-off-by: Rohit Nayak <[email protected]> Signed-off-by: Shlomi Noach <[email protected]> Signed-off-by: Tim Vaillancourt <[email protected]> Co-authored-by: vitess-bot <[email protected]> Co-authored-by: Florent Poinsard <[email protected]> Co-authored-by: vitess-bot[bot] <108069721+vitess-bot[bot]@users.noreply.github.com> Co-authored-by: Harshit Gangal <[email protected]> Co-authored-by: Mohamed Hamza <[email protected]> Co-authored-by: Andrés Taylor <[email protected]> Co-authored-by: frouioui <[email protected]> Co-authored-by: Manan Gupta <[email protected]> Co-authored-by: Manan Gupta <[email protected]>
tanjinx
pushed a commit
to slackhq/vitess
that referenced
this pull request
Nov 10, 2025
* [release-22.0] Bump to `v22.0.1-SNAPSHOT` after the `v22.0.0` release (vitessio#18225) Signed-off-by: Florent Poinsard <[email protected]> Co-authored-by: Florent Poinsard <[email protected]> * [release-22.0] fix: Preserve multi-column TupleExpr in tuple simplifier (vitessio#18216) (vitessio#18220) Signed-off-by: Harshit Gangal <[email protected]> Co-authored-by: vitess-bot[bot] <108069721+vitess-bot[bot]@users.noreply.github.com> Co-authored-by: Harshit Gangal <[email protected]> * [release-22.0] Properly handle grpc dial errors in the throttler metric aggregation (vitessio#18073) (vitessio#18231) Signed-off-by: Arthur Schreiber <[email protected]> Signed-off-by: Mohamed Hamza <[email protected]> Co-authored-by: vitess-bot[bot] <108069721+vitess-bot[bot]@users.noreply.github.com> Co-authored-by: Mohamed Hamza <[email protected]> * [release-22.0] test: TestQueryTimeoutWithShardTargeting fix flaky test (vitessio#18242) (vitessio#18250) Signed-off-by: Harshit Gangal <[email protected]> Co-authored-by: vitess-bot[bot] <108069721+vitess-bot[bot]@users.noreply.github.com> * [release-22.0] make sure to give MEMBER OF the correct precedence (vitessio#18237) (vitessio#18245) Signed-off-by: Andres Taylor <[email protected]> Co-authored-by: Andrés Taylor <[email protected]> * [release-22.0] Fix evalengine crashes on unexpected types (vitessio#18254) (vitessio#18258) Co-authored-by: vitess-bot[bot] <108069721+vitess-bot[bot]@users.noreply.github.com> * [release-22.0] Fix subquery merging regression introduced in vitessio#11379 (vitessio#18260) (vitessio#18263) Signed-off-by: Arthur Schreiber <[email protected]> Co-authored-by: Andrés Taylor <[email protected]> * [release-22.0] json array insert test (vitessio#18284) (vitessio#18286) Signed-off-by: Harshit Gangal <[email protected]> Co-authored-by: vitess-bot[bot] <108069721+vitess-bot[bot]@users.noreply.github.com> Co-authored-by: Harshit Gangal <[email protected]> * [release-22.0] Fix `SET` and `START TRANSACTION` in create procedure statements (vitessio#18279) (vitessio#18293) Signed-off-by: Manan Gupta <[email protected]> Co-authored-by: vitess-bot[bot] <108069721+vitess-bot[bot]@users.noreply.github.com> * [release-22.0] Fix deadlock in semi-sync monitor (vitessio#18276) (vitessio#18290) Signed-off-by: Manan Gupta <[email protected]> Co-authored-by: vitess-bot[bot] <108069721+vitess-bot[bot]@users.noreply.github.com> * [release-22.0] Upgrade the Golang version to `go1.24.3` (vitessio#18239) Signed-off-by: GitHub <[email protected]> Signed-off-by: Andres Taylor <[email protected]> Co-authored-by: frouioui <[email protected]> Co-authored-by: Andres Taylor <[email protected]> * [release-22.0] Atomic Copy: Handle error that was ignored while streaming tables and log it (vitessio#18313) (vitessio#18316) Signed-off-by: Rohit Nayak <[email protected]> Co-authored-by: vitess-bot[bot] <108069721+vitess-bot[bot]@users.noreply.github.com> * [release-22.0] fix: handle dml query for None opcode (vitessio#18326) (vitessio#18345) Signed-off-by: Harshit Gangal <[email protected]> Co-authored-by: vitess-bot[bot] <108069721+vitess-bot[bot]@users.noreply.github.com> Co-authored-by: Harshit Gangal <[email protected]> * [release-22.0] fix: keep LIMIT/OFFSET even when merging UNION queries (vitessio#18361) (vitessio#18363) Signed-off-by: Andres Taylor <[email protected]> Co-authored-by: vitess-bot[bot] <108069721+vitess-bot[bot]@users.noreply.github.com> Co-authored-by: Andres Taylor <[email protected]> * [release-22.0] Fix: Deadlock in `Close` and `write` in semi-sync monitor. (vitessio#18359) (vitessio#18368) Signed-off-by: Manan Gupta <[email protected]> Co-authored-by: vitess-bot[bot] <108069721+vitess-bot[bot]@users.noreply.github.com> * [release-22.0] Upgrade the Golang version to `go1.24.4` (vitessio#18329) Signed-off-by: GitHub <[email protected]> Signed-off-by: Manan Gupta <[email protected]> Co-authored-by: frouioui <[email protected]> Co-authored-by: Manan Gupta <[email protected]> * [release-22.0] fix version issue when using --mysql-shell-speedup-restore=true (vitessio#18310) (vitessio#18356) Co-authored-by: vitess-bot[bot] <108069721+vitess-bot[bot]@users.noreply.github.com> * [release-22.0] Split workflow with flaky vdiff2 e2e test. Skip flaky Migrate test. (vitessio#18300) (vitessio#18334) Signed-off-by: Rohit Nayak <[email protected]> Co-authored-by: vitess-bot[bot] <108069721+vitess-bot[bot]@users.noreply.github.com> * [release-22.0] Throttler: keep watching topo even on error (vitessio#18223) (vitessio#18322) Signed-off-by: Shlomi Noach <[email protected]> Co-authored-by: vitess-bot[bot] <108069721+vitess-bot[bot]@users.noreply.github.com> * [release-22.0] Code Freeze for `v22.0.1` (vitessio#18374) Signed-off-by: Manan Gupta <[email protected]> * [release-22.0] Release of `v22.0.1` (vitessio#18375) Signed-off-by: Manan Gupta <[email protected]> * add private repo config to new CI file Signed-off-by: Tim Vaillancourt <[email protected]> * `make generate_ci_workflows` Signed-off-by: Tim Vaillancourt <[email protected]> --------- Signed-off-by: Florent Poinsard <[email protected]> Signed-off-by: Harshit Gangal <[email protected]> Signed-off-by: Arthur Schreiber <[email protected]> Signed-off-by: Mohamed Hamza <[email protected]> Signed-off-by: Andres Taylor <[email protected]> Signed-off-by: Manan Gupta <[email protected]> Signed-off-by: GitHub <[email protected]> Signed-off-by: Rohit Nayak <[email protected]> Signed-off-by: Shlomi Noach <[email protected]> Signed-off-by: Tim Vaillancourt <[email protected]> Co-authored-by: vitess-bot <[email protected]> Co-authored-by: Florent Poinsard <[email protected]> Co-authored-by: vitess-bot[bot] <108069721+vitess-bot[bot]@users.noreply.github.com> Co-authored-by: Harshit Gangal <[email protected]> Co-authored-by: Mohamed Hamza <[email protected]> Co-authored-by: Andrés Taylor <[email protected]> Co-authored-by: frouioui <[email protected]> Co-authored-by: Manan Gupta <[email protected]> Co-authored-by: Manan Gupta <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
As stated in #18278, when we added support for create procedures in #17946, we made a couple of mistakes -
SET y = 4statements get converted toSET @@y = 4which is incorrect. This was happening in the normalisation step of the planner. This has been fixed in this PR by skipping the entire normalisation step for create procedure statements.START TRANSACTIONstatements get converted toBEGINwhich eventually throw a parsing error. In a stored procedure aBEGINstatement is considered part of aBEGIN...ENDstatement, so we need to preserveSTART TRANSACTIONstatements as is, and not change them toBEGIN. This is done by storing a new field in the AST to preserve the original type of the statement.Tests for both of these have been added to the planner as well as e2e tested that we are indeed able to create the procedures with these statements.
Related Issue(s)
SETandSTART TRANSACTIONstatements inCREATE PROCEDUREdon't work as intended #18278Checklist
Deployment Notes