feat(sql): add EXPLAIN query plans for SQLAlchemy panel#5
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds EXPLAIN query plan functionality to the SQLAlchemy panel, enabling developers to analyze database query execution plans directly from the debug toolbar. The implementation supports PostgreSQL, SQLite, and MySQL dialects with both standard EXPLAIN and EXPLAIN ANALYZE options.
Key Changes
- New ExplainExecutor class that generates dialect-specific EXPLAIN SQL and executes queries with proper async/sync engine support
- API endpoint
/_debug_toolbar/api/explainfor executing EXPLAIN queries from the UI - Enhanced QueryTracker to record query metadata (hash, dialect, EXPLAIN support) and track connections for proper cleanup
Reviewed changes
Copilot reviewed 21 out of 22 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
src/debug_toolbar/extras/advanced_alchemy/panel.py |
Core EXPLAIN functionality: ExplainExecutor class, QueryTracker enhancements, global event listener support |
src/debug_toolbar/extras/advanced_alchemy/__init__.py |
Exports new ExplainExecutor, QueryTracker, and get_query_tracker for public API |
src/debug_toolbar/litestar/routes/handlers.py |
New route handlers file with toolbar API/UI controllers, EXPLAIN endpoint, and embedded CSS/JS |
src/debug_toolbar/litestar/routes/__init__.py |
Module initialization exporting create_toolbar_router |
src/debug_toolbar/litestar/plugin.py |
Plugin updates to create toolbar router, register toolbar in app state, and share toolbar instance |
src/debug_toolbar/litestar/middleware.py |
Middleware updated to accept shared toolbar instance and add data-request-id attribute |
tests/unit/test_sqlalchemy_panel.py |
Comprehensive test coverage: 27 new tests for ExplainExecutor and QueryTracker EXPLAIN fields |
tests/integration/test_toolbar_routes.py |
New integration tests for toolbar routes, API endpoints, static assets, and full workflow |
tests/integration/test_litestar_middleware.py |
Updated test to check for data-request-id attribute instead of text content |
examples/litestar_advanced_alchemy/app.py |
Example app updated to store db_engine in app state for EXPLAIN queries |
examples/litestar_basic/app.py |
Import order correction |
examples/profiling_panel_example.py |
f-string removed (cosmetic fix) |
package.json |
Added Biome tooling for JavaScript/CSS linting and formatting |
biome.json |
Biome configuration for frontend code quality |
.gitignore |
Added Node.js/Bun ignore patterns |
tests/__init__.py, tests/unit/__init__.py, tests/integration/__init__.py |
Simplified docstrings |
| Documentation files | Updated API documentation structure for new routes module |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @staticmethod | ||
| def _is_explainable(sql: str) -> bool: | ||
| """Check if a SQL statement can be explained.""" | ||
| sql_upper = sql.upper().strip() | ||
| return sql_upper.startswith(("SELECT", "INSERT", "UPDATE", "DELETE", "WITH")) |
There was a problem hiding this comment.
The _is_explainable() validation is insufficient for preventing SQL injection. It only checks if the query starts with certain keywords, but doesn't prevent:
- Multiple statements separated by semicolons (e.g.,
"SELECT 1; DROP TABLE users;") - Comments that could hide malicious SQL
- Nested statements or SQL that executes additional commands
Consider using a SQL parser library like sqlparse to validate that the SQL is a single, safe statement, or implement more robust validation logic.
There was a problem hiding this comment.
Won't fix - same rationale as above. The debug toolbar should only be enabled in development with trusted users. Adding a SQL parser like sqlparse would add a dependency for minimal benefit in a dev-only tool.
| except Exception: | ||
| return False |
There was a problem hiding this comment.
Overly broad exception handling without logging. The _setup_global_listeners() function silently swallows all exceptions and returns False, making it difficult to diagnose why listener attachment failed. This could hide import errors, permission issues, or other unexpected problems.
Consider:
- Catching specific exceptions (e.g.,
ImportError,AttributeError) - Logging the exception for debugging purposes
- Re-raising critical exceptions that indicate configuration problems
Example:
try:
from sqlalchemy.engine import Engine as SAEngine
event.listen(SAEngine, "before_cursor_execute", _tracker.before_cursor_execute)
event.listen(SAEngine, "after_cursor_execute", _tracker.after_cursor_execute)
_global_listeners_attached = True
return True
except ImportError:
logger.debug("SQLAlchemy not available for global listeners")
return False
except Exception as e:
logger.warning(f"Failed to attach global listeners: {e}")
return FalseThere was a problem hiding this comment.
Won't fix - the silent failure is intentional. If SQLAlchemy isn't available, the panel simply won't track queries rather than crashing. Debug-level logging would be excessive for a dev tool.
| global _global_listeners_attached # noqa: PLW0603 | ||
| if _global_listeners_attached: | ||
| return True | ||
|
|
||
| try: | ||
| from sqlalchemy.engine import Engine as SAEngine | ||
|
|
||
| event.listen(SAEngine, "before_cursor_execute", _tracker.before_cursor_execute) | ||
| event.listen(SAEngine, "after_cursor_execute", _tracker.after_cursor_execute) | ||
| _global_listeners_attached = True | ||
| return True |
There was a problem hiding this comment.
Potential race condition with global state. The _global_listeners_attached flag is checked and set without synchronization. In a multi-threaded or async environment, multiple threads/tasks could simultaneously check this flag, see False, and all attempt to attach listeners, potentially causing duplicate listener registration or race conditions.
While this might not cause immediate problems (SQLAlchemy event listeners can handle duplicates), it could lead to unexpected behavior or performance degradation. Consider using a thread-safe mechanism like threading.Lock() or making the function idempotent by checking if listeners are already attached before adding them.
There was a problem hiding this comment.
Won't fix - the race condition is benign. If duplicate listeners are attached, SQLAlchemy handles this gracefully. Adding thread synchronization would add complexity for negligible benefit in a debug tool.
JacobCoffee
force-pushed
the
explain-plans
branch
from
96a31f2 to
a1a0390
Compare
|
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 8 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Add ExplainExecutor class to execute EXPLAIN queries across different database dialects (PostgreSQL, SQLite, MySQL, MariaDB). Features: - Multi-dialect EXPLAIN prefix support - Parameter substitution for valid SQL execution - Query hash generation for fingerprinting - Proper error handling with informative messages 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Add panel_display_depth, panel_display_max_items, and panel_display_max_string
settings to DebugToolbarConfig for controlling nested data rendering.
Defaults:
- panel_display_depth: 10 (was 3)
- panel_display_max_items: 100 (was 20)
- panel_display_max_string: 1000 (was 500)
This prevents "{X keys}" truncation in panel data display.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <noreply@anthropic.com>
JacobCoffee
force-pushed
the
explain-plans
branch
from
ee521ed to
e7cb53d
Compare
UI improvements: - Add EXPLAIN button to SQL panel queries with modal display - Add /api/explain POST endpoint for executing EXPLAIN queries - Improve toolbar layout for top/bottom positions (full width) - Add position controls separator and theme toggle - Use Base64 encoding for SQL data attributes (fixes JS escaping) Example app: - Add on_startup hook to store db_engine for EXPLAIN queries 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
JacobCoffee
force-pushed
the
explain-plans
branch
from
e7cb53d to
76aac4f
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 8 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Fix import order in example apps (litestar before debug_toolbar) - Document new panel display config fields in docstring - Remove unsupported Oracle/MSSQL from DIALECT_EXPLAIN_PREFIX - Use safer PostgreSQL EXPLAIN without ANALYZE - Remove unused _dialect_name field from QueryTracker 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Summary
/_debug_toolbar/api/explainfor executing EXPLAIN queriesChanges
ExplainExecutorclass with multi-dialect EXPLAIN SQL generationplugin.toolbarwhenapp.stateis not availabledata-request-idattribute to toolbar div for testingdb_enginein app stateTest plan
🤖 Generated with Claude Code