Skip to content

fix(cross_shard_disabled): provide more specific error msg when dealing with unknown sharding key - #1365

Open
jkaczman wants to merge 5 commits into
mainfrom
jk-unknown-sharding-key-error
Open

fix(cross_shard_disabled): provide more specific error msg when dealing with unknown sharding key#1365
jkaczman wants to merge 5 commits into
mainfrom
jk-unknown-sharding-key-error

Conversation

@jkaczman

Copy link
Copy Markdown
Contributor

Provides a specific error message "unknown sharding key was specified" as opposed to "cross-shard queries are disabled" when a query:

  1. has an unknown sharding key set (list-based / range-based)
  2. cross_shard_disabled=true
  3. would be cross-shard if not stopped

Fixes #1163

Comment thread pgdog/src/net/messages/error_response.rs Outdated
Co-authored-by: Lev Kokotov <levkk@users.noreply.github.com>
let query = context.client_request.query()?;
let error = ErrorResponse::cross_shard_disabled(query.as_ref().map(|q| q.query()));

let error = if let Some(sharding_key) = context.params.get("pgdog.sharding_key")

@levkk levkk Aug 14, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is fine for now. A better solution could be to check this inside the parser:

  1. If it returns Shard::All
  2. and pgdog.sharding_key is set in params (those are available in the context, see
    pub pgdog_sharding_key: Option<&'a ParameterValue>,
    )
  3. and cross-shard queries are disabled (this may be currently missing from the context but can be easily added I think:
    pub struct QueryParserContext<'a> {
    )

then the parser can return this error, which we can catch and send to the client.

The reason this is better is because it avoids connecting to Postgres when we know we can't serve that query because of the missing sharding key mapping.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another reason why the parser could be a better place for this is because it'll also catch the comment directive:

/* pgdog_sharding_key: 25 */ SELECT * FROM users;

while this check will only catch the pgdog.sharding_key SET parameter.

@codecov

codecov Bot commented Aug 14, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@jkaczman
jkaczman force-pushed the jk-unknown-sharding-key-error branch 2 times, most recently from bcc4437 to a8a56ce Compare August 14, 2026 21:10
@jkaczman
jkaczman force-pushed the jk-unknown-sharding-key-error branch from a8a56ce to 6c8dbac Compare August 14, 2026 21:12
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.

Warn about unknown sharding key rather than assume a cross-shard query

2 participants