Repository navigation
Type row identity: StorageKey internal, RowIdentity client-visible - #322
Merged
Merged
Conversation
…surrogate A keyless schemaless row and a timeseries row were self-bound under the decimal surrogate string, the same convention auto-_rowid rows use. That decimal string is not the document storage key those rows are actually stored under, so RETURNING id, SELECT id, and later lookups by that id disagreed with each other. FreshSurrogateKind now names the binding convention a fresh row takes (AutoRowId vs DocumentStorageKey), and assign_fresh returns the bound identity string alongside the surrogate so every caller uses the same value the allocator minted instead of re-deriving it. fresh_identity_string is the single formatter both the allocator and the SQL-plan converter's placeholder paths call. The response envelope also forwards its id onto a scan-wrapper body that has none, so a keyless row's identity survives projection.
A PRIMARY KEY declared on a columnar collection's natural key column
(not id / document_id) tombstoned and re-appended the row on a
duplicate insert, matching the id-based convention but not ANSI SQL
uniqueness semantics. ColumnarInsertIntent::InsertUnique now checks
the PK index first and raises RejectedConstraint (23505) instead,
while ON CONFLICT DO NOTHING keeps its existing InsertIfAbsent path
regardless of the declared key.
build_columnar_schema and build_schema_bytes take the declared
primary-key column name so the schema seeded at bootstrap and the
one built per-statement agree on which column is the key.
columnar_row_surrogates and resolve_doc_identity mint per-row
surrogates off the same declared column rather than guessing from
the id/document_id/key convention, so distinct natural keys never
collapse onto one surrogate.
The insert conversion module splits into insert/{convert,identity,
schema}.rs along those three concerns, and its wire test grows a
declared_key.rs and spatial.rs case file alongside the existing
columnar.rs cases.
A duplicate INSERT only raised 23505 when the declared primary key
sat on a column other than id/document_id; those two names silently
fell back to tombstone-and-reinsert (UPSERT semantics) even without
an explicit UPSERT. A declared PRIMARY KEY now means uniqueness on
every column it names, id included — convert_insert checks
declared_pk.is_some() rather than excluding the id/document_id
convention.
primary_key threads through ConvertInsertArgs, ConvertUpsertArgs,
resolve_doc_identity_with_declared, columnar_row_surrogates and
build_schema_bytes as a plain &str instead of Option<&str>: the
planner now always resolves one before reaching these converters,
and the DML visitor arms reject a missing resolution as a PlanError
instead of letting callers guess a fallback. DEFAULT_IDENTITY_COLUMN
replaces the "id" literal scattered across schema-building and
catalog conversion so the synthetic primary-key name has one
definition. resolve_doc_identity and its is_auto_rowid_pk/
declared_primary_key_name helpers move to narrower pub(in ...)
visibility now that upsert resolves the declared key once per
statement instead of once per row, mirroring convert_insert.
Row ids surfaced from change events, array surrogate scans, and
vector search/upsert/write now go through the same
surrogate_to_doc_id encoding the document store uses for its
storage key, instead of ad hoc decimal or "{:08x}" formatting that
could disagree with the key a row is actually stored under.
Wire tests for columnar and spatial INSERT-conflict now expect
23505 on a duplicate declared-PK insert and confirm the original
row is untouched; the prior tombstone/UPSERT-shaped coverage moves
to an explicit UPSERT statement, which still merges as before.
…e key
A document's storage key (fixed-width hex) and its client-visible
identity (decimal surrogate, or a user/DDL-declared primary key) were
conflated across the codebase: change events, array surrogate scans,
and vector search/upsert/write formatted or compared identities with
ad hoc decimal or "{:08x}" logic that could disagree with the key a
row is actually stored under.
StorageKey and RowIdentity in engine::document::store::key are now
the only two representations: StorageKey wraps a surrogate as the
redb key it is stored under, RowIdentity is what a client sees.
identity_of(doc_id) is the single place that reinterprets a storage
key's hex shape into a RowIdentity, falling back to the raw string for
a non-minted (user-supplied or declared) key. surrogate_to_doc_id and
doc_id_to_surrogate become thin wrappers over the new types so the 200+
existing call sites that hold a plain String keep working.
assign_fresh drops the FreshSurrogateKind parameter now that every
fresh row binds through the same RowIdentity convention, removing the
enum and its call sites in nodedb-physical and the surrogate assigner.
ConvertCollection's MetaOp gains a source_storage_mode field, resolved
from the catalog before the DDL mutates collection_type, so the Data
Plane decodes rows being converted the same way the collection's own
register path would; convert_collection also re-registers the Data
Plane's doc_configs entry after a successful conversion so later reads
see the new storage mode without a restart.
merge_orchestrated's apply module splits into apply/{mod,orchestrate,
insert_rows,update_rows}.rs, and the sql_typeguard_defaults wire test
splits into sql_typeguard_defaults/{mod,convert,defaults,validate}.rs,
which gains coverage confirming a strict-mode conversion preserves a
minted row's identity.
Row identity was resolved inconsistently across the write and read
paths — some call sites treated a document's storage key as an opaque
string, others re-derived it from user-supplied fields, letting a
period-lock or materialized-sum reference resolve to a row that looked
right but did not match the surrogate the Control Plane actually
bound.
Introduce nodedb_types::{StorageKey, RowIdentity} as the single
encoding boundary: StorageKey is the internal fixed-width hex key a
document is stored under, RowIdentity is what a client sees. Thread
StorageKey through the document store, sparse btree, bulk DML,
upsert/merge/transaction handlers, WAL replay, and diagnostics so every
read and write resolves the same row the planner resolved.
Extend period-lock enforcement to resolve the reference row through
the Control Plane's resolved_targets surrogate binding instead of a
raw key lookup, and add PeriodLockMisconfigured (SQLSTATE 23609) to
distinguish a reference row missing its configured status column from
an actually locked period.
Replace the free function resolve_one_target and its Vec<ResolvedSumTarget> accumulator with a ResolvedTargets struct. Dedup on (target, value) used to scan the accumulated Vec linearly per candidate; ResolvedTargets tracks seen values per target in a HashMap<String, HashSet<String>>, so a page or scan that touches the same target row many times resolves it in O(1) instead of O(n). Materialized-sum and period-lock resolution share the same struct.
A current-mode document fetch no longer falls back to scan_collection when the sparse store holds nothing for the collection: it now only ever returns sparse rows, each keyed by its rendered storage key, dropping the Fetched/ RowOrigin distinction between sparse and foreign rows entirely. That fallback was the only thing letting VERIFY_HASH_CHAIN, TEMPORAL_LOOKUP, CONVERT_CURRENCY, VERIFY_BALANCE, BALANCE_AS_OF, and CREATE GRAPH INDEX answer over a KV or columnar-family collection at all, silently over zero rows. Each now fails closed via a new CollectionReadGate::require_document_engine check (SQLSTATE 0A000), or the equivalent check in CREATE GRAPH INDEX, instead of reporting an empty result as if it were a true one. Native dispatch's build_scan now routes a plain collection scan to the collection's own engine (KV, timeseries, columnar, spatial) instead of always emitting a document scan, and collection_type propagates catalog errors and resolves against the caller's database id instead of the default one.
Secondary-index reads and writes on the sparse btree engine took document ids as raw &str/String, forcing a StorageKey -> String -> StorageKey round-trip at every index_put/delete/lookup call site. IndexEntryTxn, index_put, delete_index_entries_for_field, and range_scan now carry a StorageKey directly, and with_tenant_key4 renders its last segment via Display instead of requiring a pre-formatted &str. invalid_storage_key_err takes the source table name (DOCUMENTS, INDEXES, INDEXES_VERSIONED) so a malformed-key error reports which table produced it. Propagate the StorageKey-typed document id through the document store's put/delete/batch paths and every executor handler that builds or consumes a SecondaryIndexInputs, bulk delete/update, undo, or truncate call.
Replace raw string doc_id parameters with the typed StorageKey across the versioned btree key layout, document store engine, and every executor handler that reads or writes documents. This removes the NUL-byte validation that existed only to protect string-based keys and derives PartialOrd/Ord/Hash on StorageKey so it can be used directly as a key and in index structures. invalid_storage_key_err now takes a KeyedTable enum instead of a raw table name string, covering the versioned documents and indexes tables alongside the existing ones.
VersionedIndexEntry::doc_id and versioned_index_lookup_as_of now carry StorageKey directly instead of round-tripping through its string form, removing the parse-at-the-boundary calls in callers and tests.
Replace ad-hoc &str/String doc-id handling in the scan, merge, and facet paths with the typed StorageKey, matching the storage layer's key representation end to end. Row matching, overlay merging, bulk DML staging, and secondary-index counting now compare typed keys instead of parsing or re-stringifying hex identities per row.
The overlay's insert/tombstone/TTL paths and every stage_write caller keyed staged rows by a raw string doc_id, so a bulk write and a point get for the same row could land on different overlay slots whenever the row carried a declared or default identity column instead of its surrogate. Overlay lookups, undo journal entries, and doc-id iteration now key by the typed RowIdentity everywhere. Add RowIdentity::of_stored_row / extract_pk_value / value_to_pk_string in nodedb-types::row_identity as the single place that derives a stored row's identity from its body, the DDL's declared primary key, and its storage key, replacing the equivalent private helper that lived in target_identity::pk. Calvin's bulk delete and bulk update overlay staging, KV TTL staging, and every DML stage_write path now derive and thread this identity instead of falling back to the storage key's decimal surrogate or the doc_id's string form.
Replace document_id string params and surrogate_to_doc_id conversions with StorageKey across point put/delete/update, upsert, merge, bulk DML, transaction undo/overlay, WAL dispatch, and replay handlers. StorageKey is derived once at the entry point and passed through, removing redundant surrogate-to-string conversions along call chains.
Change events, hybrid-search fusion, and WAL replay dispatch carried row identity as raw String/&str throughout, so a batch write's "*" sentinel, a headless vector hit, and a real primary key were all the same type and could be mixed up at call sites. - Generalize nodedb_query::fusion's RankedResult/FusedResult over a typed key `K: Clone + Eq + Hash + Ord` instead of hardcoding String, and factor the shared score/sort/truncate logic into finish_fusion. - Add HybridFusionKey (Bound(StorageKey) or Headless(u32)) as the one key space the vector and text legs of a hybrid search fuse on, so a headless vector hit can never be misread as a bound surrogate. - Route change events, extract_write_metadata, and WAL CRDT replay through nodedb_types::RowIdentity instead of String/&str, rendering to text only at wire and response boundaries. - Update GraphRAG, graph expansion, and text-search-hybrid/triple handlers and CDC/WS/pgwire tests to the typed key.
… raw surrogate HybridFusionKey now carries its own MessagePack wire encoding (the hex storage key, or the __local_<id> headless sentinel) and VectorSearchHit's id field is typed as HybridFusionKey instead of a bare u32 surrogate. Dense, sparse, and multi-vector search, the transaction overlay merge, and the response-codec flatteners all read and filter through this key so a headless (surrogate-less) hit is represented distinctly rather than aliased onto surrogate 0. The Control Plane's surrogate-or-headless decode is consolidated into a shared parse_surrogate_hex helper (response_translate/hit_key.rs) used by the vector, text-hybrid, and MERGE/UPDATE...FROM exchange translators, replacing three separate ad hoc parses of the sentinel prefix. Spatial R-tree entry ids are now a dedicated SpatialEntryId newtype (hashed from either a document's storage key or a columnar row's user id) instead of raw fnv1a_hash calls at each call site, so the put and remove paths cannot hash a row's identity differently. vector_doc_map, VectorIndexDelta, and the vector undo-log entries are rekeyed from String doc ids to typed StorageKey / Option<StorageKey>, and ResolvedUpdateRow / ResolvedUpdateRowWire drop the Option<Surrogate> in favor of carrying the resolved StorageKey (or a surrogate that is now always present, since every matched row is storage-keyed) directly.
Row is now an enum (Row/Batch/Edge/Heartbeat) carrying the same RowIdentity a row's INSERT minted, instead of an ad hoc Arc<str>. Edge rows compose their src/label/dst text once at construction so the event ring pays for one identity, not four strings. Threads a declared_primary_key column through the write paths that build events and redo entries — TRUNCATE, bulk DELETE/UPDATE, UPDATE FROM, and the materialized-sum resolve pass — so a row emitted from these paths reports its declared PRIMARY KEY value, or the decimal surrogate when the collection has none, matching what live writes and WAL replay already emit. Updates the executor, WAL replication encode/decode, event plane consumers, CDC router, and CRDT sync packager to construct and read the new RowId shape, and updates cluster and in-process tests to match.
Replace surrogate_to_doc_id, doc_id_to_surrogate, and identity_of with direct calls to StorageKey::for_surrogate, StorageKey::parse, and StorageKey::to_identity at every call site. The wrappers only saved a method call and no longer earn their keep now that most callers already hold a StorageKey. Moves the otel receiver off its hand-rolled hex shim onto the workspace hex crate, promoted from dev-dependencies to a regular dependency of nodedb.
farhan-syah
added
the
run-ci
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
1 participant
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.
Why
Rows minted without a declared key had two identity encodings live at once: the internal hex storage key and the client-visible id disagreed at several call sites (period-lock and materialized-sum references, CDC row ids, vector/hybrid search hits, WAL redo). Separately, a declared
PRIMARY KEYnamedidordocument_idwas not enforced: a duplicate INSERT tombstoned and re-inserted the row (UPSERT semantics) instead of rejecting it, while any other column name already raised23505.Closes #315
Closes #309
What changes
Two new types in
nodedb-types::row_identity:StorageKeyDOCUMENTS/INDEXESredb tablesRowIdentityStorageKey::parseis the only place a stored key is reinterpreted as a surrogate, so a PK value that happens to look like 8 hex characters can never be mistaken for one.Plan/wire fields added or changed:
declared_primary_keyOption<String>DocumentOp::{BulkDelete, Truncate, ApplyBalanceDelta},MaterializedSumBindingResolvedUpdateRowWiresurrogateu32(wasOption<u32>)UPDATE ... FROMresolve passWriteSetEntry.identityRowIdentityStorageKey::to_string()RowIdRow/Batch/Edge/HeartbeatArc<str>event row idVectorSearchHit.idHybridFusionKeyu32surrogatePeriodLockMisconfigured23609ErrorDetailsvariantBehaviour changes
PRIMARY KEYnamedid/document_id23505on every engineVERIFY_HASH_CHAIN/TEMPORAL_LOOKUP/CONVERT_CURRENCY/VERIFY_BALANCE/BALANCE_AS_OF/CREATE GRAPH INDEXover a KV or columnar-family collection0A000u32: a surrogate, or a local index id for a headless hit, indistinguishable__local_<id>for a headless hitUPDATE...FROM, materialized-sum resolveCONVERTrow failure23607locked,23609when the reference row lacks the configured status columnFixed on the way
collection_typepropagates catalog errors and resolves against the caller's database id, instead of defaulting on any lookup failure.SpatialEntryIdtype instead of an untyped integer.Compatibility
Pre-1.0, no migration:
WriteSetEntry,RowId, andVectorSearchHitwire shapes changed — old and new binaries are not compatible on the same WAL or event stream.23505; callers relying on that behavior must sendUPSERTexplicitly.CREATE GRAPH INDEXover a non-document collection that previously ran over zero rows now return0A000.