New generate_query_graph tool. #9212

Tmonster · 2023-10-04T10:41:45Z

The generate query graph code was taken out in #6972. Here I'm attempting to add it back in the generate query graph code. Using some pastel colors but it can honestly be anything.

Some goals for this version of the query graph

No javascript. Javascript is hard to maintain. This tree library is basically all CSS and doesn't need maintenance
Readability. The previous query graphs were not very readable (some colors made the text hard to read), The new colors prevent this from happening.
Same timing information as the previous version.

Things to improve

The ability to zoom out and see the whole plan. If the plan is bushy, the graph can get very wide, and it becomes difficult to see all of the operators in one screen. Currently however, it is much easier for a user to just navigate across the whole plan. This can also be fixed by decreasing the font for the html.
When detailed profiling is provided, add another UI element to show the timing information for the individual optimizers
better colors?

open to some initial feedback right now about colors/design.
You can check out the branch and generate the query graph with the following commands while in the duckdb directory.

duckdb -c "load tpch; call dbgen(sf=0.1); pragma enable_profiling=json;pragma profile_output="q09.json";pragma tpch(9);"
python3 scripts/generate_querygraph.py q09.json

Old query graph generator output

New query graph generator output

…package. Realized this is useful in case people who use the python utility want to inspect join graphs This reverts commit 9661e2c.

…extra 'nice' features

…ther operators

Mause · 2023-10-04T10:52:38Z

scripts/generate_querygraph.py

+    text = f.read()
+
+if open_output:
+    os.system('open "' + output.replace('"', '\\"') + '"')


Let's use the webbrowser module here instead

Mause · 2023-10-04T10:53:15Z

