RPC handles same name attributes

[xurui-c]

Fixes https://github.com/getsentry/projects/issues/400

[codecov]

❌ 1 Tests Failed:

Tests completed	Failed	Passed	Skipped
2666	1	2665	5

View the top 1 failed tests by shortest run time

tests.web.rpc.test_common.TestCommon::test_expression_trace_id

Stack Traces | 0.003s run time

Traceback (most recent call last):
  File ".../web/rpc/test_common.py", line 11, in test_expression_trace_id
    assert attribute_key_to_expression(
AssertionError: assert CAST(\n  trace_id,\n  'String'\n) AS `sentry.trace_id TYPE_STRING` == CAST(\n  trace_id,\n  'String'\n) AS `sentry.trace_id`
  
  Matching attributes:
  ['function_name', 'parameters']
  Differing attributes:
  ['alias']
  
  Drill down into differing attribute alias:
    alias: 'sentry.trace_id TYPE_STRING' != 'sentry.trace_id'
    - sentry.trace_id
    + sentry.trace_id TYPE_STRING
  Full diff:
    CAST(
      trace_id,
      'String'
  - ) AS `sentry.trace_id`
  + ) AS `sentry.trace_id TYPE_STRING`
  ?                      ++++++++++++

To view more test analytics, go to the Test Analytics Dashboard
Got feedback? Let us know on Github

[xurui-c]

This is what CH looks like after generating the message:

This is what columns look like in the RPC request:
{ "key": { "type": "TYPE_INT", "name": "foo" }, "label": "tags[foo,number]" }, { "key": { "type": "TYPE_STRING", "name": "foo" }, "label": "tags[foo,string]" }, { "key": { "type": "TYPE_STRING", "name": "foo" }, "label": "tags[foo]" },

after processing the first column (label=tags[foo,number]), the query pipeline caches the name foo in ParsingContext() here. So when processing the second column (label=tags[foo,string]) and the third column (label=tags[foo]), the query pipeline sees that the name (or alias in this part of the codebase) is the same (foo), so it will use the cached result here.

What this caching alias does is turn the CH query into this: SELECT (name AS sentry.name), (CAST(arrayElement(attr_num_3, 'foo'), 'Int64') AS foo), foo, foo, (lower(hex(span_id)) AS sentry.span_id), (transform(CAST((project_id AS sentry.project_id), 'String'), ['4555075531898880'], ['bar'], 'unknown') AS project.name) FROM eap_spans_2_local WHERE in(project_id, [1]) AND equals(organization_id, 1) AND less(_sort_timestamp, toDateTime(1732107600)) AND greaterOrEquals(_sort_timestamp, toDateTime(1732104000)) AND true GROUP BY sentry.name, foo, foo, foo, sentry.span_id, project.nameORDER BYsentry.name ASC LIMIT 10000 OFFSET 0. So the second and third foo columns (which should've been string "five") are cached to be the same as (CAST(arrayElement(attr_num_3, 'foo'), 'Int64') AS foo), which is integer 5.

Potential solutions:

1. What if we cache at the end of a query, instead of as we process each column? I tried that in a previous commit of this PR, and made the CH query look like this:

SELECT (name AS `sentry.name`), (CAST(arrayElement(attr_num_3, 'foo'), 'Int64') AS foo), (arrayElement(attr_str_3, 'foo') AS foo), (arrayElement(attr_str_3, 'foo') AS foo), (lower(hex(span_id)) AS `sentry.span_id`), (transform(CAST((project_id AS `sentry.project_id`), 'String'), ['4555075531898880'], ['bar'], 'unknown') AS `project.name`) FROM eap_spans_2_local WHERE in(project_id, [1]) AND equals(organization_id, 1) AND less(_sort_timestamp, toDateTime(1732107600)) AND greaterOrEquals(_sort_timestamp, toDateTime(1732104000)) AND true GROUP BY `sentry.name`, foo, foo, foo, `sentry.span_id`, `project.name` ORDER BY `sentry.name` ASC LIMIT 10000 OFFSET 0

But, clickhouse does not allow the same alias (foo). The error is snuba.clickhouse.errors.ClickhouseError: DB::Exception: Different expressions with the same alias foo: attr_str_3['foo'] AS foo and CAST(attr_num_3['foo'], 'Int64') AS foo : While processing attr_str_3['foo'] AS foo. Stack trace:

2. We don't allow attributes with the same name but different types

3. Change the query pipeline so that we use the label as the alias in the CH query, so groupby will use label as well. My understanding is that label is always unique (and would thus avoid alias collisions). This effectively changes (CAST(arrayElement(attr_num_3, 'foo'), 'Int64') AS foo), (arrayElement(attr_str_3, 'foo') AS foo), (arrayElement(attr_str_3, 'foo') AS foo) in potential solution 1 to (CAST(arrayElement(attr_num_3, 'foo'), 'Int64') AS tags[foo,number]), (arrayElement(attr_str_3, 'foo') AS tags[foo,string]), (arrayElement(attr_str_3, 'foo') AS tags[foo])

[volokluev]

Great job reproducing this @xurui-c

I think what would make sense is to have the expression be generated by us here to avoid alias collisions.

And the SelectedExpression alias should have the label the client assigned in the API call. Does that make sense?

[volokluev]

almost there, just change the delimiter and then you are good to merge. Also @davidtsuk does this mess up your extrapolation aliasing business?