Skip to content

fix(cross-shard): decode merged rows with the column types the shards return - #1917

Open
murex971 wants to merge 3 commits into
mainfrom
murex-cross-shard-merge-fic
Open

murex971 wants to merge 3 commits into
mainfrom
murex-cross-shard-merge-fic

Conversation

@murex971

Copy link
Copy Markdown
Contributor

Problem

Cross-shard merges (aggregates, ORDER BY, DISTINCT, LIMIT) decode rows using a RowDescription.
For named statements, that header came from the global prepared-statement cache. The cache is keyed
by SQL text only and is never refreshed, so after resharding widened integer keys to bigint,
PgDog decoded 8-byte values with a 4-byte header: FATAL 58000 type: wrong size slice. (Refer #1837)
Unnamed statements never had a header for Bind/Execute, so the decoder panicked:
decoder has no row description set. (Refer #1836)

Fix

Get the header from the shards with the rows, like the simple protocol already does:

  • When rows are merged across shards, PgDog sends Describe right before each Execute,
    unless the client just described itself.
  • MultiShard uses those RowDescription/NoData replies to decode rows but doesn't forward them.
  • Remove the cached RowDescription from the global cache and the decoder's lookup of it,
    plus the internal statement Describe for cross-shard variants, which is now redundant.

@codecov

codecov Bot commented Oct 29, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@github-actions

github-actions Bot commented Oct 29, 2026 •

Copy link
Copy Markdown
Contributor

Metal bench (arm64) failed. 4 of 103 measures outside bounds.

suite throughput cpu/txn user cycles/txn user instr/txn kernel cycles/txn ctx/txn
pooler 28.87 (+2.4% ± 1.4%), bound +2% 29,884 (+3.1% ± 2.3%), bound +2% 34,958 (-0.3% ± 0.04%) 39,403 (+2.2% ± 1.7%)
lb 29.92 (+2.5% ± 1.6%), bound +2% 31,402 (+3.2% ± 1.4%), bound +2% 35,824 (-0.3% ± 0.05%) 40,383 (+2.2% ± 2.3%)
sharding-w2 51.73 (-0.7%) 66,689 (-1.5%) 72,053 (-0.3%) 59,729 (-0.3%) 1.415 (+0.6%)
sharding-w8 157,144 (-1.2%) 46.5 (+0.4%) 66,803 (-0.7%) 55,885 (-0.3%) 53,030 (+1.7%) 0.1484 (+5.7%)
  • pooler: user cycles +3.1%, instructions flat. Unexplained.
  • lb: user cycles +3.2%, instructions flat. Unexplained.
Counters per txn
suite frontend stall l1i refill backend stall
pooler 16,346 (+7.4% ± 4.9%) 3,478 (+3.4% ± 1.9%) 2,258 (-8.7% ± 2.9%)
lb 17,360 (+7.1% ± 2.8%) 3,574 (+3.4% ± 1.1%) 2,454 (-6.7% ± 2.7%)
sharding-w2 38,288 (-3.0%) 5,414 (-0.7%) 6,493 (+0.8%)
sharding-w8 37,919 (-3.0%) 5,039 (-1.9%) 9,166 (+8.2%)

Microbench: all 90 measures within bounds.

lb flamegraph vs 97fc159

lb

Open interactive for hover, zoom and search.

lb removed

Open interactive.

microbench flamegraph vs 97fc159

microbench

Open interactive for hover, zoom and search.

microbench removed

Open interactive.

pooler flamegraph vs 97fc159

pooler

Open interactive for hover, zoom and search.

pooler removed

Open interactive.

sharding-w2 flamegraph vs 97fc159

sharding-w2

Open interactive for hover, zoom and search.

sharding-w2 removed

Open interactive.

sharding-w8 flamegraph vs 97fc159

sharding-w8

Open interactive for hover, zoom and search.

sharding-w8 removed

Open interactive.

Logs

lb, microbench, pooler, sharding-w2, sharding-w8, verdict.json

Compared 8a5872c with 97fc159 on c7g.metal (on-demand).

@murex971
murex971 marked this pull request as ready for review October 29, 2026 15:39
@github-actions

github-actions Bot commented Oct 29, 2026 •

Copy link
Copy Markdown
Contributor

Metal bench (x86) failed. sharding-w2, sharding-w8 failed to run.

Microbench: all 90 measures within bounds.

microbench flamegraph vs 97fc159

microbench

Open interactive for hover, zoom and search.

microbench removed

Open interactive.

Logs

lb, microbench, pooler, sharding-w2, sharding-w8, verdict.json

Compared 8a5872c with 97fc159 on c7a.metal-48xl (spot).

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant