Skip to content
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

fix: PgvectorDocumentStore - use appropriate schema name if dropping index #1277

Merged
merged 4 commits into from
Jan 10, 2025

Conversation

mabartcz
Copy link
Contributor

@mabartcz mabartcz commented Jan 6, 2025

Related Issues

Proposed Changes:

In pgvector if HNSW index should drop, only index_name was used, but not schema_name. So if there were multiple schemas, the index was not dropped and could not be recreated.

How did you test it?

The check was added directly to the code, so if the index existed and should be dropped, the Error is raised, because I consider it as critical Error.

Notes for the reviewer

I tested this on multiple machines with similar setups, and the behavior was not consistent. On one PC this fix was needed, but on the other PC this fix was not needed, and it was working even without the fix, but I was not able to discover the difference...
But I believe that the fix is correct, and should be applied.

Checklist

@mabartcz mabartcz requested a review from a team as a code owner January 6, 2025 12:56
@mabartcz mabartcz requested review from mpangrazzi and removed request for a team January 6, 2025 12:56
@CLAassistant
Copy link

CLAassistant commented Jan 6, 2025

CLA assistant check
All committers have signed the CLA.

Copy link
Member

@anakin87 anakin87 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR!
I agree with the change, but I found some opportunities for improvement.

@anakin87 anakin87 changed the title fix: Add schema name if dropping index in pgvector store fix: PgvectorDocumentStore - use appropriate schema name if dropping index Jan 10, 2025
Copy link
Member

@anakin87 anakin87 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, @mabartcz.
Merging now!

@anakin87 anakin87 merged commit 20011ec into deepset-ai:main Jan 10, 2025
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

pgvector - hnsw index table drop is not using schema name
3 participants