Repository navigation
fix: Skip empty nested filter groups in SQLite and Postgres filter translators - #7052
Open
RaphaelFakhri wants to merge 1 commit into
Open
RaphaelFakhri wants to merge 1 commit into
RaphaelFakhri wants to merge 1 commit into
Conversation
…anslators Signed-off-by: Raphael Fakhri <153192858+RaphaelFakhri@users.noreply.github.com>
This branch has not been deployed
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.
What this PR does / why we need it:
SqliteFilterTranslator.translate_compoundandPostgresFilterTranslator.translate_compoundjoin every sub-clause, including the empty clause returned for an empty nestedCompoundFilter. A filter such asand[eq(title, "Title 1"), or[]]becomes(... AND ), and SQLite fails withnear ")": syntax error. Postgres gets the same trailingAND ). A group made only of empty groups becomes( AND )or().This change skips empty sub-clauses and returns an empty clause when none remain, which matches
MilvusFilterTranslator. The callers already treat an empty clause as "no filter".Tests in
tests/unit/test_filter_models.pycheck the generated clause for both translators, run the SQLite clause against an in-memory database, and cover groups made only of empty groups.Which issue(s) this PR fixes:
Fixes #7051
Checks
git commit -s)Testing Strategy
Misc
pytest tests/unit/test_filter_models.py: 4 of the new tests fail without the fix and all 27 pass with it.