Add HTML and Graphviz support for explain analyze #13942
Conversation
7f6ce3b to
6e8a5b0
Compare
|
While this technically works, it does result in a lot of code duplication and this code duplication has to be extended further for every format that gets added. I agree that this issue with analyze has to be fixed first, but hopefully we can move this logic into the |
src/main/query_profiler.cpp
Outdated
| return str.str(); | ||
| } | ||
|
|
||
| void QueryProfiler::GraphvizToStream(std::ostream &ss) const { |
There was a problem hiding this comment.
Same comment as above, can we avoid this duplication?
There was a problem hiding this comment.
Note: I also changed the call from connection.cpp to call into query_profiler::ToString(ProfilerPrintFormat)
| @@ -247,6 +252,10 @@ string QueryProfiler::ToString(ExplainFormat explain_format) const { | |||
| return ToJSON(); | |||
There was a problem hiding this comment.
Can we merge ToJSON, ToHTML and ToGraphViz, only the ExplainFormat has to be passed down I feel.
Ideally we make adjustments to QueryTreeToString to unify all these paths but that can be done at a later point
There was a problem hiding this comment.
The ToJSON implementation in query_profiler.cpp is different from the JSONTreeRenderer and they produce different output. If client code is integrated with the current json output, this will be a breaking change to use the JSONTreeRendere here. Thoughts?
There was a problem hiding this comment.
I moved the functionality into the ToString() method itself. Eventually, if we move the differences in json and querytree rendering to the tree_renderer derivatives, this this function should be able to handle it all.
There was a problem hiding this comment.
The ToJSON implementation in
query_profiler.cppis different from theJSONTreeRendererand they produce different output. If client code is integrated with the current json output, this will be a breaking change to use theJSONTreeRenderehere. Thoughts?
Yes, that's one of the reasons I haven't touched it yet and I don't have an immediate answer for you because I was not involved in creating the ToJSON logic query_profiler.cpp so I don't yet know what the implications behind changing that are
6e8a5b0 to
6be0a6a
Compare
6be0a6a to
bb3713f
Compare
bb3713f to
f618132
Compare
|
@Tishj ready for re-review. |
f618132 to
b29c21f
Compare
HTML and Graphviz were only supported for explain output, not explain analyze. This PR adds support for explain analyze as well. In addition, the total query elapsed time is written into the root (Query) node so that it is available in the rendered html or graphviz output.
b29c21f to
297afb2
Compare
|
Thanks! |
HTML and Graphviz were only supported for explain output, not explain analyze. This PR adds support for explain analyze as well. In addition, the total query elapsed time is written into the root (Query) node so that it is available in the rendered html or graphviz output.