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

Aquery ivfflat fix #17922

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open

Aquery ivfflat fix #17922

wants to merge 7 commits into from

Conversation

forbug
Copy link

@forbug forbug commented Feb 25, 2025

Description

As described in the issue, vector store index querying was not successful
when the PGVectorStore was initialized with the ivfflat_probes argument.
This problem is potentially due to the behavior of psycopg2/3 relying on server-side binding,
which does not support SET statements. However, this is just
a hypothesis and may be related to a different issue.

The proposed solution works successfully and formats the variable directly
in the SET string.

Fixes # 17877

New Package?

Did I fill in the tool.llamahub section in the pyproject.toml and provide a detailed README.md for my new integration or package?

  • Yes
  • No

Version Bump?

Did I bump the version in the pyproject.toml file of the package I am updating? (Except for the llama-index-core package)

  • Yes
  • No

Type of Change

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)

How Has This Been Tested?

Your pull-request will likely not be merged unless it is covered by some form of impactful unit testing.

  • I added new unit tests to cover this change

Suggested Checklist:

  • I have performed a self-review of my own code
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • I ran make format; make lint to appease the lint gods

Gretchen Forbush and others added 5 commits February 24, 2025 16:55
As described in the issue, vector store index querying was not successful
when the PGVectorStore was initialized with the `ivfflat_probes` argument.
This problem is potentially due to the behavior of psycopg3 sending parameters
separately, using [server-side binding](https://www.psycopg.org/psycopg3/docs/basic/from_pg2.html#server-side-binding),
which may mis-behave when performed asynchronously. However, this is just
a hypothesis and may be related to a different issue.

The proposed solution works successfully and formats the variable directly
in the `SET` string.
Adding pytest per conributing guidelines
@dosubot dosubot bot added the size:M This PR changes 30-99 lines, ignoring generated files. label Feb 25, 2025
@forbug
Copy link
Author

forbug commented Feb 25, 2025

NOTE: This integration should be updated to use psycopg, which combines the functionality of psycopg2 and asyncio. I believe it would be backwards compatible, but would need to be tested. I do not have the time at the moment to update every instance of psycopg2/asyncio within the repo to update the dependency.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size:M This PR changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant