Uh oh!
There was an error while loading. Please reload this page.
Fix PostgreSQL vector search filter rendering - #14316
Conversation
There was a problem hiding this comment.
Pull request overview
This PR fixes PostgreSQL vector search filter rendering in PostgresCollection by ensuring pre-rendered filter fragments are treated as SQL (not quoted literal values) when interpolated via psycopg.sql.SQL.format, and adds a unit regression test for the expected WHERE predicate output.
Changes:
- Wrap single and combined filter clauses with
psycopg.sql.SQL(...)before formatting/joining so they render as executable SQL predicates. - Add a regression unit test validating that an equality filter renders into the query’s
WHEREclause. - Update unit test imports to include
VectorSearchOptions.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| python/semantic_kernel/connectors/postgres.py | Ensures filter fragments are composed as SQL (not literals) when building the vector search query. |
| python/tests/unit/connectors/memory/test_postgres_store.py | Adds regression coverage asserting the rendered WHERE clause is present in the constructed query. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| VectorSearchOptions(filter="lambda x: x.id == 1"), | ||
| ) | ||
| assert 'WHERE "id" = 1' in query.as_string() |
| sql.SQL("WHERE {clause}").format( | ||
| clause=sql.SQL(" AND ").join(sql.SQL(clause) for clause in where_clauses) | ||
| ) | ||
| if isinstance(where_clauses, list) | ||
| else sql.SQL("WHERE {clause}").format(clause=where_clauses) | ||
| else sql.SQL("WHERE {clause}").format(clause=sql.SQL(where_clauses)) |
There was a problem hiding this comment.
MAF Automated Review — Iteration 1
Result: Findings reported
Scope: full PR (1 commit(s)): def46f9c135b
Model:claude-opus-4.8
Overview
The change is a correct functional fix for #14311: filter clauses were previously
handed to psycopg as plain strings, which SQL.format adapts as quoted string
literals (WHERE '"id" = 1'), so every filtered vector search rendered an inert
or invalid predicate. Wrapping clauses in sql.SQL(...) produces real predicates
for both the single and list branches, and a regression test pins the single-clause
case. The one residual risk is that the added sql.SQL(where_clauses) call feeds a
value typed Any | Sequence[Any] into an API annotated for str, which fails the
repository's required mypy lint gate and blocks merge.
Reviewed the supplied pull-request change set across correctness, security/reliability, architecture, and failure behavior.
1 verified finding remained after source verification (1 medium) across 1 file. Details are attached to the affected lines below.
Affected areas:python/semantic_kernel/connectors/postgres.py
| ) | ||
| if isinstance(where_clauses, list) | ||
| else sql.SQL("WHERE {clause}").format(clause=where_clauses) | ||
| else sql.SQL("WHERE {clause}").format(clause=sql.SQL(where_clauses)) |
There was a problem hiding this comment.
where_clauses here is the non-list return of _build_filter, typedAny | Sequence[Any] (OptionalOneOrMany), but sql.SQL(...) is annotated to
accept only str/LiteralString. mypy fails on this line witherror: Argument 1 to "SQL" has incompatible type "Any | Sequence[Any]"; expected "str" [arg-type]. The required lint workflow runs uv run mypy -p semantic_kernel
and connectors.postgres is not in mypy.ini's ignore list, so this check now
fails and blocks merge; the base commit was mypy-clean on this file. In the
single-clause path _build_filter returns a str, so this is a type-annotation
mismatch rather than a runtime fault - coerce or narrow the value so the argument
is a str, e.g. sql.SQL(str(where_clauses)), so the lint gate passes.
Fixes#14311.\n\nPostgresCollection builds filter clauses as SQL text, but passing plain strings to psycopg SQL.format treats them as quoted literal values. Wrap single and combined clauses with sql.SQL so filters produce executable predicates instead of invalid WHERE string literals.\n\nAdded a regression test covering a rendered equality filter.\n\nValidation: git diff --check and python3 -m py_compile passed. The targeted pytest could not be collected locally because the checkout is missing the OpenTelemetry dependency.