feat(data-explore): Add support in EAP to filter Array by elements#7898
Conversation
Allow searching EAP Items by arrays where a given value is included within array (as an item, membership test)
Test to check types of array columns
Attempt to return the column as it is
SELECT (
cast(lower(leftPad(hex(item_id), if(
greater(length(hex(item_id)), 16), 32, 16), '0')), 'String'
) AS
), (attributes_array.::Array(
JSON) AS frame_linenos_TYPE_ARRAY
)
FROM eap_items_1_local
Attempt(Works) return the columns as it is
Fix the filter path.. and update the tests..
Update the comments in code
Remove query printing
Remove query printing
66bc9b5 to
ee6a796
Compare
| return field, existence | ||
| elif aggregation.key.type == AttributeKey.Type.TYPE_ARRAY: | ||
| field = type_array_to_stored_array_json_path(aggregation.key) | ||
| return field, f.notEmpty(field) |
There was a problem hiding this comment.
Array aggregation uses wrong existence check after field type change
Medium Severity
_resolve_field_and_existence correctly computes f.notEmpty(field) for TYPE_ARRAY, but aggregation_to_expression discards this field_exists and calls _array_aggregation_to_expression without it. Inside _array_aggregation_to_expression, get_field_existence_expression(field) is re-computed on the JsonPath object, which falls through to f.isNotNull(field) since JsonPath isn't a FunctionCall. Per the codebase comment, missing JSON array keys return empty arrays (not NULL), so isNotNull incorrectly treats missing attributes as present. This changes the aggregation result from NULL to 0 when no rows have the attribute.
Additional Locations (2)
Reviewed by Cursor Bugbot for commit ee6a796. Configure here.
onewland
left a comment
There was a problem hiding this comment.
seems generally fine, I think, pending tests passing. Have we documented clearly somewhere what the right behavior is for null because I'm confused and not really following. Maybe literally a NULL.md somewhere in snuba/protos?
| match value_type: | ||
| case "val_str": | ||
| return literal(v.val_str) | ||
| case "val_int": | ||
| return f.toString(literal(v.val_int)) | ||
| case "val_double": | ||
| return f.toString(literal(v.val_double)) | ||
| case "val_float": | ||
| return f.toString(literal(v.val_float)) | ||
| case "val_bool": | ||
| return literal(str(v.val_bool).lower()) | ||
| case _: | ||
| raise BadSnubaRPCRequestException( | ||
| f"unsupported AttributeValue for array membership: {value_type}" |
There was a problem hiding this comment.
Is there a repeat of this logic tree elsewhere that could be reused or put in a common place?
Good Idea. let me finalize this, I need to ask product about this. |
|
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
There are 3 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 13cd6ba. Configure here.


Current implementation of arrays in TraceItemTable endpoint transforms arrays as array of strings, and then only allows
LIKE/NOT_LIKEfilter on them.With this change, arrays will preserve their original type, and client can filter EAP items by element of an array attribute.
To preserve the type, array is converted to JSON string, (clickhouse-driver doesn't support Array(JSON) type, as discussed in this PR), and post processing of results recovers the original array by decoding JSON string.
To filter, predicate maps arrays to corresponding type and use array exists with element filter.
Example Clickhouse query is: