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

langchain_community.vectorstores: PGVector filtering - top level AND and OR operator #17064 #17067

Closed
wants to merge 5 commits into from

Conversation

djo10
Copy link
Contributor

@djo10 djo10 commented Feb 5, 2024

Solution for #17064
Description:

  • Included top level AND and OR operators for filtering eg: filter = {"or": [{"rating": {"gte": 8.5}}, {"genre": "animated"}]}
  • included GTE and LTE comparators
  • Code optimized and refactored, using Comparator and Operator classes from libs/langchain/chains/query_constructor/ir.py

Copy link

vercel bot commented Feb 5, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Ignored Deployment
Name Status Preview Comments Updated (UTC)
langchain ⬜️ Ignored (Inspect) Visit Preview Feb 8, 2024 6:16pm

@dosubot dosubot bot added size:L This PR changes 100-499 lines, ignoring generated files. Ɑ: vector store Related to vector store module 🤖:improvement Medium size change to existing code to handle new use-cases 🔌: supabase Primarily related to Supabase integrations labels Feb 5, 2024
@djo10
Copy link
Contributor Author

djo10 commented Feb 7, 2024

@eyurtsev

filter_by_metadata = None

return filter_by_metadata
def _create_filter_clause(self, filters): # type: ignore[no-untyped-def]
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could we add a type for the filters?

"between": lambda f, v: f.between(str(v[0]), str(v[1])),
}

def parse_condition(field, condition): # type: ignore[no-untyped-def]
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is it possible to add type annotations here to make it easier to follow what's going on?

from langchain.chains.query_constructor.ir import Comparator, Operator

COMPARISONS = {
Comparator.EQ: lambda f, v: f == str(v),
Copy link
Collaborator

Choose a reason for hiding this comment

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

What happens if value was an integer and the field is of an integer type?

Comparator.IN: lambda f, v: f.in_(v),
Comparator.NIN: lambda f, v: f.not_in(v),
Comparator.CONTAIN: lambda f, v: f.contains(v),
"contains": lambda f, v: f.contains(v),
Copy link
Collaborator

Choose a reason for hiding this comment

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

cc @baskaryan looks like we've got some operations not covered by the existing enums?

@eyurtsev
Copy link
Collaborator

Awesome! I'm wondering are there any existing unit tests that help cover the internal refactor? Could you confirm you ran the unit tests? The code looks good from a high level, but hard for me to determine whether it works without digging in.

@eyurtsev eyurtsev added waiting-on-author PR Status: Confirmation from author is required stale Issue has not had recent activity or appears to be solved. Stale issues will be automatically closed and removed 🔌: supabase Primarily related to Supabase integrations labels Feb 22, 2024
@eyurtsev
Copy link
Collaborator

eyurtsev commented Mar 8, 2024

similar to #12977

@eyurtsev eyurtsev added the 🔌: postgres Related to postgres integrations label Mar 8, 2024
@eyurtsev
Copy link
Collaborator

eyurtsev commented Mar 8, 2024

Comandeering

@eyurtsev eyurtsev removed stale Issue has not had recent activity or appears to be solved. Stale issues will be automatically closed waiting-on-author PR Status: Confirmation from author is required labels Mar 8, 2024
@eyurtsev
Copy link
Collaborator

closing in favor of: #18992

@eyurtsev eyurtsev closed this Mar 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🤖:improvement Medium size change to existing code to handle new use-cases 🔌: postgres Related to postgres integrations size:L This PR changes 100-499 lines, ignoring generated files. Ɑ: vector store Related to vector store module
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants