Add regression test for issue ladybug#866 (SIGSEGV re-executing parameterized write) - #53
Merged
Merged
Conversation
…ting parameterized write)
Replicates the scenario from issue #866: the same parameterized write
query string (CREATE (:Contribution {id: $id, embedding: $embedding}))
executed repeatedly through Connection.execute() and the implicit
prepared-statement cache, followed by a vector similarity read.
On the 0.20.0 engine this segfaulted on the second execution: the
cached-physical-plan fast path calls prepareForReuse(), which reaches
FactorizedTable::clear() on the write statement root ResultCollector.
That FactorizedTable has an empty result schema, and the constructor
skips allocating flatTupleBlockCollection / inMemOverflowBuffer for
empty schemas, so clear() dereferenced a null pointer. The vector
extension and ARRAY_COSINE_SIMILARITY in the original report were not
involved in the crash.
Verified: this test SIGSEGVs against a v0.20.0 build and passes on
current main (which contains the clear() empty-schema guard from
LadybugDB/ladybug#862).
…ckpoint=false
Since ladybug commit 4ca1d6f5d ("Fix #755: COPY respects auto_checkpoint
setting"), COPY with auto_checkpoint=false leaves the data in the WAL
instead of checkpointing. The FSM tests captured used page ranges right
after COPY, at which point storage_info() reported in-memory chunks with
start_page_idx = INVALID (nothing was on disk yet), so after
drop + checkpoint there was nothing in the FSM to compare against and
all reclaim assertions failed.
Fix: checkpoint explicitly after COPY in the fixtures (and after the
re-copy in test_fsm_reclaim_node_table_recopy) so the tests measure real
on-disk page indices, preserving the tests' intent of verifying FSM
reclaim of persisted pages.
Contributor
Author
|
The 9 CI failures on this PR's run are pre-existing on main and fall into three groups; fixes pushed here / on the way:
Note: CI here builds ladybug main, so this PR's CI will go green only after #870 merges. |
adsharma
added a commit
to LadybugDB/ladybug
that referenced
this pull request
Aug 31, 2026
The cached-physical-plan fast path (Phase 1-3, #78e1ccfd4/e103a49f2/ 80fa473) clones the template operator tree per execution and calls prepareForReuse(). Two pieces of mutable state were not handled: 1. Table-function scans silently returned 0 rows on re-execution. TableFunctionCall::copy() shares the TableFuncSharedState, and prepareForReuse() calls sharedState->resetState() to reset the scan position. FTableScanSharedState overrides resetState(), but SimpleTableFuncSharedState - used by pandas/polars/arrow scans and most simple table functions - inherited the no-op base implementation, so curRowIdx stayed at numRows and every getMorsel() returned an invalid morsel. Concretely: executing "LOAD FROM df" (which the Python layer rewrites to "LOAD FROM $df") followed by "LOAD FROM $df RETURN *" returned 0 rows / "No more tuples in QueryResult". Fix: override resetState() to reset curRowIdx, mirroring FTableScanSharedState. 2. ResultCollector::prepareForReuse() clobbered live QueryResults. The plan template shares the ResultCollectorSharedState (and its FactorizedTable) with every executed clone, and prepareForReuse() unconditionally clear()ed that table. A QueryResult from a previous execution holds a shared_ptr to the same table, so overlapping executions (e.g. AsyncConnection's pool) corrupted live results: queries returned other queries' rows or empty tables (test_async_prepare_and_execute_concurrent asserted [96] == [1]). Fix: clear-and-reuse the table only when no QueryResult still references it (use_count() == 1); otherwise give the execution a fresh table with the same schema and let the old one live until its QueryResult is destroyed. The sequential loop case keeps the Phase 2 block-reuse fast path. Verified: full Python suite (280 tests) passes including test_scan_pandas and test_async_connection; api_test, c_api_test, and 734 e2e tests pass (remaining failures pre-exist on main). Unblocks CI on LadybugDB/ladybug-python#53.
adsharma
added a commit
to LadybugDB/ladybug
that referenced
this pull request
Aug 31, 2026
Relates to #866. Submodule PR: LadybugDB/ladybug-python#53
adsharma
added a commit
to LadybugDB/ladybug
that referenced
this pull request
Aug 31, 2026
Relates to #866. Submodule PR: LadybugDB/ladybug-python#53
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.
Summary
Root cause (for reference)
Not a vector-extension bug. The crash was FactorizedTable::clear() dereferencing null block collections: on the cached-physical-plan fast path, prepareForReuse() reaches clear() for a write statement's root ResultCollector, whose FactorizedTable has an empty result schema and therefore never allocated flatTupleBlockCollection / inMemOverflowBuffer. Already fixed on the main repo side by the empty-schema guard in clear() (LadybugDB/ladybug#862).
Verification
Relates to: LadybugDB/ladybug#866