refactor(pgvector): extract PostgreSQLDocumentStore abstract base class#3254
Open
SyedShahmeerAli12 wants to merge 4 commits intodeepset-ai:mainfrom
Open
refactor(pgvector): extract PostgreSQLDocumentStore abstract base class#3254SyedShahmeerAli12 wants to merge 4 commits intodeepset-ai:mainfrom
SyedShahmeerAli12 wants to merge 4 commits intodeepset-ai:mainfrom
Conversation
Move all SQL constants and data-layer methods into a new `_base.py` abstract base class (`PostgreSQLDocumentStore`). `PgvectorDocumentStore` is now a thin subclass that only implements `_ensure_db_setup` and `_ensure_db_setup_async` using psycopg. Future PostgreSQL-backed integrations (AlloyDB, etc.) can subclass `PostgreSQLDocumentStore` and override only the connection layer, inheriting all SQL/data logic for free. Closes deepset-ai#3239
SyedShahmeerAli12
requested review from
anakin87
and removed request for
a team
github-actions
Bot
added
integration:pgvector
type:documentation
Contributor
Coverage report (pgvector)
Click to see where and how coverage changed
This report was generated by python-coverage-comment-action |
||||||||||||||||||||||||||||||
Contributor
Author
|
@anakin87 @davidsbatista this PR implements the refactoring proposed in #3239. Would love your feedback when you get a chance! |
Contributor
|
@SyedShahmeerAli12, please, focus on one PR at a time. You currently have 6 PRs open, all of which need review, or are in review and need refining, refactoring, or cleaning. I ask you no to open any more PRs until all are closed or merged. |
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
2 participants
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.
Related Issues
PgvectorDocumentStoremaking it reusable for future PostgreSQL-related integrations #3239Proposed Changes:
Splits
PgvectorDocumentStoreinto two layers:_base.py(new file) —PostgreSQLDocumentStoreabstract base class containing all SQL constants and every data-layer method (sync and async). Declares_ensure_db_setupand_ensure_db_setup_asyncas@abstractmethod.document_store.py(slimmed from ~2040 to ~230 lines) —PgvectorDocumentStore(PostgreSQLDocumentStore)now only contains the psycopg connection setup (_ensure_db_setup,_ensure_db_setup_async) and serialization (to_dict,from_dict).__init__.py— exportsPostgreSQLDocumentStorealongsidePgvectorDocumentStoreso future integrations (AlloyDB, etc.) can import the base class frompgvector-haystack.SupabasePgvectorDocumentStorerequires no changes — it inherits fromPgvectorDocumentStoreand gets the base class for free.How did you test it?
All existing unit tests pass (
pytest -m "not integration"— 60 tests):Imports verified:
Notes for the reviewer
VALID_VECTOR_FUNCTIONSand related constants are re-exported fromdocument_store.pyfor backward compatibility (imported byPgvectorEmbeddingRetriever)."PgvectorDocumentStore"by name have been made generic so they stay meaningful when called from AlloyDB or other subclasses.Checklist
refactor: