-
-
Notifications
You must be signed in to change notification settings - Fork 57
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(eap): add virtual column support #6292
Conversation
❌ 1 Tests Failed:
View the top 1 failed tests by shortest run time
To view individual test run time comparison to the main branch, go to the Test Analytics Dashboard |
Does this feature work for only the |
|
||
SELECT | ||
-- see the project name column transformed and the value mapping injected | ||
transform( CAST( project_id, 'String'), array( '1', '2'), array( 'sentry', 'snuba'), 'unknown') AS `project_name`, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
also I think cast should go in other way, if you request a numeric column it'll be better performance if we cast '1' instead of project_id
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree with the performance being better if we cast '1'
rather than project_id
. I was doing this to keep the logic and interface simpler
@mcannizz How would virtual columns work in a timeseries? I don't think I understand |
Suppose you wanted to count events over time by project -- one time series per project -- and you wanted to sort those by project name. Would we support that? (I'm asking to understand the scope of the feature as implemented, not assert a requirement.) |
I see, right now that is not supported here and the protobuf interface for timeseries doesn't support it either |
(not to say it can't happen in the future) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
would be nice if test had multiple spans to make sure order_by actually worked, but it's probably fine
"sentry.sdk.name": "sentry.python.django", | ||
"release_version": "4.2.0.69", | ||
} | ||
for _ in range(60) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Question: Will the response first 60 samples mapped to have project_name as 'sentry' ?
Add support for virtual columns to allow for sorting by project name or semver release (as opposed to release id).