onlineddl: `ALTER VITESS_MIGRATION CANCEL CONTEXT 'ctx'` by shlomi-noach · Pull Request #20146 · vitessio/vitess

shlomi-noach · 2026-05-20T07:42:55Z

Closes #20145

Adds ALTER VITESS_MIGRATION CANCEL CONTEXT 'migration_context' — a new SQL statement that cancels all pending migrations belonging to a specific migration context, leaving migrations in other contexts untouched.

Follow-up to #20027 and #19995.

How it works

CONTEXT is added as a non-reserved keyword so it doesn't break existing SQL using context as an identifier.
The parser produces a CancelAllMigrationType AlterMigration node with a new Context string field.
CancelPendingMigrations on the executor gains a migrationContext parameter; when non-empty it filters the pending list before cancelling.
rows_affected in the response equals the number of migrations actually cancelled (leveraging AppendResult semantics already used by CANCEL ALL).

Changes

go/vt/sqlparser/ — new CONTEXT keyword, grammar rule, AST field, format, equality, parse test
go/vt/vttablet/onlineddl/executor.go — CancelPendingMigrations accepts migrationContext
go/vt/vttablet/tabletserver/query_executor.go — passes alterMigration.Context through
go/test/endtoend/onlineddl/ — CheckCancelContextMigrations helper + e2e test in scheduler suite

Example

ALTER VITESS_MIGRATION CANCEL CONTEXT 'my-context-123';

Introduces `ALTER VITESS_MIGRATION CANCEL CONTEXT 'ctx'` syntax to cancel all pending migrations belonging to a given migration context, without affecting migrations from other contexts. Complements the existing CANCEL ALL. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

The test was accidentally placed inside testSingleton where t1uuid, t2uuid, trivialAlterT1Statement, and ddlStrategy are out of scope. Moved it to the end of testScheduler where those variables are declared. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

vitess-bot · 2026-05-20T07:43:26Z

promptless · 2026-05-20T07:46:46Z

Promptless prepared a documentation update related to this change.

Triggered by vitessio/vitess#20146

Added documentation for the new ALTER VITESS_MIGRATION CANCEL CONTEXT 'context' SQL command in the Online DDL audit-and-control guide. The new section explains how users can cancel all pending migrations belonging to a specific migration context, which is useful when multiple deployment pipelines submit migrations concurrently.

Review: Document ALTER VITESS_MIGRATION CANCEL CONTEXT command

Copilot

Pull request overview

Adds support for cancelling Online DDL migrations by migration_context via a new SQL statement: ALTER VITESS_MIGRATION CANCEL CONTEXT '…', wiring it through the sqlparser AST and the onlineddl executor, and adding end-to-end coverage.

Changes:

Extend sqlparser (keyword/grammar/AST/format/equality/tests) to parse and print ALTER VITESS_MIGRATION CANCEL CONTEXT 'migration_context'.
Update OnlineDDL executor cancellation path to optionally filter pending migrations by context and return rows_affected for actual cancellations.
Add e2e helper + scheduler test coverage for cancel-by-context behavior.

Reviewed changes

Copilot reviewed 9 out of 13 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
go/vt/vttablet/tabletserver/query_executor.go	Passes parsed migration context through to the OnlineDDL executor cancellation method.
go/vt/vttablet/onlineddl/executor.go	Adds context-filtering behavior to cancelling pending migrations.
go/vt/sqlparser/sql.y	Adds `CONTEXT` token/grammar rule for `ALTER VITESS_MIGRATION CANCEL CONTEXT`.
go/vt/sqlparser/parse_test.go	Adds parser coverage for the new statement.
go/vt/sqlparser/keywords.go	Registers `context` as a keyword token.
go/vt/sqlparser/cached_size.go	Updates cached-size accounting for new AST field.
go/vt/sqlparser/ast.go	Adds `Context` field to `AlterMigration` AST node.
go/vt/sqlparser/ast_format.go	Prints `cancel context '…'` when context is present.
go/vt/sqlparser/ast_format_fast.go	Fast formatter support for the new context clause.
go/vt/sqlparser/ast_equals.go	Includes `Context` in AST equality comparisons.
go/test/endtoend/onlineddl/vtgate_util.go	Adds helper to run cancel-by-context statement and assert affected rows.
go/test/endtoend/onlineddl/scheduler/onlineddl_scheduler_test.go	Adds scheduler e2e test ensuring only matching-context migrations are cancelled.

Files not reviewed (3)

go/vt/sqlparser/ast_equals.go: Language not supported
go/vt/sqlparser/ast_format_fast.go: Language not supported
go/vt/sqlparser/cached_size.go: Language not supported

codecov · 2026-05-20T08:20:48Z

Codecov Report

❌ Patch coverage is 38.46154% with 16 lines in your changes missing coverage. Please review.
✅ Project coverage is 75.95%. Comparing base (70c7a72) to head (3b06ef4).
⚠️ Report is 266 commits behind head on main.

