fix: preserve Postgres vector search filter SQL - #14320
Conversation
There was a problem hiding this comment.
Pull request overview
This PR fixes PostgreSQL vector-search filter predicates being treated as a single SQL string literal during psycopg.sql formatting, ensuring pre-built filter fragments are emitted as SQL rather than quoted values.
Changes:
- Wraps
_build_filter()output withpsycopg.sql.SQL(...)so formattedWHEREclauses remain SQL predicates (single and multi-filter cases). - Adds an offline regression test asserting the generated SQL contains the expected
WHERE "name" = 'test'predicate.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| python/semantic_kernel/connectors/postgres.py | Adjusts vector-query filter composition to preserve predicate SQL when formatting with psycopg. |
| python/tests/unit/connectors/memory/test_postgres_store.py | Adds a regression test validating vector-search filter SQL emission. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| if where_clauses := self._build_filter(options.filter): # type: ignore | ||
| query += ( | ||
| sql.SQL("WHERE {clause}").format(clause=sql.SQL(" AND ").join(where_clauses)) | ||
| 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)) |
| from semantic_kernel.data.vector import DistanceFunction, IndexKind, VectorStoreField, vectorstoremodel | ||
| from semantic_kernel.data.vector import VectorSearchOptions |
| collection = vector_store.get_collection(collection_name="test_collection", record_type=FilterDataModel) | ||
| options = VectorSearchOptions(filter="lambda x: x.name == 'test'") | ||
|
|
||
| query, _, _ = collection._construct_vector_query([1.0, 2.0, 3.0], options) | ||
|
|
||
| assert 'WHERE "name" = \'test\'' in query.as_string() |
There was a problem hiding this comment.
MAF Automated Review — Iteration 1
Result: No findings
Scope: full PR (1 commit(s)): 5fcbf0e8081c
Model: claude-opus-4.8
Overview
The change corrects a real defect in PostgresCollection._construct_vector_query: _build_filter returns already-validated, escaped SQL predicate text, but the previous code passed it to sql.SQL("WHERE {clause}").format(...) as a value, so psycopg quoted the entire predicate into a single string literal (WHERE '"name" = ''test''') and the filter never worked. Wrapping each fragment in sql.SQL(...) for both the single-string and list branches restores the parser's intended embedding contract, and the added regression test genuinely fails pre-fix and passes post-fix. Injection safety was independently checked: predicate provenance is single-sourced through _lambda_parser, which allowlists identifiers against the data model and doubles single quotes in string constants, so under default PostgreSQL settings (standard_conforming_strings = on) no clean injection path exists. Residual concerns (manual-escaping reliance vs. parameter binding, backslash handling under non-default server config, an unescaped non-string Constant fallback, missing WHERE-clause space, and multi-filter test coverage) are all either pre-existing in unchanged code or Low-priority polish, not defects introduced by this PR.
Reviewed the supplied pull-request change set across correctness, security/reliability, architecture, and failure behavior.
No publishable findings remained after source verification for this scope.
Summary
Fix
PostgresCollectionvector-search filters being quoted as a single SQL string literal.When
_build_filterreturns pre-built predicate strings,psycopg.sql.Composable.format()treats plain strings as values. PostgreSQL therefore receives a quoted text literal instead of a boolean predicate. The fix wraps the already-validated, escaped predicate fragments withsql.SQLfor both single and multiple filters and preserves the required whitespace beforeWHERE.The regression test uses the real
VectorSearchOptionsmodel and verifies both a single filter and multiple filters joined withAND.Review follow-up
The automated review identified and this update fixes:
WHERE, which would have produced...table"WHERE....Validation
python -m py_compile python/semantic_kernel/connectors/postgres.py python/tests/unit/connectors/memory/test_postgres_store.py— passed.git diff --check— passed.maintree via the GitHub API, so the full package/test dependency environment is not present locally.Closes #14311