diff --git a/src/sentry/api/event_search.py b/src/sentry/api/event_search.py index f1bb45bcfbd5ed..adbf655756f23b 100644 --- a/src/sentry/api/event_search.py +++ b/src/sentry/api/event_search.py @@ -79,6 +79,7 @@ / has_in_filter / has_filter / is_filter + / array_includes_numeric_filter / array_includes_filter / text_in_filter / text_filter @@ -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. @@ -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 @@ -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): @@ -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 diff --git a/src/sentry/search/eap/resolver.py b/src/sentry/search/eap/resolver.py index 8f55ce68460d89..b818ab98698938 100644 --- a/src/sentry/search/eap/resolver.py +++ b/src/sentry/search/eap/resolver.py @@ -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( diff --git a/tests/sentry/api/test_event_search.py b/tests/sentry/api/test_event_search.py index 357d7614301dee..6419d8d0809d5c 100644 --- a/tests/sentry/api/test_event_search.py +++ b/tests/sentry/api/test_event_search.py @@ -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) diff --git a/tests/snuba/api/endpoints/test_organization_events_occurrences.py b/tests/snuba/api/endpoints/test_organization_events_occurrences.py index 985c4d943bac49..de2d35c045d689 100644 --- a/tests/snuba/api/endpoints/test_organization_events_occurrences.py +++ b/tests/snuba/api/endpoints/test_organization_events_occurrences.py @@ -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, @@ -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]]: @@ -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] @@ -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][*]: None: # Both occurrences share the same tag-array fixture: # tags[array_tags] = ["eap_items", "occurrences"]