fix: test dataframe failures#40
Merged
Merged
Conversation
…rrow params _pybind_value_signature used raw id(value) for pointer-type parameters (DataFrames, Arrow tables, Polars DataFrames). Python recycles object ids after garbage collection (~99.7% of the time a new allocation reuses a freed id). On a shared connection (conn_db_readonly), this caused the cache to return a stale prepared statement compiled for a completely different DataFrame — e.g. string columns from test_pyarrow_string reused for dictionary-encoded int32 columns in test_pyarrow_dict — producing data corruption (dictionary indices read as string offsets). Fix: replace raw id(value) with a weakref-validated generation counter. _pybind_pointer_tracker[id(obj)] -> (gen, weakref.ref(obj)) On each call: If id is found AND the weakref is still alive → reuse the cached generation (same object → cache hit, correct). If the weakref is dead (original object was GCd and id recycled) → purge the stale entry and assign a fresh generation. If not found → assign a fresh generation and store the weakref. This preserves correct caching for repeated calls with the same object while being immune to id recycling. Fixes test_pyarrow_dict intermittent AssertionError.
…n safety
Include a schema fingerprint (shape + column names + dtypes for pandas,
field names + types for pyarrow/polars) in the prepared-statement cache
key for pointer-type parameters. Without this, mutating a DataFrame
in place between execute() calls (e.g. df.drop(columns=[...],
inplace=True)) would reuse the cached prepared statement compiled for
the original schema, causing data corruption.
The cache key tuple expands from 4 to 5 elements:
("pointer", module, type_name, gen) -> ("pointer", module, type_name, gen, schema_fp)
The fingerprint is computed fresh on every call; two objects with the
same identity and schema share the cache entry, while schema mutations
produce a different key and trigger a fresh prepare.
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.
id(value) is not a good way to finger print a dataframe for statement caching purposes.