diff --git a/src/utils/filter.py b/src/utils/filter.py index e7d1764f..850fb70d 100644 --- a/src/utils/filter.py +++ b/src/utils/filter.py @@ -1,5 +1,6 @@ import datetime from collections.abc import Callable, Sequence +from decimal import Decimal from logging import getLogger from typing import Any, TypeVar from typing import cast as typing_cast @@ -566,6 +567,12 @@ def _build_comparison_conditions( column.type.python_type, datetime.datetime ) + # Numeric coercion applies only to actually-numeric columns. On a text + # column, `ne` is a string comparison, not a failed float parse. + is_numeric_column = hasattr(column.type, "python_type") and issubclass( + column.type.python_type, int | float | Decimal + ) + for operator, op_value in comparisons.items(): # Validate that the operator is supported if operator not in COMPARISON_OPERATORS: @@ -575,6 +582,17 @@ def _build_comparison_conditions( if op_value == "*": continue + # A null operand is a null check, not a value comparison, on every + # column type. Only `ne` is meaningful: {"col": None} already covers + # IS NULL via the equality path in _build_field_condition. + if op_value is None: + if operator != "ne": + raise FilterError( + f"Operator '{operator}' does not accept null. Use {{\"ne\": null}} for a not-null check, or null on its own for a null check." + ) + conditions.append(column.is_not(None)) + continue + condition = None # For datetime columns, cast string values to timestamp @@ -588,11 +606,12 @@ def _build_comparison_conditions( # Use the validated datetime object directly instead of string interpolation casted_value = validated_datetime else: - # if the operator is a numeric operator, the value must cast to a number - if operator in NUMERIC_OPERATORS: + # On a numeric column, a numeric operator's value must cast to a + # number. On a text column, `ne` is a string comparison. + if operator in NUMERIC_OPERATORS and is_numeric_column: try: casted_value = float(op_value) - except ValueError: + except (TypeError, ValueError): raise FilterError( f"Invalid numeric value: {op_value}. Expected a number, got {type(op_value).__name__}" ) from None diff --git a/tests/utils/test_filter.py b/tests/utils/test_filter.py index 49f84e23..140023c1 100644 --- a/tests/utils/test_filter.py +++ b/tests/utils/test_filter.py @@ -4,7 +4,7 @@ import pytest from sqlalchemy import select from src.exceptions import FilterError -from src.models import Document +from src.models import Document, Message from src.utils.filter import apply_filter @@ -29,3 +29,32 @@ def test_known_operator_dict_on_scalar_column_still_works(): def test_dict_on_jsonb_column_still_works(): stmt = apply_filter(select(Document), Document, {"metadata": {"kind": "note"}}) assert "internal_metadata" in str(stmt) + + +def test_ne_none_on_scalar_column_is_not_null(): + """Regression: float(None) raised TypeError, which the ValueError handler + missed -> unhandled 500.""" + stmt = apply_filter(select(Document), Document, {"session_id": {"ne": None}}) + assert "session_name IS NOT NULL" in str(stmt) + + +def test_ne_string_on_text_column_compares_as_string(): + """Regression: numeric operators float()-cast on every column type, so a + string inequality on a text column was rejected as a bad number.""" + stmt = apply_filter(select(Document), Document, {"session_id": {"ne": "abc"}}) + assert "session_name !=" in str(stmt) + + +def test_numeric_operator_still_validates_on_numeric_column(): + with pytest.raises(FilterError): + apply_filter(select(Message), Message, {"token_count": {"gt": "nope"}}) + + +def test_ne_none_on_numeric_column_is_not_null(): + stmt = apply_filter(select(Message), Message, {"token_count": {"ne": None}}) + assert "token_count IS NOT NULL" in str(stmt) + + +def test_null_operand_on_non_ne_operator_raises(): + with pytest.raises(FilterError): + apply_filter(select(Message), Message, {"token_count": {"gt": None}})