Repository navigation
fix: return an error for INSERT .. SELECT into sharded tables when router can't find shards of rows - #1987
Merged
Conversation
jkaczman
force-pushed
the
jk-insert-select-shard-error
branch
from
November 9, 2026 21:34
3dee97a to
f6c30c2
Compare
Contributor
|
Metal bench (arm64) passed. All 475 measures within bounds.
Counters per txn
Microbench: all 462 measures within bounds. Hot code per txn
pooler:
sharding-w2:
Logslb, microbench, pooler, sharding-w2, sharding-w8, verdict.json Compared 0b7e835 with ba27030 on c8g.metal-24xl (on-demand). |
Contributor
|
Metal bench (x86) passed. All 475 measures within bounds.
Counters per txn
Microbench: all 462 measures within bounds. Microbench latency: 53 benches changed more than 3%
Hot code per txn
pooler:
sharding-w2:
Logslb, microbench, pooler, sharding-w2, sharding-w8, verdict.json |
jkaczman
force-pushed
the
jk-insert-select-shard-error
branch
3 times, most recently
from
November 9, 2026 21:55
7d572f1 to
0b7e835
Compare
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
…uter can't find shards of rows
jkaczman
force-pushed
the
jk-insert-select-shard-error
branch
from
November 9, 2026 23:50
0b7e835 to
6df7724
Compare
jkaczman
marked this pull request as ready for review
November 9, 2026 23:50
jkaczman
added a commit
that referenced
this pull request
Nov 10, 2026
…ables (not only SELECT *) (#2000) #1988, the previous PR implementing unnest split, only handled splitting when the SELECT for the INSERT was `SELECT *`, e.g., in `INSERT INTO <sharded table> .. SELECT * FROM unnest($1, ..)`. As this is unimplemented functionality, since #1987, we (correctly) returned an error for this, as this is one of the cases not handled by that previous PR (anything other than `SELECT *`). Now, as an extension of #1988, this PR also splits these unnest INSERTs when the SELECT explicitly lists the output columns of unnest(..) one at a time instead of `*`, as well as when the SELECT includes constants (e.g., `NULL`). The motivation for this is fully completing unnest support (instead of VALUES) for re-writing subqueries. E.g., if we have a statement such as > `INSERT INTO subpackages (id, user_id, order_id) SELECT id, id + 1, NULL FROM suborders` The subquery support I'm working on for unnest will run `SELECT id, id + 1, NULL FROM suborders` on the shards. The shards return some rows, e.g., `(1, 2, NULL)`, `(2, 3, NULL)` and `(3, 4, NULL)`. Then, subqueries will send these rows back to the INSERT as unnest(..) arrays. However, the shards return the `NULL` column with the type `text` (default Postgres association for the NULL constant). If we sent the `NULL` column as an array too, the array would be a `text[]`, and Postgres would return an error when we insert `text` into the `bigint` column `order_id`. Not to mention, this is inefficient. Therefore, I opted when implementing support for this in subqueries to keep the constants in the SELECT as a nice workaround: > `INSERT INTO subpackages (id, user_id, order_id) SELECT u.c1, u.c2, NULL FROM unnest($1::pg_catalog._int8, $2::pg_catalog._int8) AS u(c1, c2)` `SELECT *` would only return the columns of unnest(..), so there'd be no way for SELECT to include the `NULL` with `SELECT *` without the array mentioned prior. Therefore, we need this PR before subquery support can use unnest(..) to properly split constants apart from the unnest array. To route the rows, this follows the alias `AS u(c1, c2)` and the SELECT to find the unnest(..) argument that Postgres uses to insert into the sharding key. Then, the rows are routed as if the client sent `INSERT INTO subpackages (id, user_id, order_id) VALUES ($1, $2, NULL)`.
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.
Previously, when the subquery rewrite was off, queries matching
INSERT INTO <sharded table> .. SELECT ..were routed incorrectly if the sharding key wasn't explicitly specified (e.g, from a generate_series or unnest function), as the router can't find the sharding keys for the rows belonging to the SELECT.E.g., if we have a statement such as
the router, right now, has no idea what
generate_series(1, 10)would return, thus, currently uses round-robin to route that query, meaning that, in a case where we have two shards, all the 10 rows from that generate_series function are routed to a single shard (e.g., Shard A), even though some of those ids belong on Shard B.Now, the router returns an error for these kind of INSERTs in this situation.
A lot of this is irrelevant after subqueries are enabled by default, but until then, this prevents incorrect functionality stemming from this kind of query.