fix: use uuid4 (not uuid1) for auto-generated ids in async constructors#314
Open
Andrew Chen (chuenchen309) wants to merge 1 commit into
Open
fix: use uuid4 (not uuid1) for auto-generated ids in async constructors#314Andrew Chen (chuenchen309) wants to merge 1 commit into
Andrew Chen (chuenchen309) wants to merge 1 commit into
Conversation
PGVector's sync constructor path (`__from`) mints auto-generated document ids with `uuid.uuid4()`, but the async path (`__afrom`, used by `afrom_texts`/`afrom_embeddings`/`afrom_documents`) used `uuid.uuid1()`. uuid1 encodes the host MAC address and a timestamp, so ids generated via the async API (a) diverge from the random ids the sync API produces and (b) leak the server's hardware address and record-creation time into the stored/returned ids. `aadd_embeddings` itself already uses uuid4, so this lone uuid1 was an oversight rather than intentional. Align `__afrom` with the sync path and the rest of the module by using uuid4. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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.
Problem
PGVector's sync constructor path (__from, used byfrom_texts/from_embeddings/from_documents) generates auto-assigned document ids withuuid.uuid4():The async path (
__afrom, used byafrom_texts/afrom_embeddings/afrom_documents) instead useduuid.uuid1():uuid1encodes the host's MAC address and a timestamp. So when a callerlets the store auto-generate ids (the common case), the async API:
produces for the same call, and
ids that get stored in the DB and returned to callers.
These ids are not discarded —
__afrompasses them intoaadd_embeddings,which writes them as the primary-key
id.aadd_embeddingsitself already usesuuid4for its own auto-generation, so this loneuuid1looks like anoversight rather than a deliberate choice.
Fix
Use
uuid.uuid4()in__afrom, matching__fromandaadd_embeddings.Test
Added
tests/unit_tests/v1/test_afrom_uuid.py, a pure-Python test (no livePostgres): constructing with
async_mode=Truedefers__post_init__so noconnection is opened, and
aadd_embeddingsis stubbed to capture the generatedids. It asserts every auto-generated id is a UUIDv4. Fails before the change
(version == 1), passes after.
ruff checkpasses on the changed files.Disclosure: I used AI assistance (Claude) to help locate this sync/async
parity gap and draft the test. I reviewed the change, ran the test suite, and
take responsibility for its correctness.