Skip to content

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

3 participants