WIP: MDEV-34041 Display additional information for materialized subqueries… #3241
Conversation
… in EXPLAIN/ANALYZE FORMAT=JSON
There was a problem hiding this comment.
Currently, there are two kinds of data structures:
- EXPLAIN data structure
- Tracker.
Explain_subq_materialization tries to be both. I know this is caused by
the fact that there's really no query plan.
I would still request to have the Explain class and the tracker class
explicitly. Even if the Explain class has only one member, the tracker.
The tracker class should follow other tracker classes:
-
data members are private.
-
execution code calls functions to report data
-
printing json output is done by local members.
-
constructor initalizes everything to some initial state which
can already be printed (currently it doesn't). This is to allow
SHOW ANALYZE and also the situation where subquery is never invoked.
sql/item_subselect.cc
Outdated
| partial_match_buffer_size); | ||
| } | ||
|
|
||
| for(uint i= 0; i < partial_match_merge_keys_counts.elements(); i++) |
There was a problem hiding this comment.
Any reason for not printing a JSON array here?
There was a problem hiding this comment.
Agree, fixed in the new revision
| }, | ||
| "subqueries": [ | ||
| { | ||
| "subquery_materialization": { |
There was a problem hiding this comment.
Can this be just materialization ?
sql/item_subselect.cc
Outdated
| if (is_analyze) | ||
| { | ||
| writer->add_member("r_exec_strategy").add_str( | ||
| exec_strategy_str[exec_strategy]); |
There was a problem hiding this comment.
is "exec" redundant in r_exec_strategy, can we just use r_strategy ?
Looking at the enum of strategies, I don't see many that make sense:
{
"undefined", "complete_match", "partial_match", "partial_match_merge",
"partial_match_scan", "impossible"
};
undefined makes sense if the execution never reached the code that determines the strategy.
complete match means "only do index lookup", perhaps we should just print "index_lookup".
partial_match - this does not occur in practice, does it?
partial_match_scan and partial_match_merge are ok.
There was a problem hiding this comment.
Agree with r_strategy. The enum of strategies indeed looks excessive, partial_match and impossible don't seem to be used anywhere. Do you think we can shrink subselect_hash_sj_engine::exec_strategy?
I don't understand what should be counted/tracked under this counter. Every lookup in the materialized subquery? |
… in EXPLAIN/ANALYZE FORMAT=JSON
Description
This is a WIP (work-in-progress) PR, that's why the test suite is not updated. The ANALYZE output format is open for discussion, so it makes sense to update existing tests after the output format is approved
Release Notes
TODO: What should the release notes say about this change?
Include any changed system variables, status variables or behaviour. Optionally list any https://mariadb.com/kb/ pages that need changing.
How can this PR be tested?
TODO: modify the automated test suite to verify that the PR causes MariaDB to behave as intended.
Consult the documentation on "Writing good test cases".
If the changes are not amenable to automated testing, please explain why not and carefully describe how to test manually.
Basing the PR against the correct MariaDB version
PR quality check