Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 26 additions & 3 deletions src/sentry/api/event_search.py
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@
/ has_in_filter
/ has_filter
/ is_filter
/ array_includes_numeric_filter
/ array_includes_filter
/ text_in_filter
/ text_filter
Expand Down Expand Up @@ -184,7 +185,8 @@
array_includes_attr_key = (key/ quoted_key) array_includes_suffix

array_includes_key = array_includes_attr_key / array_includes_tag_key
array_includes_filter = negation? array_includes_key sep wildcard_op? operator? search_value
array_includes_numeric_filter = negation? array_includes_key sep comparison_operator numeric_value
array_includes_filter = negation? array_includes_key sep wildcard_op? operator? search_value

# NOTE: These wildcard operators are internal implementation details and
# should not be included in product docs. Users should use `*` instead.
Expand Down Expand Up @@ -215,6 +217,7 @@

# NOTE: the order in which these operators are listed matters because for
# example, if < comes before <= it will match that even if the operator is <=
comparison_operator = ">=" / "<=" / ">" / "<"
operator = ">=" / "<=" / ">" / "<" / "=" / "!="
or_operator = ~r"OR"i &end_value
and_operator = ~r"AND"i &end_value
Expand Down Expand Up @@ -1929,8 +1932,6 @@ def visit_array_includes_filter(
if not search_value.raw_value:
raise InvalidSearchQuery(f"Empty value for {search_key.name}[*]")

if operator_s not in ("=", "!=") and not node.children[5].text:
raise InvalidSearchQuery("In Array Queries, only EQUAL/NOT_EQUAL operators are allowed")
operator_s = handle_negation(negation, operator_s)

if has_wildcard_op(wildcard_op) and isinstance(search_value.raw_value, str):
Expand All @@ -1940,6 +1941,28 @@ def visit_array_includes_filter(
search_value = search_value._replace(raw_value=wildcard_value)
return SearchFilter(search_key, operator_s, search_value)

def visit_comparison_operator(self, node: Node, children: object) -> str:
return node.text

def visit_array_includes_numeric_filter(
self,
node: Node,
children: tuple[
Node | tuple[Node], # ! if present
SearchKey,
Node, # :
str, # comparison_operator
tuple[str, str], # numeric_value: (digits, suffix)
],
) -> SearchFilter:
(negation, search_key, _, operator, raw_search_value) = children
operator_s = handle_negation(negation, operator)
try:
numeric_value = parse_numeric_value(*raw_search_value)
except InvalidQuery as exc:
raise InvalidSearchQuery(str(exc))
return SearchFilter(search_key, operator_s, SearchValue(numeric_value))

def generic_visit(self, node: Node, children: Sequence[Any]) -> Any:
return children or node

Expand Down
18 changes: 16 additions & 2 deletions src/sentry/search/eap/resolver.py
Original file line number Diff line number Diff line change
Expand Up @@ -915,13 +915,27 @@ def _resolve_search_value(
return AttributeValue(val_bool=value)
elif column_type == constants.ARRAY:
# Only scalar value membership in an array is allowed.
# Allowed operators: =,!=, LIKE, NOT_LIKE.
# TODO: Add support for scalar: >, < for numbers
if operator in constants.IN_OPERATORS:
raise InvalidSearchQuery(
f"{column.public_alias} (array) cannot be used with an IN filter; "
f"use {column.public_alias}[*]:value for membership"
)
if operator in (">", "<", ">=", "<="):
# value must be number.
element_type = constants.TYPE_MAP.get(column.search_type)
if element_type not in (constants.INT, constants.DOUBLE):
raise InvalidSearchQuery(
f"{column.public_alias} ({column.search_type}) does not support "
"comparison operators (>, <, >=, <=); use =, !="
)
if not isinstance(value, (int, float)):
raise InvalidSearchQuery(
f"{value} is not valid numeric value for {column.public_alias}"
)
if element_type == constants.INT:
return AttributeValue(val_int=int(value))
return AttributeValue(val_double=float(value))
# For operators: =,!=, LIKE, NOT_LIKE (value can be string values)
# All primitive types are converted to strings on EAP before comparison.
return AttributeValue(val_str=str(value))
raise InvalidSearchQuery(
Expand Down
44 changes: 21 additions & 23 deletions tests/sentry/api/test_event_search.py
Original file line number Diff line number Diff line change
Expand Up @@ -1634,31 +1634,29 @@ def test_array_includes_filter_rejects_invalid_input(query) -> None:


@pytest.mark.parametrize(
"query,expected",
"query,expected_operator,expected_value",
[
pytest.param("stack.colno[*]:>5", ">", 5.0, id="greater_than_numeric_float"),
pytest.param("stack.colno[*]:<=0", "<=", 0.0, id="less_than_or_equal_numeric_float"),
pytest.param("stack.colno[*]:>=3", ">=", 3.0, id="greater_than_or_equal_numeric_float"),
pytest.param("stack.colno[*]:<7", "<", 7.0, id="less_than_numeric_float"),
pytest.param("stack.colno[*]:>500k", ">", 500_000.0, id="comparison_with_k_suffix"),
pytest.param("!stack.colno[*]:>5", "<=", 5.0, id="negated_greater_than_flips_to_le"),
pytest.param("stack.colno[*]:=5", "=", "5", id="equals_numeric_stays_string"),
pytest.param("stack.colno[*]:!=5", "!=", "5", id="not_equals_numeric_stays_string"),
pytest.param("stack.colno[*]:5", "=", "5", id="no_operator_defaults_to_equals_string"),
pytest.param(
"stack.colno[*]:>5",
[
SearchFilter(
key=SearchKey(name="stack.colno"),
operator=">",
value=SearchValue("5"),
)
],
id="greater_than",
),
pytest.param(
"stack.colno[*]:<=0",
[
SearchFilter(
key=SearchKey(name="stack.colno"),
operator="<=",
value=SearchValue("0"),
)
],
id="less_than_or_equal",
"frame_filenames[*]:>foo", ">", "foo", id="non_numeric_value_falls_back_to_string"
),
],
)
def test_array_includes_filter_passes_through_comparison_operators(query, expected) -> None:
assert parse_search_query(query) == expected
def test_array_includes_filter_grammar_split(
query: str, expected_operator: str, expected_value: float | str
) -> None:
filters = parse_search_query(query)
assert len(filters) == 1
filter_obj = filters[0]
assert isinstance(filter_obj, SearchFilter)
assert filter_obj.operator == expected_operator
assert filter_obj.value.raw_value == expected_value
assert type(filter_obj.value.raw_value) is type(expected_value)
Original file line number Diff line number Diff line change
Expand Up @@ -517,7 +517,7 @@ class OrganizationEventsOccurrencesArrayQueryTest(
):
callsite_name = "api.events.endpoints"

def request_with_feature_flag(self, payload: dict) -> Response:
def request_with_feature_flag(self, payload: dict, assert_status_code: bool = True) -> Response:
# Array attributes are behind `organizations:trace-item-details-array-fields`.
features = {
"organizations:discover-basic": True,
Expand All @@ -527,7 +527,8 @@ def request_with_feature_flag(self, payload: dict) -> Response:
{EAPOccurrencesComparator._callsite_allowlist_option_name(): self.callsite_name}
):
response = self.do_request({**payload, "dataset": "occurrences"}, features=features)
assert response.status_code == 200, response.content
if assert_status_code:
assert response.status_code == 200, response.content
return response

def _create_occurrence_with_arrays(self, occurrence_count: int = 1) -> tuple[list, list[dict]]:
Expand Down Expand Up @@ -713,12 +714,6 @@ def test_array_includes_substring_matching(self) -> None:
f"substring {substring!r} present={is_present}, expected present={must_contain}"
)

@pytest.mark.xfail(
reason=(
"EAP TYPE_ARRAY ComparisonFilter only accepts EQUALS / NOT_EQUALS / LIKE / NOT_LIKE. "
"Comparison operators (>, <, >=, <=) are wired through Sentry but rejected by Snuba."
),
)
def test_array_includes_comparison_operators_for_numbers(self) -> None:
# i=0 colnos: [12, 0]
# i=1 colnos: [13, 1]
Expand All @@ -743,6 +738,35 @@ def test_array_includes_comparison_operators_for_numbers(self) -> None:
f"{description}: query={query!r} returned {len(data)} rows, expected {expected_count}"
)

def test_array_includes_comparison_operators_rejected_for_non_numeric(self) -> None:
# Comparison operators (>, <, >=, <=) should fail on non-numerical value types.
self._create_occurrence_with_arrays(occurrence_count=1)
cases = [
# (description, query)
("string-element array", "stack.filename[*]:>foo"),
("boolean-element array", "stack.in_app[*]:>=true"),
("tag string-element array", "tags[array_tags, array][*]:<bar"),
]
for description, query in cases:
response = self.request_with_feature_flag(
{
"field": ["id"],
"query": query,
"statsPeriod": "1h",
"project": [self.project.id],
},
assert_status_code=False,
)
assert response.status_code == 400, (
f"{description}: query={query!r} returned status {response.status_code}, "
f"expected 400"
)
detail = response.data.get("detail", "") if hasattr(response, "data") else ""
assert "comparison operators" in str(detail), (
f"{description}: query={query!r} returned detail={detail!r}, "
f"expected message mentioning 'comparison operators'"
)

def test_array_includes_equality_and_inequality_for_tag_types(self) -> None:
# Both occurrences share the same tag-array fixture:
# tags[array_tags] = ["eap_items", "occurrences"]
Expand Down
Loading