Skip to content

fix: preserve Postgres vector search filter SQL - #14320

Open
mikemikimike wants to merge 3 commits into
microsoft:mainfrom
mikemikimike:fix/postgres-filter-sql
Open

fix: preserve Postgres vector search filter SQL#14320
mikemikimike wants to merge 3 commits into
microsoft:mainfrom
mikemikimike:fix/postgres-filter-sql

Conversation

@mikemikimike

@mikemikimike mikemikimike commented Aug 23, 2026

Copy link
Copy Markdown

Summary

Fix PostgresCollection vector-search filters being quoted as a single SQL string literal.

When _build_filter returns 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 with sql.SQL for both single and multiple filters and preserves the required whitespace before WHERE.

The regression test uses the real VectorSearchOptions model and verifies both a single filter and multiple filters joined with AND.

Review follow-up

The automated review identified and this update fixes:

  • Missing whitespace before WHERE, which would have produced ...table"WHERE....
  • Missing coverage for the list-filter branch.
  • Duplicate vector-module imports in the test.

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.
  • Focused pytest was not run because the local checkout could not be completed over Git transport; the changed source and test files were obtained from the upstream main tree via the GitHub API, so the full package/test dependency environment is not present locally.

Closes #14311

Copilot AI lite review requested due to automatic review settings August 23, 2026 14:54
@mikemikimike
mikemikimike requested a review from a team as a code owner August 23, 2026 14:54

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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 with psycopg.sql.SQL(...) so formatted WHERE clauses 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.

Comment on lines +796 to +802
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))
Comment on lines +20 to +21
from semantic_kernel.data.vector import DistanceFunction, IndexKind, VectorStoreField, vectorstoremodel
from semantic_kernel.data.vector import VectorSearchOptions
Comment on lines +343 to +348
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()

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

PostgresCollection: string/lambda filter produces an invalid WHERE clause (whole predicate collapsed into a string literal)

2 participants