Skip to content

feat: add drop table #195

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Apr 15, 2025
Merged

feat: add drop table #195

merged 3 commits into from
Apr 15, 2025

Conversation

averikitsch
Copy link
Collaborator

No description provided.

@eyurtsev eyurtsev self-requested a review April 9, 2025 16:18
@eyurtsev
Copy link
Collaborator

eyurtsev commented Apr 9, 2025

I would suggest implementing on PGEngine for now rather than the vectorstore interface. To mirror the creation API.

I'd suggest re-organizing the API longer-term to decouple schema management from using the vectorstore; i.e., I don't want to have to initialize a vectorstore w/ an embedding service just to delete a table.

  • PGEngine should be the engine / connection pool etc
  • Vectorstore for managing an existing table (add docs, delete, search)

VectorstoreManager (or some other name) accepts PGEngine and is responsible for managing schema:

  • create tables
  • delete tables
  • list tables w/ introspection
  • get a vectorstore by ID
  • index application should live here as well probably?

@averikitsch
Copy link
Collaborator Author

I would suggest implementing on PGEngine for now rather than the vectorstore interface. To mirror the creation API.

I'd suggest re-organizing the API longer-term to decouple schema management from using the vectorstore; i.e., I don't want to have to initialize a vectorstore w/ an embedding service just to delete a table.

  • PGEngine should be the engine / connection pool etc
  • Vectorstore for managing an existing table (add docs, delete, search)

VectorstoreManager (or some other name) accepts PGEngine and is responsible for managing schema:

  • create tables
  • delete tables
  • list tables w/ introspection
  • get a vectorstore by ID
  • index application should live here as well probably?

Agreed! Thanks for the notes. Let's chat more about the VSManager

@eyurtsev
Copy link
Collaborator

@averikitsch feel free to merge. I'd prefer this on PGEngine, but also if we introduce another intermediate entity later we'll deprecate it from either location.

@averikitsch
Copy link
Collaborator Author

@averikitsch feel free to merge. I'd prefer this on PGEngine, but also if we introduce another intermediate entity later we'll deprecate it from either location.

Do you mean as a class method? I have moved it to be an instance method for PGEngine because it needs the connection pool instantiated. I can add more tests as a fast follow.

@eyurtsev
Copy link
Collaborator

Do you mean as a class method? I have moved it to be an instance method for PGEngine because it needs the connection pool instantiated. I can add more tests as a fast follow.

Was thinking as an instance method that takes the table id as a parameter.

@averikitsch averikitsch merged commit 164810f into main Apr 15, 2025
5 checks passed
@averikitsch averikitsch deleted the drop-table branch April 15, 2025 23:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants