-
Notifications
You must be signed in to change notification settings - Fork 414
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
[CLOSE AT THE END OF MILESTONE] You should be able to save a chart from SQL Runner #1519
Comments
Blocked by #1135 |
Is this issue still relevant? There have been no updates for 60 days, please close the issue or keep the conversation going! |
|
Is this issue still relevant? There have been no updates for 60 days, please close the issue or keep the conversation going! |
It's true that with Metabase, the equivalent "explore" functionality is sufficiently clunky that people who know SQL will just use that instead. However, even with Looker and within Lightdash, there will be edge cases where an SQL only chart makes more sense. Example:
This would be quite difficult to achieve using standard Lightdash joins but relatively simple in SQL
|
Also this sets up another very powerful feature that Metabase and Looker both have. In the above example, the "product" field and date values are obvious candidates to be parameterised. Metabase: https://www.metabase.com/glossary/parameter I think the whole meta language for LookML thing that Looker does is overkill but Metabase's set up is simple and powerful. Using the example from above, I would build a generic transition state explorer for my |
The other argument in favour of SQL runners is that yes this is bad practice and effectively tech debt but incurring debt in the short term can be valid. For example: Data analysts review SQL runner created regularly and migrate them back into proper Lightdash dashboards. If SQL runner was created for a good reason (i.e. metric missing) they can add the metric in asynchronously. If it's used for the wrong reasons, can reach out to the relevant person and advise them on best practice. Overall, users are less likely to be blocked and data analysts receive less "urgent requests". |
Is this issue still relevant? There have been no updates for 60 days, please close the issue or keep the conversation going! |
Will this be added to the product at some point? This feature would move us (100+ users) away from Metabase. |
This feature would also move us (800+ users) away from Looker. The lack of it turns migrating content barely impossible. |
Yes! Just to add further product feedback - we would entirely remove our dependence on Snowflake Dashboards (and be able to shift all of our BI to Lightdash) with this feature. We definitely understand the philosophical desire to want to avoid allowing business logic to be created ad hoc, so some way of flagging charts that are "unstable" and/or "have logic that does not live in DBT" would be phenomenal, as well. |
Similar to the comment from @dsl16 for any company without a green field install, there will likely be some migration process and allowing + flagging "ad hoc" charts from SQL queries is a huge value add as a intermediate step in that migration. I can imagine this would drive substantial adoption. As an example, we are migrating from periscope/sisense and this would be very very helpful. |
To add to this, I would find it useful to have an approval layer on charts built with SQL runner. i.e. in addition to (or alternatively to) flagging a chart as being "built through SQL runner", one possible implementation could be for SQL runner charts to be displayed as "unvalidated" (or words to that effect) by default, and for there to be a validation page accessible only to admins (similar UI to the current validator for broken charts), where an admin can review the code and, if appropriate, mark the chart as "validated" (even better if you can give feedback to the person who wrote the code). |
+1 |
Save.SQL.Runner.chart.mp4This video shows a proof of concept of:
Branch: feat/hackathon-save-sql-runner-charts InsightsFeature limitationsAs already mentioned in the ticket description there will be limitations in these charts since we can't manipulate the SQL.
Open questions
ImplementationI recommend that The biggest amount of work is:
|
Here is another option for how we could implement this: https://miro.com/app/board/uXjVMjWvUlQ=/?moveToWidget=3458764564607634429&cot=14 Overview of what's happeningOnce you create a SQL query, you are basically creating a table that you can then explore. Think of this almost as like a "custom explore builder" rather than just a SQL runner. Why is this different to the option above?Pros:
Cons
Questions
|
Talked to some users about this and showed the prototypes. We're going to go with Jose's proposal here: #1519 (comment) The reasons:
|
This issue was mentioned by a user in slack: https://lightdash.slack.com/archives/C065AQZF5CG/p1701967718594729?thread_ts=1701967718.594729&cid=C065AQZF5CG |
This issue was mentioned by a user in slack: https://lightdash.slack.com/archives/C06J7R3HC5C/p1713573213441529?thread_ts=1713573213.441529&cid=C06J7R3HC5C |
@TuringLovesDeathMetal Checking in on this, is still something planned, or has this now been archived? |
+1 |
This feature has now been released and is generally available 🎉 |
Description: What is it?
You should be able to save charts made in SQL Runner
Acceptance criteria
Actions/metadata in the SQL Runner chart view:
Actions/metadata NOT in the SQL Runner chart view:
Explore from here
Filters
View underlying data (chart or table)
Drill into (chart or table)
Users can visualise and edit the SQL-generated chart, but there is some indication that it's
non-interactive
explore from here
is greyed out and on-hover it saysthis chart is non-interactive because it was generated from the SQL runner
copy value
is shown - the rest of the options are greyed-out with the same message on hover ^Docs
guides/
calledusing the SQL runner
Analytics events
Add
num_sql_runner_charts
todashboard_created
,dashboard_updated
events.This tells us how many of the charts on a dashboard come from sql runner charts.
add
sql_runner_chart
tosaved_chart_created
,saved_chart_updated
andsaved_chart_deleted
events. This is a True/False valueThe text was updated successfully, but these errors were encountered: