Skip to content

Commit 198104b

Browse files
committed
Reject boolean values in numeric context, catch dangling OR
- Add bool guard in _build_condition and _convert_to_numeric so WHERE price = true raises ValueError instead of producing invalid @price:[True True] syntax. - Expand OR detection regex to match leading/trailing OR (e.g. 'laptop OR', 'OR tablet') so the empty-operand check catches these malformed inputs instead of silently dropping OR as a stopword.
1 parent a99cfdf commit 198104b

4 files changed

Lines changed: 44 additions & 9 deletions

File tree

sql_redis/query_builder.py

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -175,16 +175,19 @@ def build_text_condition(
175175

176176
escaped = self._escape_text_value(" ".join(phrase_words))
177177
search_value = f'"{escaped}"'
178-
elif re.search(r"\s+OR\s+", value):
178+
elif re.search(r"(?:^|\s+)OR(?:\s+|$)", value):
179179
# OR union within text field: split on uppercase-only OR with
180180
# flexible whitespace, escape each term, join with |.
181181
# Only uppercase OR is treated as a boolean operator; lowercase
182182
# "or" is treated as a regular search term (e.g. "bank or america"
183183
# stays as a multi-word AND search, not bank|america).
184184
# Multi-word operands (e.g. "gaming laptop OR tablet") are wrapped
185185
# in parentheses so each side is an atomic subexpression.
186+
# The regex also matches leading/trailing OR (e.g. "laptop OR"
187+
# or "OR tablet") so that the empty-operand check below catches
188+
# these malformed inputs instead of silently dropping "OR".
186189
or_parts: list[str] = []
187-
for part in re.split(r"\s+OR\s+", value):
190+
for part in re.split(r"(?:^|\s+)OR(?:\s+|$)", value):
188191
words = part.strip().split()
189192
if not words:
190193
raise ValueError(

sql_redis/translator.py

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -7,13 +7,9 @@
77
from dataclasses import dataclass, field
88

99
from sql_redis.analyzer import AnalyzedQuery, Analyzer
10-
from sql_redis.parser import (
11-
SQL_TO_REDIS_DATE_FUNCTIONS,
12-
Condition,
13-
GeoDistanceCondition,
14-
SQLParser,
15-
parse_date_to_timestamp,
16-
)
10+
from sql_redis.parser import (SQL_TO_REDIS_DATE_FUNCTIONS, Condition,
11+
GeoDistanceCondition, SQLParser,
12+
parse_date_to_timestamp)
1713
from sql_redis.query_builder import QueryBuilder
1814
from sql_redis.schema import AsyncSchemaRegistry, SchemaRegistry
1915

@@ -247,6 +243,12 @@ def _build_condition(self, condition: Condition, field_type: str | None) -> str:
247243
low_val = self._convert_to_numeric(low)
248244
high_val = self._convert_to_numeric(high)
249245
numeric_value = (low_val, high_val)
246+
elif isinstance(condition.value, bool):
247+
raise ValueError(
248+
f"Boolean value {condition.value!r} is not valid in a "
249+
"numeric context. Use 1/0 instead of true/false for "
250+
"numeric fields."
251+
)
250252
elif isinstance(condition.value, (int, float)):
251253
numeric_value = condition.value
252254
else:
@@ -278,6 +280,11 @@ def _convert_to_numeric(self, value: object) -> int | float:
278280
Raises:
279281
ValueError: If conversion fails.
280282
"""
283+
if isinstance(value, bool):
284+
raise ValueError(
285+
f"Boolean value {value!r} is not valid in a numeric context. "
286+
"Use 1/0 instead of true/false for numeric fields."
287+
)
281288
if isinstance(value, (int, float)):
282289
return value
283290
if isinstance(value, str):

tests/test_query_builder.py

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -679,6 +679,24 @@ def test_or_extra_whitespace(self):
679679
)
680680
assert result == "@title:(laptop|tablet)"
681681

682+
def test_or_trailing_raises(self):
683+
"""Trailing OR with no operand raises ValueError."""
684+
builder = QueryBuilder()
685+
with pytest.raises(ValueError, match="Empty operand"):
686+
builder.build_text_condition("title", "FULLTEXT", "laptop OR")
687+
688+
def test_or_leading_raises(self):
689+
"""Leading OR with no operand raises ValueError."""
690+
builder = QueryBuilder()
691+
with pytest.raises(ValueError, match="Empty operand"):
692+
builder.build_text_condition("title", "FULLTEXT", "OR tablet")
693+
694+
def test_or_only_raises(self):
695+
"""Bare 'OR' with no operands raises ValueError."""
696+
builder = QueryBuilder()
697+
with pytest.raises(ValueError, match="Empty operand"):
698+
builder.build_text_condition("title", "FULLTEXT", "OR")
699+
682700
def test_escape_asterisk_in_fulltext(self):
683701
"""Literal * in FULLTEXT is escaped to prevent wildcard."""
684702
builder = QueryBuilder()

tests/test_translator.py

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -213,6 +213,13 @@ def test_or_conditions(self, translator: Translator, basic_index: str):
213213

214214
assert "|" in result.query_string # OR uses pipe
215215

216+
def test_boolean_in_numeric_context_raises(
217+
self, translator: Translator, basic_index: str
218+
):
219+
"""WHERE price = true should raise, not produce @price:[True True]."""
220+
with pytest.raises(ValueError, match="Boolean value"):
221+
translator.translate(f"SELECT * FROM {basic_index} WHERE price = true")
222+
216223

217224
class TestTranslatorAggregate:
218225
"""Tests for FT.AGGREGATE translation."""

0 commit comments

Comments
 (0)