fix: don't sum row counts across shards for EXECUTE on omnisharded tables#1179
Draft
mscrivo wants to merge 2 commits into
Draft
fix: don't sum row counts across shards for EXECUTE on omnisharded tables#1179mscrivo wants to merge 2 commits into
mscrivo wants to merge 2 commits into
Conversation
…bles EXECUTE of a server-side prepared statement was routed through the DDL catch-all, which broadcasts to all shards without the omnisharded flag. For statements that only touch omnisharded tables, the CommandComplete row counts from each shard were summed, so a DELETE/UPDATE of N rows reported N * shards rows to the client. Route PREPARE explicitly and record the statement behind the name in the client's prepared statements state. EXECUTE resolves the name (or a globally cached name rewritten by prepared_statements = "full") and carries the omnisharded flag, so cross-shard results are deduplicated, not aggregated. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
Contributor
|
This touches the parser, which we are in the middle of a major rewrite of. This isn't likely to get reviewed until after that is done (we are hoping next week, but take that with a huge grain of salt) |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Problem
EXECUTEof a server-side prepared statement targeting an omnisharded table returned the sum of row counts from all shards instead of the count from one shard:Plain
UPDATE/DELETE/INSERTon omnisharded tables already report the correct count (#752), butExecuteStmtfell through to the DDL catch-all, which broadcasts withRoute::write(Shard::All)and no omnisharded flag — soMultiShardaggregated theCommandCompletecounts.Fix
PREPAREexplicitly: still broadcast to all shards, but record the statement behind the name in the client'sPreparedStatementsstate (newsimplemap, cleared withclose_all()).EXECUTEexplicitly: resolve the name — first from the recorded SQL-level PREPAREs, falling back to the global cache for names rewritten byprepared_statements = "full"— and set the omnisharded flag on the route when the underlying statement only touches omnisharded tables.MultiShardthen deduplicates results across shards instead of aggregating them.RouterContext::with_prepared_statements().This also extends the omni-write-in-direct-to-shard-transaction guard (#1086) to
EXECUTEof omnisharded writes, since that check keys off the same route flag.Testing
parser/query/test/test_execute.rs(fail without the fix):EXECUTEof omnisharded UPDATE/DELETE carries the omnisharded flag; sharded/unknown statements don't; global-name resolution works.EXECUTE upd('x')now returnsUPDATE 3instead ofUPDATE 6, and the write still reaches every shard. Same forDELETE.cargo nextestparser suite,cargo clippy,cargo check --features new_parserall pass.🤖 Generated with Claude Code