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

Support (EXPLAIN SELECT ...) as a subquery #40630

Merged
merged 4 commits into from
Oct 5, 2022

Conversation

vdimir
Copy link
Member

@vdimir vdimir commented Aug 25, 2022

Changelog category (leave one):

  • Improvement

Changelog entry (a user-readable short description of the changes that goes to CHANGELOG.md):

  • Support expression (EXPLAIN SELECT ...) in a subquery. Queries like SELECT * FROM (EXPLAIN PIPELINE SELECT col FROM TABLE ORDER BY col) became valid.

We have some tests that do clickhouse client -q "EXPLAIN ..." | grep ..., this PR would allow to write pure .sql tests for such cases.

$CLICKHOUSE_CLIENT -q "EXPLAIN PIPELINE SELECT date, i FROM t_read_in_order_2 WHERE date >= '2020-10-11' ORDER BY i DESC LIMIT 10" | grep -o "MergeSorting"

$CLICKHOUSE_CLIENT --query="EXPLAIN PIPELINE SELECT DISTINCT b FROM (SELECT b FROM remote('127.0.0.{1,2}', '$CLICKHOUSE_DATABASE', test_local) GROUP BY a, b)" | grep -o "DistinctTransform" || true

It allows to work with the result of EXPLAIN e.g in tests

Information about CI checks: https://clickhouse.com/docs/en/development/continuous-integration/

@robot-ch-test-poll1 robot-ch-test-poll1 added the pr-improvement Pull request with some product improvements label Aug 25, 2022
@kitaisreal
Copy link
Collaborator

I would prefer to add explain table function, because using EXPLAIN query as parameter breaks a lot of logic in parser (because EXPLAIN SELECT) is not an expression and it is impossible to analyze, because such EXPLAIN query need to be represented in query tree somehow.

@alexey-milovidov
Copy link
Member

I would prefer to allow all query types that return results: EXPLAIN, DESCRIBE, SHOW, etc.

@kitaisreal
Copy link
Collaborator

I checked and in this pull request view(EXPLAIN ...) only change parser, so it is okay. Maybe it make sense to support such queries with rewritting on parser stage. It should not affect any other stages.
Example:

SELECT (EXPLAIN subquery); 

Result of parser level rewrite:

SELECT (SELECT explain_result FROM explainView(subquery)).

@alexey-milovidov, @vdimir what do you think ?

@kitaisreal kitaisreal self-assigned this Aug 25, 2022
@kitaisreal
Copy link
Collaborator

Additional examples:

SELECT * FROM (DESCRIBE TABLE table)
SELECT * FROM (SELECT * FROM describeView(table))
SELECT * FROM (SHOW PROCESSLIST)
SELECT * FROM (SELECT * FROM processListView(table))

Maybe we can even create some generic view that can have argument that identifies its type 'PROCESS_LIST', 'DESCRIBE'.

@vdimir vdimir force-pushed the vdimir/explain-table-function branch from e9da7f7 to bada21c Compare September 28, 2022 13:15
@vdimir vdimir changed the title Support table function view(EXPLAIN SELECT ...) Support (EXPLAIN SELECT ...) as a subquery Sep 28, 2022
@vdimir
Copy link
Member Author

vdimir commented Sep 30, 2022

@kitaisreal

I've changed the implementation to support EXPLAIN subquery directly SELECT (EXPLAIN subquery)

It converts the query to SELECT (SELECT * FROM explainView(subquery)) under the hood so that it would work with the analyzer without any changes. But calling directly explainView is impossible, and I suppose not expose it to the user. It requires more changes in the parser (like for the existing view function). Also, we should pass kind and settings into it, so it may start looking like explainView('pipeline', query, header = 1, description = 1), which is difficult to parse and use, but it doesn't add any convenience.

If you are ok with implementation, let's finish this PR with EXPLAIN and I'll do the same for SHOW, DESCRIBE, and so on in the next.

@kitaisreal
Copy link
Collaborator

@vdimir great solution, I am okay with implementation. Let finish this for EXPLAIN and merge.

@vdimir vdimir force-pushed the vdimir/explain-table-function branch from 33017a9 to fa254bc Compare October 3, 2022 17:14
@vdimir
Copy link
Member Author

vdimir commented Oct 5, 2022

CI failures on fa254bc

Stateful tests (debug) — fail: 1, passed: 128, skipped: 2 Details

#41238

Stateless tests (aarch64) — fail: 1, passed: 4499, skipped: 19 Details

02267_file_globs_schema_inference

Stateless tests (debug, s3 storage) [1/3] — fail: 1, passed: 1500, skipped: 21 Details

00993_system_parts_race_condition_drop_zookeeper - #36099

Stateless tests (debug, s3 storage) [3/3] — Tests are not finished, fail: 1, passed: 1301, skipped: 10 Details

02397_system_parts_race_condition_drop_rm #39022

Stateless tests (release, s3 storage) — fail: 1, passed: 4484, skipped: 34 Details

00993_system_parts_race_condition_drop_zookeeper

Stress test (asan) — Backward compatibility check: Error message in clickhouse-server.log (see bc_check_error_messages.txt) Details

2022.10.04 11:50:24.876635 [ 496080 ] {} <Error> test_3.rmt2 (ReplicatedMergeTreeAttachThread): Initialization failed, table will remain readonly. Error: The local set of parts of table test_3.rmt2 (8c49dc8d-da00-423e-9440-7907a80c88ee) doesn't look like the set of parts in ZooKeeper: 1.00 rows of 1.00 total rows in filesystem are suspicious. There are 1 uncovered unexpected parts with 1 rows (0 of them is not just-written with 0 rows), 0 missing parts (with 0 blocks), 0 covered unexpected parts (with 0 rows).

Stress test (msan) — OOM killer (or signal 9) in clickhouse-server.log Details

OOM

Stress test (ubsan) — Killed by signal (in clickhouse-server.log) Details

Tried to lock part -3_6_6_0 for removal second time by ... #39022

All failures are known already, no new ones

@vdimir vdimir merged commit d63a1fd into master Oct 5, 2022
@vdimir vdimir deleted the vdimir/explain-table-function branch October 5, 2022 17:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr-improvement Pull request with some product improvements
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants