feat(eap): semver sort key for sentry.release ORDER BY in EAP#8103
Open
phacops wants to merge 10 commits into
Open
feat(eap): semver sort key for sentry.release ORDER BY in EAP#8103phacops wants to merge 10 commits into
phacops wants to merge 10 commits into
Conversation
Fixes lexicographic ordering of release strings by wrapping `sentry.release` in a `tuple(arrayResize(arrayMap(...), 4), is_stable)` sort key when used in ORDER BY / GROUP BY in EAP trace item table queries. This ensures: - 1.2.9 sorts before 1.2.10 (numeric, not lexicographic) - 1.2.3-beta.1 sorts before 1.2.3 (prerelease before stable) - 1.2.3-beta.1 sorts after 1.2.2 (prerelease of newer > older stable) - 1.2 and 1.2.0 are equal (arrayResize pads to 4 components) - package@version strips the prefix before comparison Uses only ClickHouse functions available on Altinity 25.3/25.8 (no naturalSortKey which requires upstream 26.3+). Co-Authored-By: Claude Opus 4.8 <[email protected]> Claude-Session: https://claude.ai/code/session_013TCwndq43WTDRf6jsERQHN
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
Two fixes for the semver sort key implementation: 1. Replace f.tuple(..., alias=alias) with FunctionCall(alias, "tuple", ...) to fix mypy "Optional[str] not compatible with str" error on the alias argument. 2. Only apply semver_sort_key in the ORDER BY path, not GROUP BY. GROUP BY sentry.release must use the raw attribute expression so that the SELECT (also the raw string) is a function of the GROUP BY key and ClickHouse accepts the aggregation query. ORDER BY on a function of a GROUP BY key is valid in ClickHouse, so semver ordering still works. This also fixes the 500 error in Sentry's test_semver_build test where the spans events API auto-groups by release. Co-Authored-By: Claude Opus 4.8 <[email protected]> Claude-Session: https://claude.ai/code/session_013TCwndq43WTDRf6jsERQHN
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 4be96cd. Configure here.
…rison The flextime pagination filter was comparing the raw release string lexicographically, while ORDER BY now uses the semver tuple key. This caused rows to be skipped or repeated on follow-up pages when `sentry.release` was in the order clause (e.g. "1.2.3-beta.1" would be missed because it sorts after "1.2.3" lexicographically but before it semantically). Fix: move `SEMVER_SORT_ATTRIBUTES` to common.py so pagination.py can import it. In `get_filters`, wrap both the column reference and the stored literal value in `semver_sort_key(...)` for any semver attribute so the page-boundary `<` comparison uses the same ordering as ORDER BY. Co-Authored-By: Claude Opus 4.8 <[email protected]> Claude-Session: https://claude.ai/code/session_013TCwndq43WTDRf6jsERQHN
sentry.release is coalesced from multiple attribute columns (attributes_string_25['sentry.release'] + attributes_string_30['release']), which makes the expression Nullable(String). ClickHouse forbids Nullable(Array(...)), so splitByChar on the raw expression would throw: Code: 43 – Nested type Array(String) cannot be inside Nullable type Fix: apply ifNull(expr, '') at the start of semver_sort_key to convert Nullable(String) to String before any string→array operations. Co-Authored-By: Claude Opus 4.8 <[email protected]> Claude-Session: https://claude.ai/code/session_013TCwndq43WTDRf6jsERQHN
"1.2" and "1.2.0" normalise to the same sort key ([1,2,0,0], 1). ClickHouse may return them in either relative order within the tied pair, making the strict `asc == list(reversed(desc))` assertion non-deterministic. Collapse contiguous tied elements into a frozenset before comparing so the test is order-insensitive within ties while still verifying that every other element is in exact reverse order. Co-Authored-By: Claude Opus 4.8 <[email protected]> Claude-Session: https://claude.ai/code/session_013TCwndq43WTDRf6jsERQHN
Resolve conflict in resolver_trace_item_table.py: - Keep master's updated _groupby_order_by_expression docstring (TYPE_ARRAY note) plus the semver paragraph. - Keep master's inlined _convert_order_by structure and use_array_map_columns aggregation arg while preserving for_order_by=True so SEMVER_SORT_ATTRIBUTES still get the semver tuple sort key in ORDER BY. Co-Authored-By: Claude Opus 4.8 <[email protected]> Claude-Session: https://claude.ai/code/session_013TCwndq43WTDRf6jsERQHN
Master's ruff config enables flake8-bugbear; B905 requires an explicit strict= argument on zip(). column_names and column_values are built in lockstep so strict=True is correct (they always have equal length). Co-Authored-By: Claude Opus 4.8 <[email protected]> Claude-Session: https://claude.ai/code/session_013TCwndq43WTDRf6jsERQHN
Resolve conflict in test_endpoint_trace_item_table.py: both branches appended independent test code at end of file. Keep both the TestSemverSorting class and master's test_uniq_with_default_value_double_casts_to_float64. Co-Authored-By: Claude Opus 4.8 <[email protected]> Claude-Session: https://claude.ai/code/session_013TCwndq43WTDRf6jsERQHN
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes incorrect lexicographic ordering of
sentry.releasestrings when used inORDER BY(andGROUP BY) in EAP trace item table queries.Bugs fixed:
1.2.10was sorting before1.2.9(classic lex bug)1.2.3-beta.1was sorting after1.2.3(pre-release should come before stable)1.2and1.2.0were not equal (length mismatch)Approach: Wrap
sentry.releasein atuple(arrayResize(arrayMap(x -> toUInt32OrZero(x), splitByChar('.', release_part)), 4), is_stable)sort key when it appears inORDER BY/GROUP BY. Uses only ClickHouse functions available on Altinity 25.3/25.8 — nonaturalSortKey(requires upstream 26.3+) and noCOLLATE(fails pre-release ordering, needsDangerousRawSQL).Key correctness properties:
1.2.9before1.2.10(numeric component comparison) ✓1.2.3-beta.1before1.2.3(pre-release sorts before stable) ✓1.2.3-beta.1after1.2.2(pre-release of newer > older stable) ✓1.2==1.2.0(arrayResize pads to 4 components) ✓[email protected]strips the@-prefix before comparison ✓25.3.8.999before25.3.8.10041✓Changes
snuba/web/rpc/common/common.pysemver_sort_key()helper and_SEMVER_COMPONENT_COUNTconstantsnuba/web/rpc/v1/resolvers/R_eap_items/resolver_trace_item_table.py_SEMVER_SORT_ATTRIBUTES; extend_groupby_order_by_expression()to wrapsentry.releaseinsemver_sort_key()tests/web/rpc/test_common.pysemver_sort_key()expression structuretests/web/rpc/v1/test_endpoint_trace_item_table/test_endpoint_trace_item_table.pyTest plan
TestSemverSortKey— checks expression shape (no DB required)TestSemverSorting— writes spans with differentsentry.releasevalues, queries withORDER BY sentry.releaseASC/DESC and asserts:1.2.9before1.2.101.2.3-beta.1before1.2.31.2.3-beta.1after1.2.2[email protected]sorts as version 2.0.01.2and1.2.0are adjacent (equal sort key)🤖 Generated with Claude Code
https://claude.ai/code/session_013TCwndq43WTDRf6jsERQHN
Generated by Claude Code