Files with missing lines	Patch %	Lines
go/vt/sqlparser/ast_format.go	11.11%	8 Missing ⚠️
go/vt/vttablet/onlineddl/executor.go	50.00%	8 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##             main   #20146       +/-   ##
===========================================
+ Coverage   69.67%   75.95%    +6.28%     
===========================================
  Files        1614      589     -1025     
  Lines      216793   110233   -106560     
===========================================
- Hits       151044    83726    -67318     
+ Misses      65749    26507    -39242

Flag	Coverage Δ
partial	`75.95% <38.46%> (?)`

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Empty migrationContext is not a supported filter value; it falls through to the cancel-all path by design. The test suite confirms this. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

shlomi-noach · 2026-05-20T09:00:45Z

all of CoPilot's current comments are about the single same concern: the fall back of empty context into CANCEL ALL. This nehavior is intentional, now documented, and also exposed in the parser tests. This is fine. Not the place to be nitty-griitty.

CANCEL CONTEXT '' would be indistinguishable from CANCEL ALL

This is intentional and documented. migrationContext == "" is the sentinel for "cancel all" — an empty context string is not a supported filter value. We've added a doc comment to CancelPendingMigrations making this explicit, and the test suite includes a case that confirms CANCEL CONTEXT '' behaves identically to CANCEL ALL.

Introducing a distinct CancelContextMigrationType to handle the empty-string edge case would add complexity (new constant, new grammar rule, new executor branch, new formatter branch) to protect against a usage pattern we consider a user error. The test-as-documentation approach gives us the same safety net without the overhead.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

Copilot

Pull request overview

Copilot reviewed 9 out of 13 changed files in this pull request and generated 2 comments.

Files not reviewed (3)

go/vt/sqlparser/ast_equals.go: Language not supported
go/vt/sqlparser/ast_format_fast.go: Language not supported
go/vt/sqlparser/cached_size.go: Language not supported

Threshold, Expire, Context, and Shards were written raw between single quotes, so values containing a single quote would produce invalid SQL. Use encodeSQLString (backed by sqltypes.EncodeStringSQL) for all four fields, matching the pattern used for other string literals in the formatter. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

CANCEL CONTEXT '' previously silently fell through to cancel-all behavior. Reject it at parse time via yylex.Error so clients that accidentally pass an empty context (e.g. an unset variable) get a clear error instead of unexpectedly affecting all migrations. Updated the parse test to expect a parse error, and simplified the CancelPendingMigrations doc comment now that the empty-context edge case is unreachable via the parser. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

shlomi-noach · 2026-05-20T11:14:17Z

OK OK copilot was right and I give up. Empty context is now rejected by the parser. I like this idea better than my original approach.

Copilot

Pull request overview

Copilot reviewed 9 out of 13 changed files in this pull request and generated no new comments.

Files not reviewed (3)

go/vt/sqlparser/ast_equals.go: Language not supported
go/vt/sqlparser/ast_format_fast.go: Language not supported
go/vt/sqlparser/cached_size.go: Language not supported

…ect error string yylex.Error alone does not abort parsing in goyacc; return 1 is required to make yyParse return non-zero. Also update the invalidSQL test entry to match the actual error format which includes position info. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

