Fix metadata deserialization in async mode for PGVector #125
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,7 @@ | ||
| # pylint: disable=too-many-lines | ||
| from __future__ import annotations | ||
|
|
||
| import json | ||
| import contextlib | ||
| import enum | ||
| import logging | ||
|
|
@@ -1057,17 +1058,38 @@ async def asimilarity_search_with_score_by_vector( | |
|
|
||
| def _results_to_docs_and_scores(self, results: Any) -> List[Tuple[Document, float]]: | ||
| """Return docs and scores from results.""" | ||
| docs = [] | ||
| for result in results: | ||
| metadata = result.EmbeddingStore.cmetadata | ||
|
|
||
| # Attempt to convert metadata to a dict | ||
| try: | ||
| if isinstance(metadata, dict): | ||
| pass # Already a dict | ||
| elif isinstance(metadata, str): | ||
| metadata = json.loads(metadata) | ||
| elif hasattr(metadata, 'buf'): | ||
| # For Fragment types (e.g., from asyncpg) | ||
|
There was a problem hiding this comment. only psycopg3 is supported There was a problem hiding this comment. Hi @eyurtsev, thanks for the review! I understand that only psycopg3 is officially supported. However, I’ve received reports of issues in async mode that suggest some users might be encountering non‑dict metadata (perhaps inadvertently using asyncpg or similar drivers). This patch adds defensive logic to convert metadata that isn’t already a dict (for example, when it’s a JSON string, a Fragment‑like object with a I’ve also added unit tests to simulate these scenarios and ensure the conversion works as expected. Please let me know if you’d like any adjustments or if you think we should further restrict this behavior given our psycopg3-only support. There was a problem hiding this comment. @shamspias feel free to @ me if I don't respond quickly enough. Are you able to create a minimal reproduction against the actual vectorstore? If so, you can send me the code snippet and I'm happy to update the tests myself.
Can you confirm that this is specifically from asyncpg where you're seeing the failures? We definitely don't want to mock the results from asyncpg. If we want to support asynpcg, the way to do it is to run the full suite of tests with that driver. There was a problem hiding this comment. Hi @eyurtsev, I’ve put together a minimal reproduction that runs against a real Postgres instance (with pgvector) using asyncpg. The test confirms that the defensive logic for non-dict metadata triggers correctly without mocking. Let me know if you’d like the code snippet or any adjustments! |
||
| metadata_bytes = metadata.buf | ||
| metadata_str = metadata_bytes.decode('utf-8') | ||
| metadata = json.loads(metadata_str) | ||
| elif hasattr(metadata, 'decode'): | ||
| # For other byte-like types | ||
| metadata_str = metadata.decode('utf-8') | ||
| metadata = json.loads(metadata_str) | ||
| else: | ||
| metadata = {} # Default to empty dict if unknown type | ||
| except Exception as e: | ||
| self.logger.warning(f"Failed to deserialize metadata: {e}") | ||
| metadata = {} | ||
|
|
||
| doc = Document( | ||
| id=str(result.EmbeddingStore.id), | ||
| page_content=result.EmbeddingStore.document, | ||
| metadata=metadata, | ||
| ) | ||
| score = result.distance if self.embeddings is not None else None | ||
| docs.append((doc, score)) | ||
| return docs | ||
|
|
||
| def _handle_field_filter( | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.