feat(eap-items): read and filter array attributes natively from typed columns#8110
Conversation
… path Past the typed-column cutoff (use_array_map_columns), array attributes are now returned natively from the typed attributes_array_* map columns instead of the legacy attributes_array JSON column — no toJSONString round-trip, which is the point of storing arrays as typed Map(String, Array(T)) columns. trace-item table SELECT: a TYPE_ARRAY key reads the typed columns as a native tuple(string[], int[], float[], bool[]); the converter flattens it into a typed val_array (str/int/double/bool), instead of toJSONString over the JSON column. trace-item details: drops the hardcoded attributes_array allowlist and selects the four typed array maps whole, emitting an attribute per key — so every array attribute is returned (the allowlist existed only to avoid materializing the dynamic JSON value, which typed columns make unnecessary). The four maps are merged by attribute name. Pre-cutoff data has no typed columns populated, so both paths fall back to the JSON column (toJSONString / allowlist) as before. Homogeneous arrays (the common case) keep element order; a mixed-type array's elements are grouped by type, since the typed columns store each element type separately. get_trace and export_trace_items share the same allowlist and will get the same treatment in a follow-up. The filter/WHERE read path is unchanged from #8101. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018AH9MHDdP2k6S52KpQvLo6
066643d to
c40e7fc
Compare
…ch SELECT _column_to_expression selects a TYPE_ARRAY attribute via the native typed-column tuple past the cutoff, but _groupby_order_by_expression still produced the legacy toJSONString form. When an array attribute is used as a group-by or order-by key, the SELECT and GROUP BY expressions diverged, which ClickHouse rejects with "Column ... is not under aggregate function and not in GROUP BY". Thread read_arrays_from_typed_columns(use_array_map_columns(meta)) through _groupby_order_by_expression at both call sites so the expressions match. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018AH9MHDdP2k6S52KpQvLo6
…cutoff The typed array columns are double-written, so the same row read with the typed read path (query window on/after the cutoff) and the legacy JSON read path (before it) must agree. Toggle use_array_map_columns_timestamp_seconds (0 = JSON path, low = typed path) over one written row and assert identical val_array: - trace-item table SELECT: a homogeneous string and int array decode to the same val_array both ways. - trace-item details: an allowlisted array attribute is identical both ways; past the cutoff a NON-allowlisted array attribute is also returned, proving the allowlist is dropped on the typed-column read path. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018AH9MHDdP2k6S52KpQvLo6
Mirror the trace-item table/details migration in the two remaining representation endpoints. Past the cutoff (use_array_map_columns): - get_trace: per-attribute SELECTs read the typed columns (read path threaded through attribute_key_to_expression); the bulk "all attributes" mode reads the four typed array maps whole instead of the attributes_array JSON-column allowlist, merging them by name. The per-attribute tuple is flattened to a native typed array. - export_trace_items: same allowlist -> typed-maps switch in the bulk select and conversion. Pre-cutoff data still uses the JSON column (allowlist / toJSONString). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018AH9MHDdP2k6S52KpQvLo6
…ey]) Select array attributes from the typed attributes_array_* map columns using the same SubscriptableReference (col[key]) access as the other attribute map columns, instead of arrayElement. A missing key reads as an empty array, so these references are excluded from the NULL existence-check wrap (add_existence_check_to_subscriptable_references) — ClickHouse has no Nullable(Array), so wrapping them would be illegal. The result is unchanged: a native tuple of per-type arrays, flattened to a typed val_array. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018AH9MHDdP2k6S52KpQvLo6
…cketed) Read the typed attributes_array_* map columns with arrayElement(col, key) — the access pattern used for the other non-bucketed map columns (attributes_int / attributes_bool), not the SubscriptableReference form used for the bucketed attributes_string / attributes_float. This reverts the SubscriptableReference approach (and the related existence-check exclusion it required): non-bucketed maps are read directly with arrayElement, which is also why they never need the NULL existence-check wrap. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018AH9MHDdP2k6S52KpQvLo6
…ry window ExportTraceItems adjusts the queried time window via the routing decision (_export_query_meta), and the WHERE clause uses that adjusted window. The typed array column choice (SELECT) and decode must use the same window, otherwise a request whose original start is post-cutoff but whose routed window is pre-cutoff would select the typed columns over a range where they are unpopulated and silently drop array attributes. Gate both _build_query and _convert_rows on the routing-adjusted meta instead of in_msg.meta. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018AH9MHDdP2k6S52KpQvLo6
…race get_trace's _process_results only treated a per-attribute typed array column as a tuple when it was a Python tuple; if the driver delivers the ClickHouse Tuple(string[], int[], float[], bool[]) as a list it fell through to the generic branch and was encoded as a nested array-of-arrays. Accept both tuple and length-4 list past the cutoff, consistent with flatten_typed_array_tuple and the trace-item table converter. Adds a parametrized unit test for both shapes. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018AH9MHDdP2k6S52KpQvLo6
…lumns Replace the tuple(string[], int[], float[], bool[]) SELECT representation of an array attribute read from the typed attributes_array_* map columns with four separate native arrayElement sub-columns, merged back into one typed val_array in application code. Arrays are homogeneous in production (every element shares one type, living in a single typed column), so exactly one sub-column is non-empty; the merge concatenates them in column order, defensively handling mixed-type arrays too. - protos/common: drop type_array_to_typed_columns_select_expression / the read_arrays_from_typed_columns flag on attribute_key_to_expression; add type_array_typed_columns_select_expressions returning the four arrayElement sub-column expressions (aliased <label_mapping_key>.<column> so SELECT and GROUP BY / ORDER BY agree). - common: add typed_array_select_subcolumn_name / merge_typed_array_subcolumns; drop flatten_typed_array_tuple. - TraceItemTable resolver: emit the four sub-columns in the SELECT and expand GROUP BY / ORDER BY of a TYPE_ARRAY key to the same four expressions; merge them back into the column label in convert_results (no per-converter array flag). - GetTrace: per-attribute array reads emit the four sub-columns and merge them by attribute name in _process_results. Tests: cover the sub-column merge (homogeneous + mixed) for GetTrace and the table converter; make the shared i_am_an_array fixture homogeneous (matching the production invariant) and expect array attributes from every endpoint past the cutoff (export, get_trace bulk). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018AH9MHDdP2k6S52KpQvLo6
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018AH9MHDdP2k6S52KpQvLo6
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
There are 2 total unresolved issues (including 1 from previous review).
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 1a6145f. Configure here.
TYPED_ARRAY_SELECT_COLUMNS (protos/common) and TYPED_ARRAY_MAP_COLUMNS (web/rpc/common) listed the same four typed array map columns in the same order. Keep a single TYPED_ARRAY_MAP_COLUMNS in the base protos.common module and import it everywhere (common, table resolver, get_trace). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018AH9MHDdP2k6S52KpQvLo6
Array attributes now support SELECT only. Filtering, group-by, order-by, and aggregations on array attributes are rejected with a clear error, and the supporting machinery is removed: - Filtering: revert the array-membership path (#8101) — drop type_array_to_membership_array_expression[_from_typed_columns], the array branches in trace_item_filters_to_expression, the array filter validators, and the now-dead use_array_map_columns filter parameter threaded through trace_item_filters_to_expression / aggregation_to_expression / the get_*_column helpers and all their callers. Array comparison/exists filters now raise. - Aggregations: _resolve_field_and_existence rejects TYPE_ARRAY keys instead of reading the JSON column. - group-by / order-by: rejected in TraceItemTable request validation; the group-by/order-by expression builder no longer special-cases arrays. SELECT is unchanged and still reads both the legacy attributes_array JSON column (allowlist, pre-cutoff) and the typed attributes_array_* map columns (past the cutoff). Tests for the removed array filter/agg/group/order paths are replaced with rejection tests. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018AH9MHDdP2k6S52KpQvLo6
…by/order-by Reverts the array-filtering removal from the previous "select-only" change: filtering on array attributes (the #8101 array-membership path) is kept, since getsentry/sentry actively uses and tests it (`<array_field>[*]:value`). Native typed-column array SELECT is unchanged. Array attributes now support SELECT and filtering, but NOT: - aggregations: _resolve_field_and_existence rejects TYPE_ARRAY keys, and the now-dead _array_aggregation_to_expression dispatch is removed. - group-by / order-by: rejected in TraceItemTable request validation; the group-by/order-by expression builder no longer special-cases arrays. Tests: restore the array-filter tests; update the array-aggregation tests to expect rejection; add TestArrayOperationsRejected for group-by/order-by/aggregation. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018AH9MHDdP2k6S52KpQvLo6
When filtering an array attribute against a scalar value past the cutoff, read only the typed attributes_array_* column(s) that can hold a value of that type instead of arrayConcat-ing all four: string -> attributes_array_string, number -> attributes_array_int + attributes_array_float (the query value type doesn't always match the stored int/float element type), bool -> attributes_array_bool. The per-element comparison is unchanged (still normalized to string), so matching is identical — we just avoid scanning irrelevant columns. Exists filters have no value, so they still read all four columns. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018AH9MHDdP2k6S52KpQvLo6
…value Sentry's EAP query builder sends every array-membership filter value as val_str, and the filter key only carries TYPE_ARRAY (never the element type), so the typed attributes_array_* column to read can't be picked from the value type. The value-type narrowing reverted here read the string column for a numeric value (e.g. stack.colno[*]:12) and matched 0 rows. Instead coerce the val_str to each native type it parses as and OR a native arrayExists per matching column: numeric strings search the int and float columns, true/false the bool column, and every string the string column. Matching uses each column's native type rather than stringifying the stored elements. LIKE patterns read only the string column. Exists filters are unchanged (notEmpty over all four columns). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018AH9MHDdP2k6S52KpQvLo6
Past the cutoff a TYPE_ARRAY column is read as four typed sub-columns; an absent attribute reads as four empty sub-columns. Return NULL for it (like a missing scalar attribute) instead of an empty list so it serializes as is_null. The typed columns can't distinguish an absent array from a stored-empty one, so both map to NULL. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018AH9MHDdP2k6S52KpQvLo6
The conditional-aggregation array-filter test still asserted all four typed columns. With per-column native matching a LIKE pattern can only match string elements, so the filter reads just attributes_array_string. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018AH9MHDdP2k6S52KpQvLo6
Rejecting all array aggregations broke crash-free user rate, which uses FUNCTION_UNIQ over the user_ids array attribute (test_crash_free_user_rate_ with_array_attributes, pre-existing on master). Restore master's behavior: uniq is supported via uniqArrayIfOrNull over the legacy attributes_array JSON path (the typed-column read path is unaffected); all other aggregations on arrays remain rejected. group_by / order_by on arrays stay rejected. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018AH9MHDdP2k6S52KpQvLo6
onewland
left a comment
There was a problem hiding this comment.
I wasn't around for the discussion and implementation of separating types out in arrays, but this change seems fine and safe/quick to revert if we find a problem.
Will we be able to tell quickly if the change is broken?
| def merge_typed_array_subcolumns( | ||
| row: dict[str, Any], bases: Iterable[str] | ||
| ) -> list[tuple[str, list[Any]]]: | ||
| """Pop the four typed sub-columns of each ``base`` array attribute and merge them into | ||
| ``(base, elements)`` pairs (per-attribute counterpart of ``merge_typed_array_maps``). | ||
| Arrays are homogeneous, so one sub-column is non-empty; the four are concatenated in | ||
| column order.""" | ||
| merged: list[tuple[str, list[Any]]] = [] | ||
| for base in bases: | ||
| elements: list[Any] = [] | ||
| for typed_col in TYPED_ARRAY_MAP_COLUMNS: | ||
| values = row.pop(typed_array_select_subcolumn_name(base, typed_col), None) | ||
| if values: | ||
| elements.extend(values) | ||
| merged.append((base, elements)) | ||
| return merged |
There was a problem hiding this comment.
have we considered (and rejected) not doing this for our upstream clients?
I understand right now that we allow them to treat arrays as heterogeneous, and we need to do this mapping for a transition, but it would be nice if we could drop all this pre/post-processing later
There was a problem hiding this comment.
It's accepted we're not going to support heterogeneous arrays. The problem I'm trying to dance around here is we don't have a good type system for array attributes.
We defined a TYPE_ARRAY that doesn't tell you what scalar values you store, so when you query the array attribute, it has to look into the 4 columns and "merge", basically picking whatever value exists. And then this would pick the one that returned something and extend the results.
It's an evolution of a function that was trying to merge arrays with different types together so that's why it's complicated and I can definitely simplifies.
Yes, it would be fast, there are a few queries we can check after deploy in |
Merge the "Modernize Python tooling (ruff, uv, mypy)" change (#8040) from master into the eap-items typed-array read branch. Conflicts were all between this branch's typed-array logic and master's mechanical ruff modernization of the same lines. Resolved by keeping the branch's behavior and applying master's modernized style: - snuba/web/rpc/common/common.py: keep Iterable usage; import Callable and Iterable from collections.abc, use datetime.UTC (PEP 585 / pyupgrade). - endpoint_export_trace_items.py / endpoint_get_trace.py / endpoint_trace_item_details.py: keep the new read_typed_arrays / array_attribute_names parameters; adopt lowercase dict/tuple builtins. - endpoint_trace_item_details.py: keep the read_typed_arrays call argument while taking master's `except StopIteration as e:` (needed by `raise ... from e`). - endpoint_trace_item_table.py: keep the array group_by rejection; adopt set-comprehension form for non_aggregated/grouped columns. - resolvers/common/trace_item_table.py: keep the TYPE_ARRAY branch comment with master's `if` (no elif after return); adopt the single-line modernized _add_converter signature. - test_endpoint_trace_item_details.py / test_endpoint_trace_item_table.py: union the imports — keep state and get_storage, add master's get_writable_storage (all three are used). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AQf5jPfKy9N6VjEVMUExGs
The tooling modernization merged from master enables ruff's flake8-bugbear B905 rule, which flags zip() calls without an explicit strict= argument. Two branch-authored zip() calls over the typed-array columns now trip it. TYPED_ARRAY_MAP_COLUMNS and type_array_typed_columns_select_expressions() are 1:1 by construction (the latter is built by iterating the former), so strict=True is correct and documents that invariant. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AQf5jPfKy9N6VjEVMUExGs

Summary
Reading array attributes — both selecting and filtering — now goes natively through the typed
attributes_array_{string,int,float,bool}map columns past the cutoff (use_array_map_columns) instead of the legacyattributes_arrayJSON column, with notoJSONStringround-trip and no string coercion of stored elements. Returning/matching typed arrays directly is the whole point of double-writing arrays into typedMap(String, Array(T))columns.Pre-cutoff data has no typed columns populated, so both paths fall back to the JSON column exactly as before. The data is double-written, so the two paths return the same results.
Linear: EAP-562
Selecting arrays
We don't know an array attribute's element type at query time, and ClickHouse has no single column type that holds string/int/float/bool together, so a requested array attribute is selected as four native
arrayElement(attributes_array_<type>, key)sub-columns (one per typed map column), using the non-bucketed access pattern (arrayElement, likeattributes_int/attributes_bool). The four sub-columns are merged back into a single typedval_arrayin application code. Arrays are homogeneous, so exactly one sub-column is non-empty; element types (val_str/val_int/val_double/val_bool) are preserved natively — notuple, no JSON. An absent array attribute (all four sub-columns empty) is returned asis_null, matching the legacy scalar behaviour.Both read paths are retained:
attributes_arrayJSON-column allowlist.Filtering arrays
Array-membership (
<field>[*]:valueand its negation, plusLIKE/NOT LIKE, across string/number/boolean) keeps working — getsentry/sentry uses and tests it.Sentry's EAP query builder sends every array-membership value as
val_str, and the filter key only carriesTYPE_ARRAY(never the element type), so the typed column to read can't be picked from the value's proto type. Instead, on the typed path theval_stris coerced to each native type it parses as and matched natively against that column, OR'd together:"12") searches the int and float columns (it could be stored in either),true/falsesearches the bool column,Matching uses each column's native element type — no stringifying the stored arrays (so e.g.
stack.colno[*]:12matches the int12stored inattributes_array_int).LIKEpatterns can only match string elements, so they read justattributes_array_string. Existence checks arenotEmpty(...)over all four columns. Pre-cutoff, filtering uses the legacyattributes_arrayJSON column unchanged.Aggregations / group-by / order-by on arrays
uniqaggregation ✅ kept (unchanged from master) —FUNCTION_UNIQover an array attribute (e.g. crash-free user rate overuser_ids) still resolves touniqArrayIfOrNullover the legacyattributes_arrayJSON path. This is the one supported array aggregation and is independent of the typed-column read path.BadSnubaRPCRequestException(_array_aggregation_to_expression), same as master.Tests
test_endpoint_trace_item_table.py::test_select_array_column_before_and_after_cutoff,test_endpoint_trace_item_details.py::test_array_attributes_before_and_after_cutoff+test_convert_results_reads_typed_array_maps,test_endpoint_get_trace.py::test_process_results_merges_typed_array_subcolumns.test_common.py::TestTraceItemFiltersArrayMapColumns(string-only for plain strings, int+float for numeric strings, bool fortrue/false, native rhs literals, noarrayConcat/toStringover stored elements) + legacy JSON-path tests retained; ClickHouse-backedtest_endpoint_trace_item_table.pyarray wildcard-search / equals tests (string ignore-case, int, all scalar rhs types).test_aggregation.py::test_aggregation_to_expression_uniq_type_array(uniq kept) +..._sum_type_array_raises(others rejected);test_endpoint_time_series.py::test_crash_free_user_rate_with_array_attributes(pre-existing, exercises uniq over arrays).test_endpoint_trace_item_table.py::TestArrayOperationsRejected(group-by / order-by / non-uniq aggregation).ruff+ affected unit tests pass locally; ClickHouse-backed integration tests run in CI.🤖 Generated with Claude Code
https://claude.ai/code/session_018AH9MHDdP2k6S52KpQvLo6