Support isNull and isNotNull in PgVectorFilterExpressionConverter - #6904
Open
draviteja wants to merge 1 commit into
Open
Support isNull and isNotNull in PgVectorFilterExpressionConverter#6904draviteja wants to merge 1 commit into
isNull and isNotNull in PgVectorFilterExpressionConverter#6904draviteja wants to merge 1 commit into
Conversation
Signed-off-by: Raviteja Daggupati <raviteja5255@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
isNull()andisNotNull()fromFilterExpressionBuilderdon't work withPgVector. They throw when the search runs.
The cause is in
PgVectorFilterExpressionConverter.doExpression, which startswith:
ISNULLandISNOTNULLhave no right operand, so this always fails. Theawkward bit is that it only shows up at query time. The expression itself
builds fine, so any test that stops at building it will pass.
This change handles the two types before that assert. The JSONPath it
generates:
A key counts as null if it's missing, or if it's there but set to JSON null.
That's the same rule
SimpleVectorStoreFilterExpressionEvaluatoruses.The Elasticsearch converter does it differently and only checks whether the
field exists. That gives a different answer for a key that exists holding
null, which can happen in JSONB. I went with the
SimpleVectorStorebehaviour, but happy to switch if you'd rather match Elasticsearch.
The brackets are needed. JSONPath gives
&&higher precedence than||, sowithout them
isNull(k) && year >= 2020would also match a document that hasno
kandyear2019.One thing this doesn't fix:
NOT(x IS NULL)still fails with "Unknownexpression type: ISNULL" from
FilterHelper.negate. That's in core andaffects every store, not just PgVector. I have a fix for it if you want it as
a separate PR.
Tests: 7 unit tests in
PgVectorFilterExpressionConverterTestsand 4 cases inPgVectorStoreIT. I couldn't run the IT locally (no Docker, no OpenAI key),so I ran the generated SQL against a local Postgres to check the behaviour
instead.