Summary
SearchFilter.append_to_statement (sqlspec/core/filters.py:714-744) handles field_name: str and field_name: set[str], but its else branch silently returns the unmodified statement:
def append_to_statement(self, statement):
...
if isinstance(self.field_name, str):
result = statement.where(...)
elif isinstance(self.field_name, set) and self.field_name:
...
else:
result = statement # ← silent drop, no warning, no error
A user passing the wrong type (e.g. an exp.Expression because they assumed expression input was supported, or None, or anything else) gets no filter at all, with no diagnostic. The query runs, returns more rows than expected, and the bug is invisible until someone notices the data is wrong.
Reproduction
from sqlglot import exp
from sqlspec import sql
from sqlspec.core.filters import SearchFilter
stmt = sql.select(\"id\", \"name\").from_(\"things\").to_statement()
# Pass an Expression (a reasonable mistake — there's no type-checker enforcement)
sf = SearchFilter(field_name=exp.func(\"LOWER\", exp.column(\"name\")), value=\"foo\")
stmt = sf.append_to_statement(stmt)
stmt.compile()
print(stmt.sql)
# → SELECT \"things\".\"id\" AS \"id\", \"things\".\"name\" AS \"name\" FROM \"things\" AS \"things\"
# ↑ NO WHERE CLAUSE — filter silently dropped
Expected
The else branch should raise TypeError describing the supported types, e.g.:
else:
raise TypeError(
f\"SearchFilter.field_name must be str | set[str], got {type(self.field_name).__name__}\"
)
Audit
Other field-name-bearing filters (NotInSearchFilter, etc.) should be checked for the same pattern.
Notes
Summary
SearchFilter.append_to_statement(sqlspec/core/filters.py:714-744) handlesfield_name: strandfield_name: set[str], but itselsebranch silently returns the unmodified statement:A user passing the wrong type (e.g. an
exp.Expressionbecause they assumed expression input was supported, orNone, or anything else) gets no filter at all, with no diagnostic. The query runs, returns more rows than expected, and the bug is invisible until someone notices the data is wrong.Reproduction
Expected
The else branch should
raise TypeErrordescribing the supported types, e.g.:Audit
Other field-name-bearing filters (
NotInSearchFilter, etc.) should be checked for the same pattern.Notes
str | exp.Expressionproposed in Feature: OrderByFilter accepts sqlglot expressions for sort key (e.g. COALESCE) #427 would naturally fix this case, but the underlying "silent drop on unknown type" pattern should be raised regardless of any feature work — it's a correctness bug today.