diff --git a/src/utils/filter.py b/src/utils/filter.py index 3d035e06..6af13dfd 100644 --- a/src/utils/filter.py +++ b/src/utils/filter.py @@ -79,6 +79,35 @@ SCALAR_OPERAND_TYPES = ( ) +def _coerce_numeric(op_value: Any) -> int | float | Decimal: + """Validate a numeric operand without losing precision. + + float() rounds an int past 2**53 and flattens a Decimal, so an operand that + is already numeric is passed through untouched and only strings are parsed. + int() is tried before float() so "5" stays exact while "5.5" still parses. + bool is narrowed to int: it is an int subclass, but binding it as a boolean + against a numeric column produces SQL Postgres has no operator for. + + Args: + op_value: The operand to validate. + + Returns: + The operand as an exact numeric value. + + Raises: + ValueError, TypeError: If the operand is not numeric. Callers convert + these to FilterError. + """ + if isinstance(op_value, bool): + return int(op_value) + if isinstance(op_value, int | float | Decimal): + return op_value + try: + return int(op_value) + except ValueError: + return float(op_value) + + def _require_bindable_operand( column_name: str, op_value: Any, operator: str = "" ) -> None: @@ -670,7 +699,7 @@ def _build_comparison_conditions( # 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) + casted_value = _coerce_numeric(op_value) except (TypeError, ValueError): raise FilterError( f"Invalid numeric value: {op_value}. Expected a number, got {type(op_value).__name__}" diff --git a/tests/utils/test_filter.py b/tests/utils/test_filter.py index ac10e53e..bb578fa1 100644 --- a/tests/utils/test_filter.py +++ b/tests/utils/test_filter.py @@ -35,7 +35,34 @@ 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) + # Assert on the WHERE clause specifically: internal_metadata is in the + # SELECT projection either way, so checking the whole statement passes even + # when no condition was applied at all. + assert stmt.whereclause is not None + assert "internal_metadata" in str(stmt.whereclause) + compiled = stmt.compile(dialect=postgresql.dialect()) + assert {"kind": "note"} in [bind.value for bind in compiled.binds.values()] + + +def test_numeric_operand_keeps_integer_precision(): + """float() rounds anything past 2**53, silently shifting the comparison.""" + big = 2**53 + 1 + stmt = apply_filter(select(Message), Message, {"token_count": {"gt": big}}) + compiled = stmt.compile(dialect=postgresql.dialect()) + assert big in [bind.value for bind in compiled.binds.values()] + + +def test_fractional_operand_on_integer_column_is_not_truncated(): + """Coercing to the column's int type would turn `lt 5.5` into `lt 5`.""" + stmt = apply_filter(select(Message), Message, {"token_count": {"lt": 5.5}}) + compiled = stmt.compile(dialect=postgresql.dialect()) + assert 5.5 in [bind.value for bind in compiled.binds.values()] + + +def test_numeric_string_operand_stays_exact(): + stmt = apply_filter(select(Message), Message, {"token_count": {"gt": "5"}}) + compiled = stmt.compile(dialect=postgresql.dialect()) + assert 5 in [bind.value for bind in compiled.binds.values()] def test_ne_none_on_scalar_column_is_not_null():