From dfe9932ebd6a6e81acee5c1f733d7806baeb4f48 Mon Sep 17 00:00:00 2001 From: Claude Code Date: Thu, 29 Jan 2026 10:41:21 -0800 Subject: [PATCH 1/3] init --- .../2026-01-28-fix-boolean-tag-validation.md | 479 ++++++++++++++++++ .../boolean_tag_condition_processor.py | 88 ++++ .../R_eap_items/resolver_time_series.py | 10 +- 3 files changed, 571 insertions(+), 6 deletions(-) create mode 100644 docs/plans/2026-01-28-fix-boolean-tag-validation.md create mode 100644 snuba/query/processors/physical/boolean_tag_condition_processor.py diff --git a/docs/plans/2026-01-28-fix-boolean-tag-validation.md b/docs/plans/2026-01-28-fix-boolean-tag-validation.md new file mode 100644 index 0000000000..897097a9cc --- /dev/null +++ b/docs/plans/2026-01-28-fix-boolean-tag-validation.md @@ -0,0 +1,479 @@ +# Fix Boolean Tag Validation Implementation Plan + +> **For Claude:** REQUIRED SUB-SKILL: Use superpowers:executing-plans to implement this plan task-by-task. + +**Goal:** Fix validation error when boolean fields (0/1) are used in tag conditions by auto-converting integer literals to strings in TagConditionValidator. + +**Architecture:** Modify the TagConditionValidator to detect when integer literals 0 or 1 are used in tag comparisons and automatically convert them to their string equivalents ("0" or "1"). This handles the case where boolean search filters end up being classified as tags and passed with integer values instead of strings. + +**Tech Stack:** Python 3.11, pytest, Snuba query validation framework + +**Context:** Issue SNUBA-9VC reports validation failures like "invalid tag condition on 'tags[stack.in_app]': 1 must be a string". This occurs when boolean fields like `error.main_thread` are parsed as integers (0 or 1) but treated as tag conditions. Since tags are always strings in ClickHouse, we need to convert boolean integers to strings. + +--- + +## Task 1: Add test for boolean integer in tag condition (single value) + +**Files:** +- Modify: `tests/datasets/validation/test_tag_condition_checker.py:90-91` + +**Step 1: Write failing test for boolean integer tag condition** + +Add a new test case to the `tests` list (after line 90, before the closing bracket): + +```python + pytest.param( + LogicalQuery( + QueryEntity(EntityKey.EVENTS, get_entity(EntityKey.EVENTS).get_data_model()), + selected_columns=[ + SelectedExpression("time", Column("_snuba_timestamp", None, "timestamp")), + ], + condition=binary_condition( + "equals", + SubscriptableReference( + "_snuba_tags[error.main_thread]", + Column("_snuba_tags", None, "tags"), + Literal(None, "error.main_thread"), + ), + Literal(None, 1), + ), + ), + None, + id="boolean integer 1 in tag condition is auto-converted to string", + ), +``` + +**Step 2: Run test to verify it fails** + +Run: `pytest tests/datasets/validation/test_tag_condition_checker.py::test_subscription_clauses_validation -v -k "boolean integer 1"` + +Expected output: FAIL with error message like: +``` +InvalidQueryException: invalid tag condition on 'tags[error.main_thread]': 1 must be a string +``` + +This confirms the current behavior rejects integer 1 in tag conditions. + +**Step 3: Commit the failing test** + +```bash +git add tests/datasets/validation/test_tag_condition_checker.py +git commit -m "test: add failing test for boolean integer in tag condition + +Refs SNUBA-9VC" +``` + +--- + +## Task 2: Add test for boolean integer 0 in tag condition + +**Files:** +- Modify: `tests/datasets/validation/test_tag_condition_checker.py:110-111` + +**Step 1: Write failing test for boolean integer 0** + +Add another test case to the `tests` list (after the previous test): + +```python + pytest.param( + LogicalQuery( + QueryEntity(EntityKey.EVENTS, get_entity(EntityKey.EVENTS).get_data_model()), + selected_columns=[ + SelectedExpression("time", Column("_snuba_timestamp", None, "timestamp")), + ], + condition=binary_condition( + "equals", + SubscriptableReference( + "_snuba_tags[error.handled]", + Column("_snuba_tags", None, "tags"), + Literal(None, "error.handled"), + ), + Literal(None, 0), + ), + ), + None, + id="boolean integer 0 in tag condition is auto-converted to string", + ), +``` + +**Step 2: Run test to verify it fails** + +Run: `pytest tests/datasets/validation/test_tag_condition_checker.py::test_subscription_clauses_validation -v -k "boolean integer 0"` + +Expected output: FAIL with error: +``` +InvalidQueryException: invalid tag condition on 'tags[error.handled]': 0 must be a string +``` + +**Step 3: Commit the failing test** + +```bash +git add tests/datasets/validation/test_tag_condition_checker.py +git commit -m "test: add failing test for boolean integer 0 in tag condition + +Refs SNUBA-9VC" +``` + +--- + +## Task 3: Add test for boolean integers in array (IN clause) + +**Files:** +- Modify: `tests/datasets/validation/test_tag_condition_checker.py:130-131` + +**Step 1: Write failing test for boolean integers in array** + +Add test case for the IN operator with boolean values: + +```python + pytest.param( + LogicalQuery( + QueryEntity(EntityKey.EVENTS, get_entity(EntityKey.EVENTS).get_data_model()), + selected_columns=[ + SelectedExpression("time", Column("_snuba_timestamp", None, "timestamp")), + ], + condition=binary_condition( + "in", + SubscriptableReference( + "_snuba_tags[error.main_thread]", + Column("_snuba_tags", None, "tags"), + Literal(None, "error.main_thread"), + ), + FunctionCall( + None, + "array", + ( + Literal(None, 0), + Literal(None, 1), + ), + ), + ), + ), + None, + id="boolean integers in array are auto-converted to strings", + ), +``` + +**Step 2: Run test to verify it fails** + +Run: `pytest tests/datasets/validation/test_tag_condition_checker.py::test_subscription_clauses_validation -v -k "boolean integers in array"` + +Expected output: FAIL with error: +``` +InvalidQueryException: invalid tag condition on 'tags[error.main_thread]': array literal 0 must be a string +``` + +**Step 3: Commit the failing test** + +```bash +git add tests/datasets/validation/test_tag_condition_checker.py +git commit -m "test: add failing test for boolean integers in array tag condition + +Refs SNUBA-9VC" +``` + +--- + +## Task 4: Implement boolean-to-string conversion in TagConditionValidator + +**Files:** +- Modify: `snuba/query/validation/validators.py:339-365` + +**Step 1: Update validate method to convert boolean integers to strings** + +In the `TagConditionValidator.validate()` method, modify the validation logic to convert integers 0 and 1 to strings before validation. Replace the section from line 339 to 365: + +```python + def validate(self, query: Query, alias: Optional[str] = None) -> None: + condition = query.get_condition() + if not condition: + return + for cond in condition: + match = self.condition_matcher.match(cond) + if match: + column = match.expression("column") + col_str: str + if not isinstance(column, SubscriptableReferenceExpr): + return # only fail things on the tags[] column + + col_str = f"{column.column.column_name}[{column.key.value}]" + error_prefix = f"invalid tag condition on '{col_str}':" + + rhs = match.expression("rhs") + if isinstance(rhs, Literal): + # Auto-convert boolean integers (0, 1) to strings for tag comparisons + # Tags are always strings in ClickHouse, so boolean values must be "0" or "1" + if isinstance(rhs.value, int) and rhs.value in (0, 1): + # Modify the literal in place to convert int to string + rhs.value = str(rhs.value) + elif not isinstance(rhs.value, str): + raise InvalidQueryException(f"{error_prefix} {rhs.value} must be a string") + elif isinstance(rhs, FunctionCall): + # The rhs is guaranteed to be an array function because of the match + for param in rhs.parameters: + if isinstance(param, Literal): + # Auto-convert boolean integers in arrays + if isinstance(param.value, int) and param.value in (0, 1): + param.value = str(param.value) + elif not isinstance(param.value, str): + raise InvalidQueryException( + f"{error_prefix} array literal {param.value} must be a string" + ) +``` + +**Step 2: Run all tests to verify they pass** + +Run: `pytest tests/datasets/validation/test_tag_condition_checker.py::test_subscription_clauses_validation -v` + +Expected output: All tests PASS, including: +- ✓ boolean integer 1 in tag condition is auto-converted to string +- ✓ boolean integer 0 in tag condition is auto-converted to string +- ✓ boolean integers in array are auto-converted to strings +- ✓ comparing to non-string literal fails (for non-boolean integers like 419) + +**Step 3: Commit the implementation** + +```bash +git add snuba/query/validation/validators.py +git commit -m "fix: auto-convert boolean integers to strings in tag conditions + +Boolean search filters (like error.main_thread) are parsed with integer +values (0 or 1) but can be classified as tags when not in entity schema. +Since tags are always strings in ClickHouse, we now auto-convert integer +literals 0 and 1 to '0' and '1' in tag conditions. + +This allows queries like 'error.main_thread:1' to work correctly even +when the field is treated as a tag. + +Fixes SNUBA-9VC" +``` + +--- + +## Task 5: Add integration test with search_issues entity + +**Files:** +- Create: `tests/datasets/validation/test_search_issues_boolean_tags.py` + +**Step 1: Write integration test for search_issues entity** + +Create a new test file to verify the fix works with the actual search_issues entity: + +```python +from __future__ import annotations + +import pytest + +from snuba.datasets.entities.entity_key import EntityKey +from snuba.datasets.entities.factory import get_entity +from snuba.datasets.factory import reset_dataset_factory +from snuba.query import SelectedExpression +from snuba.query.conditions import binary_condition +from snuba.query.data_source.simple import Entity as QueryEntity +from snuba.query.expressions import ( + Column, + Literal, + SubscriptableReference, +) +from snuba.query.logical import Query as LogicalQuery +from snuba.query.validation.validators import TagConditionValidator + +reset_dataset_factory() + + +def test_search_issues_boolean_tag_validation() -> None: + """ + Test that boolean integer values in tag conditions work with search_issues entity. + + This simulates the real-world scenario from SNUBA-9VC where error.main_thread + and stack.in_app fields are treated as tags with integer values. + """ + entity = get_entity(EntityKey.SEARCH_ISSUES) + + # Query with error.main_thread:1 (boolean field as tag) + query = LogicalQuery( + QueryEntity(EntityKey.SEARCH_ISSUES, entity.get_data_model()), + selected_columns=[ + SelectedExpression("group_id", Column("_snuba_group_id", None, "group_id")), + ], + condition=binary_condition( + "and", + binary_condition( + "equals", + Column("_snuba_project_id", None, "project_id"), + Literal(None, 123), + ), + binary_condition( + "equals", + SubscriptableReference( + "_snuba_tags[error.main_thread]", + Column("_snuba_tags", None, "tags"), + Literal(None, "error.main_thread"), + ), + Literal(None, 1), # Boolean integer value + ), + ), + ) + + # Validator should not raise an exception + validator = TagConditionValidator() + validator.validate(query) # Should pass without error + + # Verify the literal was converted to string + condition = query.get_condition() + assert condition is not None + + +def test_search_issues_stack_in_app_tag() -> None: + """ + Test the specific case from the error message: tags[stack.in_app] + """ + entity = get_entity(EntityKey.SEARCH_ISSUES) + + query = LogicalQuery( + QueryEntity(EntityKey.SEARCH_ISSUES, entity.get_data_model()), + selected_columns=[ + SelectedExpression("group_id", Column("_snuba_group_id", None, "group_id")), + ], + condition=binary_condition( + "and", + binary_condition( + "equals", + Column("_snuba_project_id", None, "project_id"), + Literal(None, 456), + ), + binary_condition( + "equals", + SubscriptableReference( + "_snuba_tags[stack.in_app]", + Column("_snuba_tags", None, "tags"), + Literal(None, "stack.in_app"), + ), + Literal(None, 1), # The problematic integer from SNUBA-9VC + ), + ), + ) + + validator = TagConditionValidator() + validator.validate(query) # Should pass after fix +``` + +**Step 2: Run integration test** + +Run: `pytest tests/datasets/validation/test_search_issues_boolean_tags.py -v` + +Expected output: Both tests PASS: +- ✓ test_search_issues_boolean_tag_validation +- ✓ test_search_issues_stack_in_app_tag + +**Step 3: Commit the integration test** + +```bash +git add tests/datasets/validation/test_search_issues_boolean_tags.py +git commit -m "test: add integration tests for boolean tags in search_issues + +Verify that the TagConditionValidator fix works correctly with the +search_issues entity for real-world scenarios like error.main_thread +and stack.in_app. + +Refs SNUBA-9VC" +``` + +--- + +## Task 6: Run full test suite and verify no regressions + +**Files:** +- N/A (verification only) + +**Step 1: Run the full validation test suite** + +Run: `pytest tests/datasets/validation/ -v` + +Expected output: All tests PASS with no failures or regressions. + +**Step 2: Run validator-related tests across the codebase** + +Run: `pytest -k validator -v` + +Expected output: All tests PASS. The change should not break any existing validator tests. + +**Step 3: Document results** + +If all tests pass, proceed to final commit. If any tests fail, investigate and fix before proceeding. + +--- + +## Task 7: Create PR and close issue + +**Files:** +- N/A (PR creation) + +**Step 1: Push branch to remote** + +```bash +git push -u origin HEAD +``` + +**Step 2: Create pull request** + +Use the GitHub CLI to create a PR: + +```bash +gh pr create \ + --title "fix: auto-convert boolean integers to strings in tag conditions" \ + --body "$(cat <<'EOF' +## Summary +Fixes validation error when boolean fields (0/1) are used in tag conditions by auto-converting integer literals to strings in `TagConditionValidator`. + +## Problem +Issue SNUBA-9VC reported validation failures like: +``` +Validation failed for entity search_issues: invalid tag condition on 'tags[stack.in_app]': 1 must be a string +``` + +This occurs when boolean search filters (like `error.main_thread:1`) are: +1. Parsed with integer values (0 or 1) +2. Classified as tags (when not in entity schema) +3. Sent to Snuba with integer literals in tag conditions + +Since tags are always strings in ClickHouse, the `TagConditionValidator` rejected these integer values. + +## Solution +Modified `TagConditionValidator` to automatically convert integer literals `0` and `1` to their string equivalents `"0"` and `"1"` when they appear in tag conditions. This handles both: +- Single value comparisons: `tags[error.main_thread] = 1` → `tags[error.main_thread] = "1"` +- Array comparisons: `tags[field] IN [0, 1]` → `tags[field] IN ["0", "1"]` + +Non-boolean integers (like 419) still fail validation as expected. + +## Testing +- Added unit tests for boolean integer conversion in tag conditions +- Added integration tests with `search_issues` entity +- Verified no regressions in existing validator tests + +Fixes SNUBA-9VC + +🤖 Generated with [Claude Code](https://claude.com/claude-code) +EOF +)" +``` + +**Step 3: Verify PR created successfully** + +Expected output: PR URL like `https://github.com/getsentry/snuba/pull/XXXX` + +The PR should automatically reference and close SNUBA-9VC when merged. + +--- + +## Summary + +This plan fixes SNUBA-9VC by modifying the `TagConditionValidator` to automatically convert boolean integer literals (0 and 1) to strings when used in tag conditions. The fix: + +- ✅ Handles the immediate issue with minimal code changes +- ✅ Provides a safety net for all boolean tag comparisons +- ✅ Maintains backwards compatibility +- ✅ Includes comprehensive test coverage +- ✅ Does not require schema migrations + +The solution is surgical, well-tested, and addresses the root cause of the validation error while maintaining strict validation for non-boolean integer values. diff --git a/snuba/query/processors/physical/boolean_tag_condition_processor.py b/snuba/query/processors/physical/boolean_tag_condition_processor.py new file mode 100644 index 0000000000..624439567b --- /dev/null +++ b/snuba/query/processors/physical/boolean_tag_condition_processor.py @@ -0,0 +1,88 @@ +from snuba.clickhouse.query import Query +from snuba.query.conditions import ConditionFunctions, build_match +from snuba.query.expressions import Expression, Literal, SubscriptableReference +from snuba.query.processors.physical import ClickhouseQueryProcessor +from snuba.query.query_settings import QuerySettings + + +class BooleanTagConditionProcessor(ClickhouseQueryProcessor): + """ + Converts boolean integer literals (0 and 1) to string literals ("0" and "1") + when used in tag conditions. This provides a defensive fix for queries that + incorrectly use integers in tag comparisons (e.g., `tags[stack.in_app] = 1`). + + The root cause is typically that Sentry treats boolean fields like `stack.in_app` + as tags instead of proper columns, converting boolean values to integers (0 or 1). + Snuba's TagConditionValidator enforces string-only tag values, so we convert + these integers to strings to avoid validation errors. + + This processor should run before TagConditionValidator. + """ + + def __init__(self, column_name: str) -> None: + self.column_name = column_name + # Build matchers for tag conditions with various operators + self.condition_matchers = [ + build_match( + subscriptable=column_name, + ops=[op], + ) + for op in [ + ConditionFunctions.EQ, + ConditionFunctions.NEQ, + ConditionFunctions.LTE, + ConditionFunctions.GTE, + ConditionFunctions.LT, + ConditionFunctions.GT, + ConditionFunctions.LIKE, + ConditionFunctions.NOT_LIKE, + ] + ] + self.array_condition_matchers = [ + build_match( + subscriptable=column_name, + array_ops=[op], + ) + for op in [ConditionFunctions.IN, ConditionFunctions.NOT_IN] + ] + + def process_query(self, query: Query, query_settings: QuerySettings) -> None: + def process_condition(exp: Expression) -> Expression: + # Check if this is a tag condition + for matcher in self.condition_matchers: + result = matcher.match(exp) + if result: + column = result.expression("column") + if isinstance(column, SubscriptableReference): + rhs = result.expression("rhs") + if isinstance(rhs, Literal) and isinstance(rhs.value, int): + # Convert boolean integers (0 or 1) to strings + if rhs.value in (0, 1): + return exp.transform( + lambda e: Literal(e.alias, str(e.value)) + if isinstance(e, Literal) + and isinstance(e.value, int) + and e.value in (0, 1) + else e + ) + return exp + + # Check array conditions (IN, NOT IN) + for matcher in self.array_condition_matchers: + result = matcher.match(exp) + if result: + column = result.expression("column") + if isinstance(column, SubscriptableReference): + # Transform the expression to convert any 0 or 1 literals to strings + return exp.transform( + lambda e: Literal(e.alias, str(e.value)) + if isinstance(e, Literal) + and isinstance(e.value, int) + and e.value in (0, 1) + else e + ) + return exp + + condition = query.get_condition() + if condition is not None: + query.set_ast_condition(condition.transform(process_condition)) diff --git a/snuba/web/rpc/v1/resolvers/R_eap_items/resolver_time_series.py b/snuba/web/rpc/v1/resolvers/R_eap_items/resolver_time_series.py index 097c98479e..674aa9fdf2 100644 --- a/snuba/web/rpc/v1/resolvers/R_eap_items/resolver_time_series.py +++ b/snuba/web/rpc/v1/resolvers/R_eap_items/resolver_time_series.py @@ -469,12 +469,10 @@ def resolve( # if the user passes it in assert len(in_msg.aggregations) == 0 - # This metric is expected to be 0, aggregation is deprecated and should be converted to - # conditional aggregation. This metric is to verify before deprecating in the protobuf. - # @kylemumma 01/27/2026 - for expr in in_msg.expressions: - if expr.WhichOneof("expression") == "aggregation": - self._metrics_backend.increment("aggregation_expression") + # aggregation is deprecated, it gets converted to conditional_aggregation + if state.get_int_config("aggregation_deprecation_enabled", 1): + for expr in in_msg.expressions: + assert expr.WhichOneof("expression") != "aggregation" query_settings = setup_trace_query_settings() if in_msg.meta.debug else HTTPQuerySettings() try: From 3e19de2bd175e9fdfe84b0a55c5ce991a0063893 Mon Sep 17 00:00:00 2001 From: Claude Code Date: Thu, 29 Jan 2026 10:44:03 -0800 Subject: [PATCH 2/3] rmove --- .../2026-01-28-fix-boolean-tag-validation.md | 479 ------------------ .../boolean_tag_condition_processor.py | 88 ---- 2 files changed, 567 deletions(-) delete mode 100644 docs/plans/2026-01-28-fix-boolean-tag-validation.md delete mode 100644 snuba/query/processors/physical/boolean_tag_condition_processor.py diff --git a/docs/plans/2026-01-28-fix-boolean-tag-validation.md b/docs/plans/2026-01-28-fix-boolean-tag-validation.md deleted file mode 100644 index 897097a9cc..0000000000 --- a/docs/plans/2026-01-28-fix-boolean-tag-validation.md +++ /dev/null @@ -1,479 +0,0 @@ -# Fix Boolean Tag Validation Implementation Plan - -> **For Claude:** REQUIRED SUB-SKILL: Use superpowers:executing-plans to implement this plan task-by-task. - -**Goal:** Fix validation error when boolean fields (0/1) are used in tag conditions by auto-converting integer literals to strings in TagConditionValidator. - -**Architecture:** Modify the TagConditionValidator to detect when integer literals 0 or 1 are used in tag comparisons and automatically convert them to their string equivalents ("0" or "1"). This handles the case where boolean search filters end up being classified as tags and passed with integer values instead of strings. - -**Tech Stack:** Python 3.11, pytest, Snuba query validation framework - -**Context:** Issue SNUBA-9VC reports validation failures like "invalid tag condition on 'tags[stack.in_app]': 1 must be a string". This occurs when boolean fields like `error.main_thread` are parsed as integers (0 or 1) but treated as tag conditions. Since tags are always strings in ClickHouse, we need to convert boolean integers to strings. - ---- - -## Task 1: Add test for boolean integer in tag condition (single value) - -**Files:** -- Modify: `tests/datasets/validation/test_tag_condition_checker.py:90-91` - -**Step 1: Write failing test for boolean integer tag condition** - -Add a new test case to the `tests` list (after line 90, before the closing bracket): - -```python - pytest.param( - LogicalQuery( - QueryEntity(EntityKey.EVENTS, get_entity(EntityKey.EVENTS).get_data_model()), - selected_columns=[ - SelectedExpression("time", Column("_snuba_timestamp", None, "timestamp")), - ], - condition=binary_condition( - "equals", - SubscriptableReference( - "_snuba_tags[error.main_thread]", - Column("_snuba_tags", None, "tags"), - Literal(None, "error.main_thread"), - ), - Literal(None, 1), - ), - ), - None, - id="boolean integer 1 in tag condition is auto-converted to string", - ), -``` - -**Step 2: Run test to verify it fails** - -Run: `pytest tests/datasets/validation/test_tag_condition_checker.py::test_subscription_clauses_validation -v -k "boolean integer 1"` - -Expected output: FAIL with error message like: -``` -InvalidQueryException: invalid tag condition on 'tags[error.main_thread]': 1 must be a string -``` - -This confirms the current behavior rejects integer 1 in tag conditions. - -**Step 3: Commit the failing test** - -```bash -git add tests/datasets/validation/test_tag_condition_checker.py -git commit -m "test: add failing test for boolean integer in tag condition - -Refs SNUBA-9VC" -``` - ---- - -## Task 2: Add test for boolean integer 0 in tag condition - -**Files:** -- Modify: `tests/datasets/validation/test_tag_condition_checker.py:110-111` - -**Step 1: Write failing test for boolean integer 0** - -Add another test case to the `tests` list (after the previous test): - -```python - pytest.param( - LogicalQuery( - QueryEntity(EntityKey.EVENTS, get_entity(EntityKey.EVENTS).get_data_model()), - selected_columns=[ - SelectedExpression("time", Column("_snuba_timestamp", None, "timestamp")), - ], - condition=binary_condition( - "equals", - SubscriptableReference( - "_snuba_tags[error.handled]", - Column("_snuba_tags", None, "tags"), - Literal(None, "error.handled"), - ), - Literal(None, 0), - ), - ), - None, - id="boolean integer 0 in tag condition is auto-converted to string", - ), -``` - -**Step 2: Run test to verify it fails** - -Run: `pytest tests/datasets/validation/test_tag_condition_checker.py::test_subscription_clauses_validation -v -k "boolean integer 0"` - -Expected output: FAIL with error: -``` -InvalidQueryException: invalid tag condition on 'tags[error.handled]': 0 must be a string -``` - -**Step 3: Commit the failing test** - -```bash -git add tests/datasets/validation/test_tag_condition_checker.py -git commit -m "test: add failing test for boolean integer 0 in tag condition - -Refs SNUBA-9VC" -``` - ---- - -## Task 3: Add test for boolean integers in array (IN clause) - -**Files:** -- Modify: `tests/datasets/validation/test_tag_condition_checker.py:130-131` - -**Step 1: Write failing test for boolean integers in array** - -Add test case for the IN operator with boolean values: - -```python - pytest.param( - LogicalQuery( - QueryEntity(EntityKey.EVENTS, get_entity(EntityKey.EVENTS).get_data_model()), - selected_columns=[ - SelectedExpression("time", Column("_snuba_timestamp", None, "timestamp")), - ], - condition=binary_condition( - "in", - SubscriptableReference( - "_snuba_tags[error.main_thread]", - Column("_snuba_tags", None, "tags"), - Literal(None, "error.main_thread"), - ), - FunctionCall( - None, - "array", - ( - Literal(None, 0), - Literal(None, 1), - ), - ), - ), - ), - None, - id="boolean integers in array are auto-converted to strings", - ), -``` - -**Step 2: Run test to verify it fails** - -Run: `pytest tests/datasets/validation/test_tag_condition_checker.py::test_subscription_clauses_validation -v -k "boolean integers in array"` - -Expected output: FAIL with error: -``` -InvalidQueryException: invalid tag condition on 'tags[error.main_thread]': array literal 0 must be a string -``` - -**Step 3: Commit the failing test** - -```bash -git add tests/datasets/validation/test_tag_condition_checker.py -git commit -m "test: add failing test for boolean integers in array tag condition - -Refs SNUBA-9VC" -``` - ---- - -## Task 4: Implement boolean-to-string conversion in TagConditionValidator - -**Files:** -- Modify: `snuba/query/validation/validators.py:339-365` - -**Step 1: Update validate method to convert boolean integers to strings** - -In the `TagConditionValidator.validate()` method, modify the validation logic to convert integers 0 and 1 to strings before validation. Replace the section from line 339 to 365: - -```python - def validate(self, query: Query, alias: Optional[str] = None) -> None: - condition = query.get_condition() - if not condition: - return - for cond in condition: - match = self.condition_matcher.match(cond) - if match: - column = match.expression("column") - col_str: str - if not isinstance(column, SubscriptableReferenceExpr): - return # only fail things on the tags[] column - - col_str = f"{column.column.column_name}[{column.key.value}]" - error_prefix = f"invalid tag condition on '{col_str}':" - - rhs = match.expression("rhs") - if isinstance(rhs, Literal): - # Auto-convert boolean integers (0, 1) to strings for tag comparisons - # Tags are always strings in ClickHouse, so boolean values must be "0" or "1" - if isinstance(rhs.value, int) and rhs.value in (0, 1): - # Modify the literal in place to convert int to string - rhs.value = str(rhs.value) - elif not isinstance(rhs.value, str): - raise InvalidQueryException(f"{error_prefix} {rhs.value} must be a string") - elif isinstance(rhs, FunctionCall): - # The rhs is guaranteed to be an array function because of the match - for param in rhs.parameters: - if isinstance(param, Literal): - # Auto-convert boolean integers in arrays - if isinstance(param.value, int) and param.value in (0, 1): - param.value = str(param.value) - elif not isinstance(param.value, str): - raise InvalidQueryException( - f"{error_prefix} array literal {param.value} must be a string" - ) -``` - -**Step 2: Run all tests to verify they pass** - -Run: `pytest tests/datasets/validation/test_tag_condition_checker.py::test_subscription_clauses_validation -v` - -Expected output: All tests PASS, including: -- ✓ boolean integer 1 in tag condition is auto-converted to string -- ✓ boolean integer 0 in tag condition is auto-converted to string -- ✓ boolean integers in array are auto-converted to strings -- ✓ comparing to non-string literal fails (for non-boolean integers like 419) - -**Step 3: Commit the implementation** - -```bash -git add snuba/query/validation/validators.py -git commit -m "fix: auto-convert boolean integers to strings in tag conditions - -Boolean search filters (like error.main_thread) are parsed with integer -values (0 or 1) but can be classified as tags when not in entity schema. -Since tags are always strings in ClickHouse, we now auto-convert integer -literals 0 and 1 to '0' and '1' in tag conditions. - -This allows queries like 'error.main_thread:1' to work correctly even -when the field is treated as a tag. - -Fixes SNUBA-9VC" -``` - ---- - -## Task 5: Add integration test with search_issues entity - -**Files:** -- Create: `tests/datasets/validation/test_search_issues_boolean_tags.py` - -**Step 1: Write integration test for search_issues entity** - -Create a new test file to verify the fix works with the actual search_issues entity: - -```python -from __future__ import annotations - -import pytest - -from snuba.datasets.entities.entity_key import EntityKey -from snuba.datasets.entities.factory import get_entity -from snuba.datasets.factory import reset_dataset_factory -from snuba.query import SelectedExpression -from snuba.query.conditions import binary_condition -from snuba.query.data_source.simple import Entity as QueryEntity -from snuba.query.expressions import ( - Column, - Literal, - SubscriptableReference, -) -from snuba.query.logical import Query as LogicalQuery -from snuba.query.validation.validators import TagConditionValidator - -reset_dataset_factory() - - -def test_search_issues_boolean_tag_validation() -> None: - """ - Test that boolean integer values in tag conditions work with search_issues entity. - - This simulates the real-world scenario from SNUBA-9VC where error.main_thread - and stack.in_app fields are treated as tags with integer values. - """ - entity = get_entity(EntityKey.SEARCH_ISSUES) - - # Query with error.main_thread:1 (boolean field as tag) - query = LogicalQuery( - QueryEntity(EntityKey.SEARCH_ISSUES, entity.get_data_model()), - selected_columns=[ - SelectedExpression("group_id", Column("_snuba_group_id", None, "group_id")), - ], - condition=binary_condition( - "and", - binary_condition( - "equals", - Column("_snuba_project_id", None, "project_id"), - Literal(None, 123), - ), - binary_condition( - "equals", - SubscriptableReference( - "_snuba_tags[error.main_thread]", - Column("_snuba_tags", None, "tags"), - Literal(None, "error.main_thread"), - ), - Literal(None, 1), # Boolean integer value - ), - ), - ) - - # Validator should not raise an exception - validator = TagConditionValidator() - validator.validate(query) # Should pass without error - - # Verify the literal was converted to string - condition = query.get_condition() - assert condition is not None - - -def test_search_issues_stack_in_app_tag() -> None: - """ - Test the specific case from the error message: tags[stack.in_app] - """ - entity = get_entity(EntityKey.SEARCH_ISSUES) - - query = LogicalQuery( - QueryEntity(EntityKey.SEARCH_ISSUES, entity.get_data_model()), - selected_columns=[ - SelectedExpression("group_id", Column("_snuba_group_id", None, "group_id")), - ], - condition=binary_condition( - "and", - binary_condition( - "equals", - Column("_snuba_project_id", None, "project_id"), - Literal(None, 456), - ), - binary_condition( - "equals", - SubscriptableReference( - "_snuba_tags[stack.in_app]", - Column("_snuba_tags", None, "tags"), - Literal(None, "stack.in_app"), - ), - Literal(None, 1), # The problematic integer from SNUBA-9VC - ), - ), - ) - - validator = TagConditionValidator() - validator.validate(query) # Should pass after fix -``` - -**Step 2: Run integration test** - -Run: `pytest tests/datasets/validation/test_search_issues_boolean_tags.py -v` - -Expected output: Both tests PASS: -- ✓ test_search_issues_boolean_tag_validation -- ✓ test_search_issues_stack_in_app_tag - -**Step 3: Commit the integration test** - -```bash -git add tests/datasets/validation/test_search_issues_boolean_tags.py -git commit -m "test: add integration tests for boolean tags in search_issues - -Verify that the TagConditionValidator fix works correctly with the -search_issues entity for real-world scenarios like error.main_thread -and stack.in_app. - -Refs SNUBA-9VC" -``` - ---- - -## Task 6: Run full test suite and verify no regressions - -**Files:** -- N/A (verification only) - -**Step 1: Run the full validation test suite** - -Run: `pytest tests/datasets/validation/ -v` - -Expected output: All tests PASS with no failures or regressions. - -**Step 2: Run validator-related tests across the codebase** - -Run: `pytest -k validator -v` - -Expected output: All tests PASS. The change should not break any existing validator tests. - -**Step 3: Document results** - -If all tests pass, proceed to final commit. If any tests fail, investigate and fix before proceeding. - ---- - -## Task 7: Create PR and close issue - -**Files:** -- N/A (PR creation) - -**Step 1: Push branch to remote** - -```bash -git push -u origin HEAD -``` - -**Step 2: Create pull request** - -Use the GitHub CLI to create a PR: - -```bash -gh pr create \ - --title "fix: auto-convert boolean integers to strings in tag conditions" \ - --body "$(cat <<'EOF' -## Summary -Fixes validation error when boolean fields (0/1) are used in tag conditions by auto-converting integer literals to strings in `TagConditionValidator`. - -## Problem -Issue SNUBA-9VC reported validation failures like: -``` -Validation failed for entity search_issues: invalid tag condition on 'tags[stack.in_app]': 1 must be a string -``` - -This occurs when boolean search filters (like `error.main_thread:1`) are: -1. Parsed with integer values (0 or 1) -2. Classified as tags (when not in entity schema) -3. Sent to Snuba with integer literals in tag conditions - -Since tags are always strings in ClickHouse, the `TagConditionValidator` rejected these integer values. - -## Solution -Modified `TagConditionValidator` to automatically convert integer literals `0` and `1` to their string equivalents `"0"` and `"1"` when they appear in tag conditions. This handles both: -- Single value comparisons: `tags[error.main_thread] = 1` → `tags[error.main_thread] = "1"` -- Array comparisons: `tags[field] IN [0, 1]` → `tags[field] IN ["0", "1"]` - -Non-boolean integers (like 419) still fail validation as expected. - -## Testing -- Added unit tests for boolean integer conversion in tag conditions -- Added integration tests with `search_issues` entity -- Verified no regressions in existing validator tests - -Fixes SNUBA-9VC - -🤖 Generated with [Claude Code](https://claude.com/claude-code) -EOF -)" -``` - -**Step 3: Verify PR created successfully** - -Expected output: PR URL like `https://github.com/getsentry/snuba/pull/XXXX` - -The PR should automatically reference and close SNUBA-9VC when merged. - ---- - -## Summary - -This plan fixes SNUBA-9VC by modifying the `TagConditionValidator` to automatically convert boolean integer literals (0 and 1) to strings when used in tag conditions. The fix: - -- ✅ Handles the immediate issue with minimal code changes -- ✅ Provides a safety net for all boolean tag comparisons -- ✅ Maintains backwards compatibility -- ✅ Includes comprehensive test coverage -- ✅ Does not require schema migrations - -The solution is surgical, well-tested, and addresses the root cause of the validation error while maintaining strict validation for non-boolean integer values. diff --git a/snuba/query/processors/physical/boolean_tag_condition_processor.py b/snuba/query/processors/physical/boolean_tag_condition_processor.py deleted file mode 100644 index 624439567b..0000000000 --- a/snuba/query/processors/physical/boolean_tag_condition_processor.py +++ /dev/null @@ -1,88 +0,0 @@ -from snuba.clickhouse.query import Query -from snuba.query.conditions import ConditionFunctions, build_match -from snuba.query.expressions import Expression, Literal, SubscriptableReference -from snuba.query.processors.physical import ClickhouseQueryProcessor -from snuba.query.query_settings import QuerySettings - - -class BooleanTagConditionProcessor(ClickhouseQueryProcessor): - """ - Converts boolean integer literals (0 and 1) to string literals ("0" and "1") - when used in tag conditions. This provides a defensive fix for queries that - incorrectly use integers in tag comparisons (e.g., `tags[stack.in_app] = 1`). - - The root cause is typically that Sentry treats boolean fields like `stack.in_app` - as tags instead of proper columns, converting boolean values to integers (0 or 1). - Snuba's TagConditionValidator enforces string-only tag values, so we convert - these integers to strings to avoid validation errors. - - This processor should run before TagConditionValidator. - """ - - def __init__(self, column_name: str) -> None: - self.column_name = column_name - # Build matchers for tag conditions with various operators - self.condition_matchers = [ - build_match( - subscriptable=column_name, - ops=[op], - ) - for op in [ - ConditionFunctions.EQ, - ConditionFunctions.NEQ, - ConditionFunctions.LTE, - ConditionFunctions.GTE, - ConditionFunctions.LT, - ConditionFunctions.GT, - ConditionFunctions.LIKE, - ConditionFunctions.NOT_LIKE, - ] - ] - self.array_condition_matchers = [ - build_match( - subscriptable=column_name, - array_ops=[op], - ) - for op in [ConditionFunctions.IN, ConditionFunctions.NOT_IN] - ] - - def process_query(self, query: Query, query_settings: QuerySettings) -> None: - def process_condition(exp: Expression) -> Expression: - # Check if this is a tag condition - for matcher in self.condition_matchers: - result = matcher.match(exp) - if result: - column = result.expression("column") - if isinstance(column, SubscriptableReference): - rhs = result.expression("rhs") - if isinstance(rhs, Literal) and isinstance(rhs.value, int): - # Convert boolean integers (0 or 1) to strings - if rhs.value in (0, 1): - return exp.transform( - lambda e: Literal(e.alias, str(e.value)) - if isinstance(e, Literal) - and isinstance(e.value, int) - and e.value in (0, 1) - else e - ) - return exp - - # Check array conditions (IN, NOT IN) - for matcher in self.array_condition_matchers: - result = matcher.match(exp) - if result: - column = result.expression("column") - if isinstance(column, SubscriptableReference): - # Transform the expression to convert any 0 or 1 literals to strings - return exp.transform( - lambda e: Literal(e.alias, str(e.value)) - if isinstance(e, Literal) - and isinstance(e.value, int) - and e.value in (0, 1) - else e - ) - return exp - - condition = query.get_condition() - if condition is not None: - query.set_ast_condition(condition.transform(process_condition)) From 0a4bd8be3cb7b3f07e58488579cecd71872c23c8 Mon Sep 17 00:00:00 2001 From: Claude Code Date: Thu, 29 Jan 2026 11:37:09 -0800 Subject: [PATCH 3/3] runtime --- .../web/rpc/v1/resolvers/R_eap_items/resolver_time_series.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/snuba/web/rpc/v1/resolvers/R_eap_items/resolver_time_series.py b/snuba/web/rpc/v1/resolvers/R_eap_items/resolver_time_series.py index 674aa9fdf2..aa46559a9f 100644 --- a/snuba/web/rpc/v1/resolvers/R_eap_items/resolver_time_series.py +++ b/snuba/web/rpc/v1/resolvers/R_eap_items/resolver_time_series.py @@ -472,7 +472,10 @@ def resolve( # aggregation is deprecated, it gets converted to conditional_aggregation if state.get_int_config("aggregation_deprecation_enabled", 1): for expr in in_msg.expressions: - assert expr.WhichOneof("expression") != "aggregation" + if expr.WhichOneof("expression") == "aggregation": + raise RuntimeError( + "Unexpected state: aggregation is deprecated and should have been converted to conditional_aggregation" + ) query_settings = setup_trace_query_settings() if in_msg.meta.debug else HTTPQuerySettings() try: