Skip to content

fix(sharding): apply default mapping for any table#1077

Open
meskill wants to merge 1 commit into
mainfrom
push-kunuyyukxqnr
Open

fix(sharding): apply default mapping for any table#1077
meskill wants to merge 1 commit into
mainfrom
push-kunuyyukxqnr

Conversation

@meskill

@meskill meskill commented Jun 15, 2026

Copy link
Copy Markdown
Contributor

There could be some discrepancies for sharding depending on configuration that is not very obvious.

Here is the before and after map.

[[sharded_tables]] name [[sharded_mappings]] table Before After
absent absent ✅ mapping applied ✅ mapping applied
"users" "users" ✅ mapping applied ✅ mapping applied
"users" absent ❌ hash routing - no mapping ✅ mapping applied
absent "users" ❌ hash routing - no mapping ❌ not fixed yet

I wanted to verify if that what do we actually want, or do we want to apply mapping directly to the sharded_table configuration and require specific table/mapping to be defined in order to it to be merged together.

I'd think about fix both cases in this pr and apply catch-all settings for any other specific configuration or add the validation for configuration to prevent misuse.

@codecov

codecov Bot commented Jun 15, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 85.12397% with 36 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
pgdog/src/frontend/router/sharding/test/mod.rs 84.25% 34 Missing ⚠️
pgdog/src/backend/databases.rs 92.30% 2 Missing ⚠️

📢 Thoughts on this report? Let us know!

@levkk levkk left a comment

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.

Kinda complicated, but I get it. I think we may want a more tight config for sharding here. name is really optional if the column is the same in all tables, for example. We may even want to provide some kind of config builder based on the schema and users' input.

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.

2 participants