Conversation
SipHash-only $label keys merged colliding labels into one bitmap. Identity is now digest plus length-delimited UTF-8, so hash-only keys are unrepresentable. Co-authored-by: Cursor <cursoragent@cursor.com>
Comment on lines
+87
to
+90
| fn equality_data_key(scope: DataScope, property: &str, value: &str) -> Result<Bytes, HelixDbError> { | ||
| if property == NODE_LABEL_PROPERTY { | ||
| return node_label_data_key(scope, value); | ||
| } |
Contributor
There was a problem hiding this comment.
Current stores skip label migration
When an existing version-0x0004 database contains hash-only label indexes, startup runs no migration while these lookups construct only canonical label keys, causing existing nodes and edges to disappear from label scans, labeled expansion and counts, and labeled shortest-path traversal until the indexes are rebuilt.
Knowledge Base Used:
Existing 0x0004/0x0005 stores must fail closed and rebuild CanonicalLabel indexes from graph rows instead of serving empty LabelScan. Co-authored-by: Cursor <cursoragent@cursor.com>
Member
|
main issue here is the storage format change, will need to take a bit longer to look at it and ensure correctness and migration support |
This branch has not been deployed
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.
Fixes #1060.
Summary
$labelkeys were SipHash-only, so colliding labels shared one Roaring bitmap. Identity is nowCanonicalLabel: digest is a scan accelerator, exact identity is length-delimited UTF-8.0x07). Edge-label and neighbor keys carry the same canonical frame. Parse fails closed on digest mismatch and on old 10-byte hash keys.LabelScan, expand, count, shortest path) construct from the label string. Dual-read of hash keys is not the contract.Test plan
cargo test -p db --lib labelcargo test -p db --lib topologyMade with Cursor
Greptile Summary
The PR replaces hash-only graph-label identities with digest-accelerated, exact UTF-8 canonical keys across encoding, topology mutation, and graph lookup paths.
Important Files Changed
Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart LR A[Existing v4 database] --> B[Writer bootstrap] B --> C[Version already 0x0004] C --> D[No label-index migration] D --> E[Old hash-only label rows remain] F[Label lookup at head] --> G[Construct canonical label key] G --> H[No matching row] E --> H H --> I[Existing labeled graph data omitted]Reviews (1): Last reviewed commit: "fix: give graph labels exact CanonicalLa..." | Re-trigger Greptile
Context used: