Skip to content
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

Web console: revamp the experimental explore view #17180

Merged
merged 8 commits into from
Sep 30, 2024

Conversation

vogievetsky
Copy link
Contributor

This PR changed Explore view to be able to work on top of an arbitrary query as its "source" instead of just a table.

To achieve this I needed to get rid of @druid-toolkit/visuals-core and @druid-toolkit/visuals-react and reimplement a lot of the logic. That (and also the fixed to the test for the grouping table) account for the majority of the changes.

With this PR the explore view can:

be configured on top of a "source" query

image

make edits to the source query via point-and-click

image

store "Measures" in the source query

image

Have stateful URLs

image

This PR also fixes misc bugs in the explore view (and probably creates a greater number of new ones due to the increased surface area).

Copy link
Member

@clintropolis clintropolis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is an amazing tool, kudos 🤘

its like all the beauty and speed of exploration provided by a data visualization tool with all of the power and control of writing raw SQL when you need it, this can do so very much, i'm incredibly impressed 🎉

clicked around for a long time and found a few issues (see you've pushed a couple commits since i pulled so idk if any of these are already fixed):

  • changing time granularity doesnt refresh chart in time chart view
  • browser back does not refresh chart (but does go back, if i refresh page it shows the previous thing)
  • deleting max rows sets to 0 which makes a query that fails (maybe druid could do better here too)
  • if you dont select __time in the base query, lots of stuff that works by issuing queries on time stops working
  • filters seem like they get created incorrectly for array columns, e.g. WHERE "arrayDouble" = '[1.1,2.2,3.3]' should be something like WHERE "arrayDouble" = ARRAY[1.1,2.2,3.3]
  • multi-axis chart layout seems to get a bit wonky if adding too many measures
  • dev tools open in chrome wreaks havoc on chart tooltip info popup thingy

I don't think these all need to be resolved before merging, though the back button one might be nice. I also feel like this is cool enough now that maybe we should consider putting it on the top bar instead of hidden behind the '...' to get it more exposure, but that discussion also doesn't need to block merging.

I had so much fun clicking around various things I found a druid bug, #17183, the 0 limit thing might also be something to improve, haven't looked at that yet

@clintropolis clintropolis added this to the 31.0.0 milestone Sep 28, 2024
@vogievetsky
Copy link
Contributor Author

Thank you so much for the review and the positive feedback!

changing time granularity doesnt refresh chart in time chart view

Can't repro this, is it possible your datasource was already floored to hour (query granularity) and you were changing between say 5min and 1hr?

browser back does not refresh chart (but does go back, if i refresh page it shows the previous thing)

Fixed, made back and forward work as expected

deleting max rows sets to 0 which makes a query that fails (maybe druid could do better here too)

Fixed, it was not listening to it's min of 1

if you dont select __time in the base query, lots of stuff that works by issuing queries on time stops working

Partially fixed by making things more robust and show errors better. Some things will still not work without a TIMESTAMP column but at least they will tell you about it

filters seem like they get created incorrectly for array columns, e.g. WHERE "arrayDouble" = '[1.1,2.2,3.3]' should be something like WHERE "arrayDouble" = ARRAY[1.1,2.2,3.3]

Yeah, this is also a bug in the query view. I will fix it in a followup PR. Would be cool to make an ARRAY filter UX also

multi-axis chart layout seems to get a bit wonky if adding too many measures

Yup. skipping this for now but will look into it in a followup

dev tools open in chrome wreaks havoc on chart tooltip info popup thingy

Can't repro this either but not super concerning if devtools need to be open (it might be some mode that you have set, like touch mode). Will followup offline.

@vogievetsky vogievetsky merged commit d982727 into apache:master Sep 30, 2024
90 checks passed
@vogievetsky vogievetsky deleted the explore_source branch September 30, 2024 06:15
vogievetsky added a commit to vogievetsky/druid that referenced this pull request Sep 30, 2024
* explore revamp

* remove ToDo

* fix CodeQL

* add tooltips

* show issue on echart chars

* fix: browser back does not refresh chart

* fix maxRows 0

* be more resiliant to missing __time
kfaraz pushed a commit that referenced this pull request Oct 1, 2024
#17200)

* Web console query view improvements (#16991)
* Made maxNumTaskOptions configurable in the Query view
* Updated the copy for taskAssignment options
* Reordered options in engine menu for msq engine
* fixed snapshot
* maxNumTaskOptions -> maxTasksOptions
* added back select destination item
* fixed duplicate menu item
* snapshot
* Added the ability to hide certain engine menu options
* Added the ability to hide/show more menu items
* Make the tooltip better and improve structure (#17132)
* switch to using arrays by default (#17133)
* Web console: add stage graph (#17135)
* Web console: revamp the experimental explore view (#17180)
* explore revamp
* remove ToDo
* fix CodeQL
* add tooltips
* show issue on echart chars
* fix: browser back does not refresh chart
* fix maxRows 0
* be more resiliant to missing __time
---------
Co-authored-by: Sébastien <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants