Skip to content

Apply node table update before updating indexes (fixes HNSW SET-on-NULL segfault, #896) - #900

Merged
adsharma merged 2 commits into
mainfrom
fix-896-update-before-index
Sep 3, 2026
Merged

Apply node table update before updating indexes (fixes HNSW SET-on-NULL segfault, #896)#900
adsharma merged 2 commits into
mainfrom
fix-896-update-before-index

Conversation

@adsharma

@adsharma adsharma commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Fixes #896.

Problem

SET n.embedding = $vec on a node whose embedding is NULL, while an HNSW vector index exists on the column, segfaults (SIGSEGV) nondeterministically and can leave the WAL unreplayable (Corrupted wal file. Read out invalid WAL record type. on next open). Reported with a self-contained repro and stacks in #896 (0.20.1, macOS arm64).

Root cause

NodeTable::update() called index->update() before writing the new value into the node table. HNSW index maintenance (OnDiskHNSWIndex::update -> commitInsert -> insertToLayer -> createRels -> shrinkForNode) scans the node table to read the updated node's embedding. With the old ordering it read the stale value — NULL when the row had no embedding at index build — producing a null EmbeddingHandle whose getPtr() was passed to the distance function (simsimd_cos_f32_neon), i.e. an out-of-bounds read that segfaults depending on memory layout. This also explains the issue's observed shape-dependence: re-SET of a row that already had a vector and CREATE of a row carrying its vector never hit the NULL path.

Changes

Index kinds have conflicting needs when their column is updated:

  • FTS delete_ re-tokenizes the old document from the node table → must run before the row write,
  • HNSW re-scans the updated node's new embedding during re-insertion → must run after.

NodeTable::update() is now two-phase: run "before" indexes, apply the row update to the node table (local storage or node group), then run "after" indexes. The ordering is keyed on the serialized index type name ("HNSW") to avoid extending the extension-facing Index API, so this PR needs no coordinated extension change. The existing isLoaded() orphaned-holder guard is preserved in both loops.

Paired extension change (already merged, submodule bumped here): LadybugDB/extensions#78 — guards shrinkForNode() against a NULL scanned embedding and adds regression tests.

Testing

  • All FTS + vector extension e2e tests pass (including the 3 regression cases from extensions#78: SET on a row that was NULL at index build, SET to NULL and back, and NULL-row SET surviving checkpoint + reopen). The only local failure is show_loaded_extensions, which requires duckdb/httpfs extensions not built in this environment.
  • 140 core update/set-related e2e tests pass.
  • Issue-scale C++ repro (2000x768-dim vectors + 1000 NULL rows, mu 30 / ml 60 / cosine / efc 200): 50 SETs on NULL rows + CHECKPOINT + reopen, 5/5 runs clean, WAL replays without corruption.

Indexes have conflicting needs when a column they are built on is
updated: FTS delete_ re-tokenizes the OLD document from the node table
(must run before the write), while the HNSW index re-scans the updated
node's embedding during re-insertion (must run after the write).
Previously all indexes ran before the write, so HNSW observed a stale
embedding: when the previous value was NULL it dereferenced a null
EmbeddingHandle and segfaulted in the distance function, potentially
leaving the WAL unreplayable.

Run "before" indexes first (all indexes except HNSW, preserving existing
behavior), then apply the row update to the node table or local storage,
then run the HNSW index update. The ordering is keyed on the serialized
index type name to avoid extending the extension-facing Index API. The
isLoaded() orphaned-holder guard is preserved in both loops.

Fixes #896.
Points ladybugdb/extensions at 0b4f40d7f (merged PR #78), which guards
shrinkForNode() against a NULL embedding scanned from the node table and
adds regression tests for SET on NULL-embedding rows.

Fixes #896 (together with the NodeTable::update two-phase ordering).
@adsharma
adsharma force-pushed the fix-896-update-before-index branch from 4906e9f to b01b1da Compare September 2, 2026 19:27
@adsharma
adsharma merged commit af109bd into main Sep 3, 2026
4 checks passed
@adsharma
adsharma deleted the fix-896-update-before-index branch September 3, 2026 17:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant