Fix segfault when SETting an indexed embedding while a live HNSW index exists (#896) - #78
Merged
Merged
Conversation
- Guard shrinkForNode() against a null EmbeddingHandle when the node's embedding scanned from the node table is NULL (e.g. its embedding was SET to NULL while still referenced in the graph). Previously this dereferenced a null handle and segfaulted in the distance function. - Document that OnDiskHNSWIndex::update() runs after the node table has been updated, so scanned embeddings are the new value. - Add regression tests: SET on a row that was NULL at index build, SET to NULL and back, and NULL-row SET surviving checkpoint + reopen. Fixes LadybugDB/ladybug#896 (together with the NodeTable::update ordering fix in the main repo).
adsharma
added a commit
to LadybugDB/ladybug
that referenced
this pull request
Sep 2, 2026
Bumps ladybugdb/extensions to the fix-896-hnsw-set-null branch (pr LadybugDB/extensions#78): guards shrinkForNode() against a NULL embedding scanned from the node table and adds regression tests.
adsharma
added a commit
to LadybugDB/ladybug
that referenced
this pull request
Sep 2, 2026
Bumps ladybugdb/extensions to the fix-896-hnsw-set-null branch (pr LadybugDB/extensions#78): guards shrinkForNode() against a NULL embedding scanned from the node table and adds regression tests.
adsharma
added a commit
to LadybugDB/ladybug
that referenced
this pull request
Sep 2, 2026
Bumps ladybugdb/extensions to the merged fix (0b4f40d7f, PR LadybugDB/extensions#78): guards shrinkForNode() against a NULL embedding scanned from the node table and adds regression tests.
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 the vector-extension half of LadybugDB/ladybug#896.
Problem
SET n.embedding = $vecon a node whose embedding is NULL, under a live HNSW index, segfaults nondeterministically insideshrinkForNode() -> computeDistance().shrinkForNode()read the node's embedding from the node table and could get a NULL (nullEmbeddingHandle) value, then passedgetPtr()straight to the distance function — reading through a garbage pointer in release builds. Depending on memory layout, this either segfaults or silently corrupts search distances. A crash also leaves the WAL unreplayable.Changes
shrinkForNode(): return early when the scanned embedding is a null handle (the node has no embedding, e.g. its embedding was SET to NULL while still referenced in the graph) instead of asserting in debug / crashing in release.OnDiskHNSWIndex::update()runs after the node table row was updated (paired main-repo change), so embeddings scanned during re-insertion are the new value.update.test:CHECKPOINT+ reopen (on-disk only).Testing
Paired with the main-repo PR (ladybugdb/ladybug) which reorders
NodeTable::update()to apply the table update before index maintenance.