tools/pythonpkg/duckdb/__init__.py

 _exported_symbols.extend([
    "typing",
-    "functional"
+    "functional",
+    "duckdb_query_graph"


I don't think the module name needs to include duckdb

Mause · 2023-10-04T10:56:17Z

tools/pythonpkg/duckdb/duckdb_query_graph/__init__.py

+			total_timing_sum += self.get_summary_phase_timings(phase).time
+		return total_timing_sum
+
+QUERY_TIMINGS = AllTimings()


I don't see this being reset anywhere, can the graph generator only be used once per interpreter session?

That's the idea yea, but it's probably good to just reset this every time a new profile output is passed

Better to not use a global to begin with probably?

Mause · 2023-10-04T10:56:49Z

scripts/generate_querygraph.py

@@ -0,0 +1,45 @@
+# generate_querygraph.py


Let's include this code and ship it as an executable module instead

So that means it runs as python3 -m generate_querygraph explain_output.json ?

Like

python -m duckdb.query_graph ...

But basically yeah. Could even publish it an standalone executable on the user machines that way too

Tishj · 2023-10-05T08:30:22Z

scripts/generate_querygraph.py

+
+sys.path.insert(0, 'benchmark')
+
+arguments = sys.argv


I think using the argparse library makes this much less cumbersome

…o globals

Tmonster · 2023-10-06T09:22:09Z

Pressed the re-review button too early. Wait a couple of minutes

Mause · 2023-10-06T09:24:01Z

tools/pythonpkg/duckdb/query_graph/__main__.py

+        text = f.read()
+
+    if open_output:
+        webbrowser.open('file:///Users/tomebergen/scripts/run_imdb/tpch_new_html/q08.html', new=2)


Forgot to replace this?

Mause · 2023-10-06T09:24:46Z

tools/pythonpkg/duckdb/query_graph/__main__.py

+    translate_json_to_html(input, output)
+
+    with open(output, 'r') as f:
+        text = f.read()


What's this for?

Mause · 2023-10-06T09:25:04Z

tools/pythonpkg/duckdb/query_graph/__main__.py

+        'chart_script': ''
+    }
+
+# ????


Mause · 2023-10-06T09:26:29Z

tools/pythonpkg/duckdb/query_graph/__main__.py

+# if detailed profiling is enabled, then information on the timing of each stage of the query is available
+# that is "top level timing information"
+def get_top_level_timings(json):
+    return []


Is this supposed to contain something?

Mause · 2023-10-06T09:26:46Z

tools/pythonpkg/duckdb/query_graph/__main__.py

+
+def open_utf8(fpath, flags):
+    import sys
+    if sys.version_info[0] < 3:


We don't support python 2 anymore

I think you can just inline this function, no warning required

Mause · 2023-10-06T09:27:39Z

scripts/generate_querygraph.py

@@ -0,0 +1,45 @@
+# generate_querygraph.py


This file can be deleted now right?

…e unused code, remove support for python2

Mause · 2023-10-09T06:42:17Z

tools/pythonpkg/duckdb/query_graph/__main__.py

+
+def open_utf8(fpath, flags):
+    import sys
+    if sys.version_info[0] < 3:


I think you can just inline this function, no warning required

Mause · 2023-10-09T06:43:11Z

tools/pythonpkg/duckdb/query_graph/__main__.py

+    # add up all of the times
+    # measure each time as a percentage of the total time.
+    # then you can return a list of [phase, time, percentage]
+    child_timings = get_child_timings(json['children'][0], query_timings)


nitpick: this variable is never used, just call the function

Mause · 2023-10-09T06:43:44Z

tools/pythonpkg/duckdb/query_graph/__main__.py

+        f.write(html)
+
+def main():
+    sys.path.insert(0, 'benchmark')


Why have you added this line?

…ucture

Tmonster · 2023-10-09T12:29:30Z

@Mause Some of those lines are leftover from the old script which I dropped in. Sorry for not thoroughly checking it. Thank you for the reviews 🙏 !

Mause

Looks good. Just need to run the make format-fix script and update for the stubs test

…ucture

This reverts commit 0707761.

Mytherin · 2023-10-19T11:32:45Z

@Tmonster @Mause is this ready to merge?

Tmonster · 2023-10-19T14:03:25Z

Ready from my side 👍

Mytherin · 2023-10-19T14:19:03Z

Thanks!

Tmonster added 15 commits December 6, 2022 10:59

basic workings are up. Just need to get the original timings in there

0a0af3a

made a yellow background. Up next, the timing table on the left

b5266f0

adding table of timings

15e07bf

table sorta works. Need to minify JS and re-style the squares

9cd661e

Reverted a commit that took out the graph generation from the python …

3015c79

…package. Realized this is useful in case people who use the python utility want to inspect join graphs This reverts commit 9661e2c.

can create graphs from the package now. Still need to implement some …

9cd3eba

…extra 'nice' features

Merge remote-tracking branch 'origin' into new_query_graph_structure

540691d

Merge branch 'main' into new_query_graph_structure

c18a17f

renaming and moving around, but can no longer import duckdb

902030f

remove duckdb_query_graph from python building

5a6974a

script works again. pure css

d7a6178

some changes to get nicer colors. might need to add more colors for o…

623f146

…ther operators

new colors for more operators. delete the css

fb6cd9b

format-fix

fa773c3

new query graph tool

48e1b13

Tmonster requested review from Tishj and lnkuiper October 4, 2023 10:42

Mause suggested changes Oct 4, 2023

View reviewed changes

Tishj reviewed Oct 5, 2023

View reviewed changes

PR review comments. query_graph is exceutable module. use argparse, n…

7ccf53b

…o globals

github-actions bot marked this pull request as draft October 6, 2023 09:18

Tmonster requested review from Mause and Tishj October 6, 2023 09:20

Mause suggested changes Oct 6, 2023

View reviewed changes

remove hardcoded string, remove support for detailed profiling, remov…

40255e9

…e unused code, remove support for python2

Tmonster marked this pull request as ready for review October 6, 2023 10:58

Mause suggested changes Oct 9, 2023

View reviewed changes

Tmonster added 2 commits October 9, 2023 10:04

Merge remote-tracking branch 'upstream/main' into new_query_graph_str…

e84a17e

…ucture

PR comments still cleaning up old generat_query_graph.py script

1ac182c

github-actions bot marked this pull request as draft October 9, 2023 08:24

Tmonster marked this pull request as ready for review October 9, 2023 12:28

Tmonster requested a review from Mause October 9, 2023 12:50

Mause suggested changes Oct 10, 2023

View reviewed changes

Tmonster added 4 commits October 10, 2023 10:11

run make format fix a regenerate_python_stubs

0707761

Merge remote-tracking branch 'upstream/main' into new_query_graph_str…

27ccba7

…ucture

Revert "run make format fix a regenerate_python_stubs"

e7cee6c

This reverts commit 0707761.

add stubs to query graph

03940a0

github-actions bot marked this pull request as draft October 10, 2023 09:47

Tmonster added 2 commits October 10, 2023 15:44

remove query graph from stubs

06f5601

Merge branch 'main' into new_query_graph_structure

f5680dc

Tmonster marked this pull request as ready for review October 11, 2023 08:05

Tmonster requested a review from Mause October 11, 2023 12:49

Mytherin merged commit 6ff89e6 into duckdb:main Oct 19, 2023
14 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

New generate_query_graph tool. #9212

New generate_query_graph tool. #9212

Tmonster commented Oct 4, 2023 •

edited

Loading

Mause Oct 4, 2023

Mause Oct 4, 2023

Mause Oct 4, 2023

Tmonster Oct 5, 2023

Mause Oct 5, 2023

Mause Oct 4, 2023

Tishj Oct 5, 2023

Mause Oct 5, 2023

Tishj Oct 5, 2023

Mause Oct 5, 2023

Tmonster commented Oct 6, 2023

Mause Oct 6, 2023

Mause Oct 6, 2023

Mause Oct 6, 2023

Mause Oct 6, 2023

Mause Oct 6, 2023

Mause Oct 9, 2023

Mause Oct 6, 2023

Mause Oct 9, 2023

Mause Oct 9, 2023 •

edited

Loading

Mause Oct 9, 2023

Tmonster commented Oct 9, 2023 •

edited

Loading

Mause left a comment

Mytherin commented Oct 19, 2023

Tmonster commented Oct 19, 2023

Mytherin commented Oct 19, 2023

New generate_query_graph tool. #9212

New generate_query_graph tool. #9212

Conversation

Tmonster commented Oct 4, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Tmonster commented Oct 6, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Mause Oct 9, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Tmonster commented Oct 9, 2023 • edited Loading

Mause left a comment

Choose a reason for hiding this comment

Mytherin commented Oct 19, 2023

Tmonster commented Oct 19, 2023

Mytherin commented Oct 19, 2023

Tmonster commented Oct 4, 2023 •

edited

Loading

Mause Oct 9, 2023 •

edited

Loading

Tmonster commented Oct 9, 2023 •

edited

Loading