Skip to content

add explain visualizer - #1279

Merged
j-bennet merged 8 commits into
dbcli:mainfrom
dkuku:explain_visualizer
Apr 4, 2022
Merged

add explain visualizer#1279
j-bennet merged 8 commits into
dbcli:mainfrom
dkuku:explain_visualizer

Conversation

@dkuku

@dkuku dkuku commented Aug 7, 2021

Copy link
Copy Markdown
Contributor

Description

adds a formatter for explain analyze output
based on https://github.com/dkuku/pyev

Checklist

  • I've added this contribution to the changelog.rst.
  • I've added my name to the AUTHORS file (or it's already there).
  • I installed pre-commit hooks (pip install pre-commit && pre-commit install), and ran black on my code.
  • Please squash merge this pull request (uncheck if you'd like us to merge as multiple commits)

@dkuku

dkuku commented Aug 13, 2021

Copy link
Copy Markdown
Contributor Author

tbh I have no idea whats the best way to write tests for it ?

@dkuku
dkuku marked this pull request as ready for review August 13, 2021 07:54
@codecov-commenter

codecov-commenter commented Aug 14, 2021

Copy link
Copy Markdown

Codecov Report

Merging #1279 (f696e30) into master (3a95530) will decrease coverage by 5.55%.
The diff coverage is 35.22%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1279      +/-   ##
==========================================
- Coverage   83.80%   78.25%   -5.56%     
==========================================
  Files          21       23       +2     
  Lines        2594     2888     +294     
==========================================
+ Hits         2174     2260      +86     
- Misses        420      628     +208     
Impacted Files Coverage Δ
pgcli/pgcompleter.py 96.95% <ø> (ø)
pgcli/pgtoolbar.py 28.94% <0.00%> (-2.49%) ⬇️
pgcli/pyev.py 15.38% <15.38%> (ø)
pgcli/explain_output_formatter.py 41.66% <41.66%> (ø)
pgcli/completion_refresher.py 90.58% <50.00%> (-1.08%) ⬇️
pgcli/key_bindings.py 52.94% <50.00%> (-0.19%) ⬇️
pgcli/pgexecute.py 80.05% <73.68%> (-1.49%) ⬇️
pgcli/main.py 75.82% <80.00%> (-0.11%) ⬇️
... and 1 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e0a4c18...f696e30. Read the comment docs.

@amjith

amjith commented Aug 15, 2021

Copy link
Copy Markdown
Member

The explain plan looks gorgeous. 😄

I'm still playing around with it, haven't had a chance to look through the code yet.

Looks like some of the colors you've used from click aren't available in the older versions of click. So we should consider requiring a modern version of click in setup.py.

@amjith

amjith commented Aug 15, 2021

Copy link
Copy Markdown
Member

@dkuku Instead of binding the explain mode to F5 key can we detect an EXPLAIN query and automatically display the results using ExplainVisualizer?

@dkuku

dkuku commented Aug 15, 2021

Copy link
Copy Markdown
Contributor Author

@amjith we can but you need to pass some params to the explain query and when these are missing it may crash due to missing fields or the format (it needs json). I was tinking more of having an easy way to show the explain for your current query. I think its more useful than having a way to switch between vim/emacs mode with a shortcut because you use one or the other?

@j-bennet

Copy link
Copy Markdown
Contributor

Hey @dkuku, sorry it's been forever since you submitted this PR, but I think it's good to merge. Would you be able to rebase it on top of main and resolve the conflicts?

@dkuku

dkuku commented Mar 27, 2022

Copy link
Copy Markdown
Contributor Author

Hi @j-bennet I merged master into this branch.

@j-bennet
j-bennet merged commit 372da81 into dbcli:main Apr 4, 2022
@j-bennet

j-bennet commented Apr 4, 2022

Copy link
Copy Markdown
Contributor

Merged. Nice work! 🍫

DiegoDAF added a commit to DiegoDAF/pgcli.daf that referenced this pull request Jul 16, 2026
In pgexecute.run(), the control flow was `if explain_mode: prepend EXPLAIN
prefix / elif pgspecial: <detect+run special>`. Because it was if/elif, when
explain mode (F5) was ON the pgspecial branch never ran, so \q, \d, exit,
named queries, \i, \autocommit, \G, etc. were turned into
`EXPLAIN (...) <command>` and shipped to the server as invalid SQL. The user
could not even quit -- typing \q gave `syntax error at or near "\"`, which is
especially painful after an idle-timeout reconnect (the reconnect retries the
command and it errors again).

Fix: detect special commands FIRST (drop the `elif`, make it `if pgspecial:`),
and apply the EXPLAIN prefix ONLY to genuine SQL, right before
execute_normal_sql(). Special commands now keep working while explain mode is
on. Side effects: \G is now stripped correctly under explain mode, and the
restrict-mode meta-command guard (CVE-2025-8714) stays authoritative instead
of being bypassed by turning on F5.

This is an UPSTREAM bug (dbcli/pgcli, present since the explain visualizer was
added in dbcli#1279); candidate for an upstream PR.

Tests: 4 new @dbtest in test_pgexecute.py (special not wrapped, describe runs
as special, normal SQL still wrapped, \G stripped). 3 of them fail on the old
code, all pass on the fix. Full suite 2969 passed (no DB) / all green with DB;
ruff + mypy clean. Bump 4.5.6 -> 4.5.7.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants