Summary
Collection names travel as bare &str through the planner and the catalog, and the distinction that matters — request form (whatever the caller typed) versus canonical form ({database_id}/{bare}, what the plan routes on, what the engine is keyed by) — is carried only by convention. The constructors exist (QualifiedCollection::new, QualifiedCollection::from_stored), but nothing forces a caller to use them, and from_stored is documented as "never call this with an unqualified name" — a rule enforced by comment alone.
Why it matters
This is the mechanism behind a recurring defect class, not a hypothetical one:
- Catalog reads are keyed by the bare name while plans carry the qualified one, so a read handed the wrong form misses and reads as "absent" instead of erroring. Several non-default-database gates and the edge-recon gate were each fixed separately for exactly this.
- Admission initially routed on the caller's form while the planner derived its vShard from the qualified form, so the two disagreed about which vShard a CRDT write belonged to.
from_stored is public and takes any string, so an unqualified name can become a plan's routing key with no error.
A typed constructor that takes the request form and returns the canonical key — something like QualifiedCollection::from_request(database_id, caller_form) — would make the reduction happen in one place. Call sites that must use the bare form (catalog reads) would take an explicit accessor rather than re-deriving it with a string helper, and the compiler would stop a request-form string from reaching a vShard derivation or a catalog key.
Scope note
Proposed while fixing the third instance of this class; deliberately not folded into that fix, which stays a correctness change. Related: the six duplicated db-prefix strippers (#380) are the other half of the same missing abstraction.
Consolidated from #380: control: six copies of the db-prefix strip rule, five of them byte-identical
#380 is the same fix from the other side: every one of the six db-prefix strip copies moves onto the typed CollectionKey conversion (CollectionKey::from_qualified_str). Closing #380 as a duplicate of this issue.
Original body of #380
Summary
Six copies of the same rule — "strip a leading {database_id}/ qualifier to recover the bare collection name the catalog is keyed by" — exist across the Control Plane. Five are byte-identical strip_db_prefix helpers returning &str; the sixth is bare_collection_name, which allocates a String.
Where
| location |
signature |
control/target_identity/naming.rs:11 |
bare_collection_name(DatabaseId, &str) -> String |
control/clone/resolver/rewrite.rs:446 |
strip_db_prefix(DatabaseId, &str) -> &str |
control/server/shared/clone_write/util.rs:43 |
strip_db_prefix(DatabaseId, &str) -> &str |
control/planner/materialized_sum/cross_shard.rs:271 |
strip_db_prefix(DatabaseId, &str) -> &str |
control/planner/materialized_sum/resolve.rs:420 |
strip_db_prefix(DatabaseId, &str) -> &str |
control/planner/period_lock/lookup.rs:49 |
strip_db_prefix(DatabaseId, &str) -> &str |
Why it matters
The rule is easy to get wrong and getting it wrong is silent: passing a qualified name to a catalog keyed by the bare name misses, and the caller reads "absent" rather than "error". That class already caused a run of defects on the non-default-database path — several catalog reads in the planner and admission were fixed one site at a time.
Suggested direction
One helper in target_identity, offered in both forms so hot paths can borrow: a &str-returning function plus the owning wrapper where a String is genuinely needed. Note the return-type split above — the five copies are allocation-free and bare_collection_name is not.
Summary
Collection names travel as bare
&strthrough the planner and the catalog, and the distinction that matters — request form (whatever the caller typed) versus canonical form ({database_id}/{bare}, what the plan routes on, what the engine is keyed by) — is carried only by convention. The constructors exist (QualifiedCollection::new,QualifiedCollection::from_stored), but nothing forces a caller to use them, andfrom_storedis documented as "never call this with an unqualified name" — a rule enforced by comment alone.Why it matters
This is the mechanism behind a recurring defect class, not a hypothetical one:
from_storedis public and takes any string, so an unqualified name can become a plan's routing key with no error.A typed constructor that takes the request form and returns the canonical key — something like
QualifiedCollection::from_request(database_id, caller_form)— would make the reduction happen in one place. Call sites that must use the bare form (catalog reads) would take an explicit accessor rather than re-deriving it with a string helper, and the compiler would stop a request-form string from reaching a vShard derivation or a catalog key.Scope note
Proposed while fixing the third instance of this class; deliberately not folded into that fix, which stays a correctness change. Related: the six duplicated db-prefix strippers (#380) are the other half of the same missing abstraction.
Consolidated from #380: control: six copies of the db-prefix strip rule, five of them byte-identical
#380 is the same fix from the other side: every one of the six db-prefix strip copies moves onto the typed
CollectionKeyconversion (CollectionKey::from_qualified_str). Closing #380 as a duplicate of this issue.Original body of #380
Summary
Six copies of the same rule — "strip a leading
{database_id}/qualifier to recover the bare collection name the catalog is keyed by" — exist across the Control Plane. Five are byte-identicalstrip_db_prefixhelpers returning&str; the sixth isbare_collection_name, which allocates aString.Where
control/target_identity/naming.rs:11bare_collection_name(DatabaseId, &str) -> Stringcontrol/clone/resolver/rewrite.rs:446strip_db_prefix(DatabaseId, &str) -> &strcontrol/server/shared/clone_write/util.rs:43strip_db_prefix(DatabaseId, &str) -> &strcontrol/planner/materialized_sum/cross_shard.rs:271strip_db_prefix(DatabaseId, &str) -> &strcontrol/planner/materialized_sum/resolve.rs:420strip_db_prefix(DatabaseId, &str) -> &strcontrol/planner/period_lock/lookup.rs:49strip_db_prefix(DatabaseId, &str) -> &strWhy it matters
The rule is easy to get wrong and getting it wrong is silent: passing a qualified name to a catalog keyed by the bare name misses, and the caller reads "absent" rather than "error". That class already caused a run of defects on the non-default-database path — several catalog reads in the planner and admission were fixed one site at a time.
Suggested direction
One helper in
target_identity, offered in both forms so hot paths can borrow: a&str-returning function plus the owning wrapper where aStringis genuinely needed. Note the return-type split above — the five copies are allocation-free andbare_collection_nameis not.