vtgate: route inserts on pinned sharded tables to their pinned shard#19746
vtgate: route inserts on pinned sharded tables to their pinned shard#19746officialasishkumar wants to merge 2 commits intovitessio:mainfrom
Conversation
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
|
There was a problem hiding this comment.
Pull request overview
This PR fixes INSERT routing for pinned tables in sharded keyspaces by propagating the pinned keyspace-id destination into insert planning/execution so inserts can be sent directly to the pinned shard when no vindex evaluation is needed.
Changes:
- Carry a pinned-table
TargetDestinationthrough insert planning to avoid requiring a primary vindex for pinned tables with no vindexes. - Add execution support for single-destination inserts (resolve destination → execute on exactly one shard).
- Add regression tests in both vtgate executor and engine layers.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| go/vt/vtgate/planbuilder/operator_transformers.go | Sets InsertCommon.TargetDestination for pinned tables with no column vindexes during insert plan construction. |
| go/vt/vtgate/engine/insert_common.go | Adds TargetDestination to InsertCommon and a helper to execute inserts by an explicit shard destination. |
| go/vt/vtgate/engine/insert.go | Routes sharded inserts directly by TargetDestination when present. |
| go/vt/vtgate/engine/insert_select.go | Adds a TargetDestination fast-path for sharded INSERT ... SELECT and includes destination in primitive description. |
| go/vt/vtgate/engine/insert_test.go | Adds an engine-level regression test for sharded pinned inserts. |
| go/vt/vtgate/executor_dml_test.go | Adds an executor-level regression test ensuring pinned-table inserts only hit the pinned shard. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cbed6ea41e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if len(ins.ColVindexes) == 0 && ins.VTable.Pinned != nil { | ||
| ic.TargetDestination = key.DestinationKeyspaceID(ins.VTable.Pinned) | ||
| } |
There was a problem hiding this comment.
Propagate pinned destination to INSERT ... SELECT plans
This change wires TargetDestination only in buildInsertPrimitive, which fixes INSERT ... VALUES but leaves the INSERT ... SELECT transformer path (transformInsertionSelection) constructing engine.InsertSelect without a pinned destination. For pinned sharded tables that have no insert vindexes, InsertSelect will still fall through to sharded vindex routing and call processVindexes with an empty ColVindexes slice, so INSERT ... SELECT on pinned tables remains broken instead of being routed to the pinned shard.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Wired TargetDestination for pinned tables in the INSERT...SELECT path too, matching the existing logic in buildInsertPrimitive (commit f322ddb).
The pinned table feature has been implemented for some time. This removes the outdated statement in the V3 Vindex design doc that said pinned tables were yet to be implemented. Related: PR #19746 which fixes INSERT support for pinned tables.
The TargetDestination for pinned sharded tables was only set in buildInsertPrimitive (INSERT...VALUES). Apply the same check in transformInsertionSelection so INSERT...SELECT also routes to the pinned shard when ColVindexes is empty.
Description
This PR fixes inserts for pinned tables in sharded keyspaces.
Pinned tables already carry a fixed keyspace id in the vschema, and generic sharded routing already treats them as single-shard
EqualUniqueroutes. The insert pipeline was bypassing that routing path and unconditionally requiring primary vindex values, which causedINSERTinto pinned tables to fail withVT09001.This change carries the pinned destination through insert planning and execution, then routes inserts directly to the pinned shard when there are no insert vindex columns to evaluate. Regression coverage was added in both the engine and vtgate executor tests.
Related Issue(s)
Fixes #19529
Checklist
Deployment Notes
None.
AI Disclosure
This PR was written with assistance from OpenAI Codex. I reviewed the final diff and local test results.