diff --git a/python/semantic_kernel/connectors/sql_server.py b/python/semantic_kernel/connectors/sql_server.py index c8aa904e8d7a..c338f137f12d 100644 --- a/python/semantic_kernel/connectors/sql_server.py +++ b/python/semantic_kernel/connectors/sql_server.py @@ -570,7 +570,7 @@ async def _fetch_records(self, query: SqlCommand) -> AsyncIterable[dict[str, Any cur.execute(*query.to_execute()) col_names = [desc[0] for desc in cur.description] for row in cur: - record = { + yield { col: ( json.loads(row.__getattribute__(col)) if col in self.definition.vector_field_names @@ -578,7 +578,6 @@ async def _fetch_records(self, query: SqlCommand) -> AsyncIterable[dict[str, Any ) for col in col_names } - yield record await asyncio.sleep(0) @override @@ -598,33 +597,43 @@ def parse(node: ast.AST) -> str: op = node.ops[idx] values.append(parse(ast.Compare(left=left, ops=[op], comparators=[right]))) return f"({' AND '.join(values)})" - left = parse(node.left) # type: ignore + left_node = node.left right_node = node.comparators[0] op = node.ops[0] + if isinstance(op, (ast.Eq, ast.NotEq, ast.Is, ast.IsNot)): + if isinstance(left_node, ast.Constant) and left_node.value is None: + left_node, right_node = right_node, left_node + if isinstance(right_node, ast.Constant) and right_node.value is None: + if isinstance(left_node, ast.Constant) and ( + left_node.value is None or isinstance(left_node.value, (str, int, float, bool, bytes)) + ): + matches = (left_node.value is None) == isinstance(op, (ast.Eq, ast.Is)) + return "(1 = 1)" if matches else "(1 = 0)" + return ( + f"{parse(left_node)} IS NULL" + if isinstance(op, (ast.Eq, ast.Is)) + else f"{parse(left_node)} IS NOT NULL" + ) + if isinstance(op, (ast.Is, ast.IsNot)): + raise NotImplementedError(f"Unsupported operator: {type(op)}") + left = parse(left_node) # type: ignore + right = parse(right_node) # type: ignore match op: case ast.In(): - right = parse(right_node) # type: ignore return f"{left} IN {right}" case ast.NotIn(): - right = parse(right_node) # type: ignore return f"{left} NOT IN {right}" case ast.Eq(): - right = parse(right_node) # type: ignore return f"{left} = {right}" case ast.NotEq(): - right = parse(right_node) # type: ignore return f"{left} <> {right}" case ast.Gt(): - right = parse(right_node) # type: ignore return f"{left} > {right}" case ast.GtE(): - right = parse(right_node) # type: ignore return f"{left} >= {right}" case ast.Lt(): - right = parse(right_node) # type: ignore return f"{left} < {right}" case ast.LtE(): - right = parse(right_node) # type: ignore return f"{left} <= {right}" raise NotImplementedError(f"Unsupported operator: {type(op)}") case ast.BoolOp(): @@ -658,7 +667,10 @@ def parse(node: ast.AST) -> str: return QueryBuilder.escape_identifier(node.id) case ast.Constant(): value = node.value - if isinstance(value, (str, int, float, bool, bytes)) or value is None: + if isinstance(value, bool): + command.add_parameter("1" if value else "0") + return "?" + if isinstance(value, (str, int, float, bytes)) or value is None: command.add_parameter(str(value)) return "?" raise VectorStoreOperationException(f"Unsupported constant type: {type(value)}") diff --git a/python/tests/unit/connectors/memory/test_sql_server.py b/python/tests/unit/connectors/memory/test_sql_server.py index 24d235de524f..3a0fbef56c06 100644 --- a/python/tests/unit/connectors/memory/test_sql_server.py +++ b/python/tests/unit/connectors/memory/test_sql_server.py @@ -3,7 +3,7 @@ import json import sys from dataclasses import dataclass -from typing import NamedTuple +from typing import Annotated, NamedTuple from unittest.mock import AsyncMock, MagicMock, NonCallableMagicMock, patch from pytest import fixture, mark, param, raises @@ -21,11 +21,19 @@ _build_select_query, _build_select_table_names_query, ) -from semantic_kernel.data.vector import DistanceFunction, IndexKind, VectorSearchOptions, VectorStoreField +from semantic_kernel.data.vector import ( + DistanceFunction, + IndexKind, + VectorSearchOptions, + VectorStoreField, + default_dynamic_filter_function, + vectorstoremodel, +) from semantic_kernel.exceptions.vector_store_exceptions import ( VectorStoreInitializationException, VectorStoreOperationException, ) +from semantic_kernel.functions.kernel_parameter_metadata import KernelParameterMetadata class TestQueryBuilder: @@ -599,3 +607,107 @@ async def test_no_connection(self, sql_server_unit_test_env, definition): await collection.get("1") with raises(VectorStoreOperationException): await collection.delete("1") + + +@fixture +def filter_collection(sql_server_unit_test_env): + @vectorstoremodel + @dataclass + class FilterRecord: + id: Annotated[int, VectorStoreField("key")] + tenant: Annotated[str, VectorStoreField("data")] + active: Annotated[bool, VectorStoreField("data")] = False + embedding: Annotated[ + list[float], + VectorStoreField("vector", dimensions=3, distance_function=DistanceFunction.COSINE_DISTANCE), + ] = None + + return SqlServerCollection( + collection_name="filter_records", + record_type=FilterRecord, + env_file_path="test.env", + ) + + +@mark.parametrize( + "expression, expected_sql, expected_params", + [ + ("x.tenant == None", "[tenant] IS NULL", []), + ("x.tenant != None", "[tenant] IS NOT NULL", []), + ("x.tenant is None", "[tenant] IS NULL", []), + ("x.tenant is not None", "[tenant] IS NOT NULL", []), + ("None == x.tenant", "[tenant] IS NULL", []), + ("None != x.tenant", "[tenant] IS NOT NULL", []), + ("None is x.tenant", "[tenant] IS NULL", []), + ("None is not x.tenant", "[tenant] IS NOT NULL", []), + ("None == None", "(1 = 1)", []), + ("None != None", "(1 = 0)", []), + ("None is None", "(1 = 1)", []), + ("None is not None", "(1 = 0)", []), + ("'value' != None", "(1 = 1)", []), + ("None == 'value'", "(1 = 0)", []), + ("'value' is not None", "(1 = 1)", []), + ("None is 'value'", "(1 = 0)", []), + ("0 != None", "(1 = 1)", []), + ("False == None", "(1 = 0)", []), + ("not x.tenant == None", "NOT ([tenant] IS NULL)", []), + ("not x.tenant != None", "NOT ([tenant] IS NOT NULL)", []), + ("x.tenant == None or x.id == 1", "([tenant] IS NULL OR [id] = ?)", ["1"]), + ("None == x.tenant == None", "([tenant] IS NULL AND [tenant] IS NULL)", []), + ("'value' == x.tenant != None", "(? = [tenant] AND [tenant] IS NOT NULL)", ["value"]), + ("x.active == True", "[active] = ?", ["1"]), + ("x.active == False", "[active] = ?", ["0"]), + ("x.active != True", "[active] <> ?", ["1"]), + ("x.active != False", "[active] <> ?", ["0"]), + ("x.tenant == 'None'", "[tenant] = ?", ["None"]), + ], +) +def test_null_filter_parser(filter_collection, expression, expected_sql, expected_params): + command = filter_collection._build_filter(f"lambda x: {expression}") + + assert str(command) == expected_sql + assert command.parameters == expected_params + + +def test_null_filter_query_parameter_order(filter_collection): + filters = [ + "lambda x: x.id > 0", + "lambda x: x.tenant == None or x.tenant == 'value'", + "lambda x: x.id < 5", + ] + parsed_filters = filter_collection._build_filter(filters) + cmd = _build_search_query( + "dbo", + "filter_records", + filter_collection.definition.key_field, + filter_collection.definition.data_fields, + filter_collection.definition.vector_fields, + [1.0, 0.0, 0.0], + VectorSearchOptions(vector_property_name="embedding", filter=filters), + filter=parsed_filters, + ) + + assert str(cmd) == ( + "SELECT [id], [tenant], [active], VECTOR_DISTANCE('cosine', [embedding], CAST(? AS VECTOR(3))) as " + "_vector_distance_value\n" + " FROM [dbo].[filter_records] \n" + " WHERE [id] > ? \n" + " AND ([tenant] IS NULL OR [tenant] = ?) \n" + " AND [id] < ? \n" + "ORDER BY _vector_distance_value ASC\n" + "OFFSET 0 ROWS FETCH NEXT 3 ROWS ONLY;" + ) + assert cmd.parameters == ["[1.0, 0.0, 0.0]", "0", "value", "5"] + + +def test_dynamic_null_filter(filter_collection): + filter = default_dynamic_filter_function(parameters=[KernelParameterMetadata(name="tenant")], tenant=None) + clause = filter_collection._build_filter(filter) + + assert str(clause) == "[tenant] IS NULL" + assert clause.parameters == [] + + +def test_unsupported_is_operator(filter_collection): + with raises(NotImplementedError, match="Unsupported operator"): + filter_collection._build_filter("lambda x: x.tenant is 'test'")