mhamza15 · 2026-05-20T13:57:07Z

 	buf.astPrintf(node, " %#s", alterType)
 	if node.Threshold != "" {
-		buf.astPrintf(node, " '%#s'", node.Threshold)
+		buf.astPrintf(node, " %s", encodeSQLString(node.Threshold))


Should these remain as %#s to maintain casing?

In my understanding the # directive is only applicable to keyword/identifier nodes. It will be completely harmless to change to %#s and I'm happy to do that -- at the same time it will be ineffective and possibly misleading since there is no keyword/identifier involved. I have absolutely zero preferences. What would make more sense in your opinion?

It does seem to apply to literals as well, here's an example unit test: 300c07d

❯ go test ./go/vt/sqlparser -run TestCanonicalOutput/alter_vitess_migration_cancel_context -count 1 --- FAIL: TestCanonicalOutput (0.00s) --- FAIL: TestCanonicalOutput/alter_vitess_migration_cancel_context_'ProdCtx' (0.00s) tracked_buffer_test.go:292: Error Trace: /Users/mhamza/.local/share/forest/worktrees/vitess/planetscale-onlineddl-cancel-migration-context/go/vt/sqlparser/tracked_buffer_test.go:292 Error: Not equal: expected: "ALTER VITESS_MIGRATION cancel CONTEXT 'ProdCtx'" actual : "ALTER VITESS_MIGRATION cancel CONTEXT 'PRODCTX'" Diff: --- Expected +++ Actual @@ -1 +1 @@ -ALTER VITESS_MIGRATION cancel CONTEXT 'ProdCtx' +ALTER VITESS_MIGRATION cancel CONTEXT 'PRODCTX' Test: TestCanonicalOutput/alter_vitess_migration_cancel_context_'ProdCtx' Messages: bad serialization FAIL FAIL vitess.io/vitess/go/vt/sqlparser 0.394s FAIL

Done and pushed! Interestingly ast_format_fast.go was unaffected by the change.

…g fields Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

Copilot

Pull request overview

Copilot reviewed 9 out of 13 changed files in this pull request and generated no new comments.

Files not reviewed (3)

go/vt/sqlparser/ast_equals.go: Language not supported
go/vt/sqlparser/ast_format_fast.go: Language not supported
go/vt/sqlparser/cached_size.go: Language not supported

mhamza15

LGTM!

mattlord

LGTM. Thanks, @shlomi-noach ! ❤️

shlomi-noach and others added 2 commits May 20, 2026 10:30

Copilot AI review requested due to automatic review settings May 20, 2026 07:42

shlomi-noach requested review from harshit-gangal, mattlord, rohit-nayak-ps, systay and timvaillancourt as code owners May 20, 2026 07:42

shlomi-noach added Type: Enhancement Logical improvement (somewhere between a bug and feature) Component: Online DDL Online DDL (vitess/native/gh-ost/pt-osc) labels May 20, 2026

github-actions Bot added this to the v25.0.0 milestone May 20, 2026

github-actions Bot added the Component: VTTablet label May 20, 2026

Copilot started reviewing on behalf of shlomi-noach May 20, 2026 07:43 View session

shlomi-noach removed NeedsDescriptionUpdate The description is not clear or comprehensive enough, and needs work NeedsIssue A linked issue is missing for this Pull Request NeedsBackportReason If backport labels have been applied to a PR, a justification is required labels May 20, 2026

Copilot AI reviewed May 20, 2026

View reviewed changes

Comment thread go/vt/sqlparser/sql.y

Comment thread go/vt/vttablet/onlineddl/executor.go

Comment thread go/vt/sqlparser/ast_format.go

Comment thread go/vt/sqlparser/ast_format_fast.go

onlineddl: add parse test confirming CANCEL CONTEXT '' == CANCEL ALL

7efe450

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

Copilot AI review requested due to automatic review settings May 20, 2026 09:01

Copilot started reviewing on behalf of shlomi-noach May 20, 2026 09:02 View session

Copilot AI reviewed May 20, 2026

View reviewed changes

Comment thread go/vt/sqlparser/ast_format_fast.go Outdated

Comment thread go/test/endtoend/onlineddl/vtgate_util.go

shlomi-noach and others added 2 commits May 20, 2026 12:22

Copilot AI review requested due to automatic review settings May 20, 2026 11:13

Copilot started reviewing on behalf of shlomi-noach May 20, 2026 11:13 View session

Copilot AI reviewed May 20, 2026

View reviewed changes

mhamza15 reviewed May 20, 2026

View reviewed changes

sqlparser: use %#s verb with encodeSQLString for AlterMigration strin…

3b06ef4

…g fields Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Shlomi Noach <2607934+shlomi-noach@users.noreply.github.com>

Copilot AI review requested due to automatic review settings May 20, 2026 15:03

Copilot started reviewing on behalf of shlomi-noach May 20, 2026 15:04 View session

Copilot AI reviewed May 20, 2026

View reviewed changes

mhamza15 removed NeedsWebsiteDocsUpdate What it says Component: VTTablet labels May 20, 2026

mhamza15 approved these changes May 20, 2026

View reviewed changes

mattlord approved these changes May 20, 2026

View reviewed changes

shlomi-noach merged commit da94cc8 into vitessio:main May 21, 2026
121 of 123 checks passed

shlomi-noach deleted the onlineddl-cancel-migration-context branch May 21, 2026 07:56

promptless Bot mentioned this pull request May 21, 2026

docs: Document ALTER VITESS_MIGRATION CANCEL CONTEXT command vitessio/website#2123

Merged

This was referenced May 21, 2026

onlineddl: context-scoped bulk migration operations #20147

Open

Feature Request: ALTER VITESS_MIGRATION … CONTEXT 'ctx': extend context-scoped control to all bulk migration operations #20177

Open

Conversation

shlomi-noach commented May 20, 2026

How it works

Changes

Example

Uh oh!

vitess-bot Bot commented May 20, 2026

Review Checklist

General

Tests

Documentation

New flags

If a workflow is added or modified:

Backward compatibility

Uh oh!

promptless Bot commented May 20, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

codecov Bot commented May 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

shlomi-noach commented May 20, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

shlomi-noach commented May 20, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

mhamza15 May 20, 2026

Choose a reason for hiding this comment

Uh oh!

shlomi-noach May 20, 2026

Choose a reason for hiding this comment

Uh oh!

mhamza15 May 20, 2026

Choose a reason for hiding this comment

Uh oh!

shlomi-noach May 20, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

mhamza15 left a comment

Choose a reason for hiding this comment

Uh oh!

mattlord left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

codecov Bot commented May 20, 2026 •

edited

Loading