Repository navigation
fix(query-safety): ask before EXPLAIN ANALYZE runs a write - #1873
Open
notlakshitha wants to merge 3 commits into
Open
notlakshitha wants to merge 3 commits into
notlakshitha wants to merge 3 commits into
Conversation
A PostgreSQL WITH is operated by its outer SELECT, so the confirmation gate only noticed UPDATE … SET inside the CTE list. DELETE, MERGE, and INSERT in that list now ask too. Those words are read from the statement's code, and only before the keyword that operates the statement, so a quoted or commented word still does not ask and a plain WITH … SELECT does not either. CockroachDB uses the same grammar. Closes libredb#1862
On PostgreSQL, EXPLAIN ANALYZE executes the statement it explains. The confirmation gate read the operative keyword EXPLAIN and never looked past it, so a typed EXPLAIN ANALYZE DELETE or DROP ran with no dialog. The gate now strips ANALYZE, ANALYSE and VERBOSE, or a parenthesised option list, and when ANALYZE is on asks about the remaining statement. ANALYZE set to false, off or 0, and a plain EXPLAIN, only plan, so they do not ask. CockroachDB shares the postgres grammar. Closes libredb#1863
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Member
|
I'm working on a VS Code extension for LibreDB Studio. I'll review the open PRs once it's ready. https://marketplace.visualstudio.com/items?itemName=libredb.libredb-studio |
Contributor
Author
|
no rush at all, thanks! the vs code extension looks cool |
cevheri
requested changes
Oct 21, 2026
cevheri
left a comment
Member
There was a problem hiding this comment.
Thanks, the grammar flag is the right home for this and the option-list reading is careful.
Two things before it can go in:
EXPLAIN (ANALYZE 0x1) DELETE FROM usersdeletes every row on PostgreSQL 18.6 and the gate answers false;0b1,0o1and0_1do the same. The number reader stops at the first non-digit, so0x1reads as0.
Done when: ANALYZE counts as off only for a whole value PostgreSQL reads as false, anything else counts as on, and these four spellings are test rows that ask.- This branch sits on the first commit of #1872, so it still carries the CTE-list reading that asks for a column named
merge.
Done when: rebased onto #1872 after it merges, with the docs conflict inquery-optimization.mdresolved and the full gate test file green.
EXPLAIN (ANALYZE 0x1) deletes on PostgreSQL, and so do 0b1, 0o1 and 0_1. The option reader stopped at the first non-digit, so each of those was the integer 0 and the confirmation never opened. ANALYZE is off only when the whole value is a boolean false PostgreSQL accepts (false, off, no, 0 and their prefixes) or an integer literal whose value is 0. Any other value, including a nonzero numeric literal, asks.
Contributor
Author
|
fixed the analyze value parsing, only real false values count as off now (so 0x1, 0b1, 0o1, 0_1 ask for confirmation). will rebase once 1872 is in |
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
On PostgreSQL,
EXPLAIN ANALYZEexecutes the statement it explains. The confirmation gate read the operative keywordEXPLAIN, which is not dangerous, soEXPLAIN ANALYZE DELETEandEXPLAIN ANALYZE DROP TABLEran with no dialog. The gate now strips the option list and asks about the remaining statement whenANALYZEis on.This builds on #1872 (the #1862 CTE confirmation change). A data-modifying CTE written under
EXPLAIN ANALYZEasks for the same reason that CTE asks on its own.Type of Change
Related Issue
Closes #1863
Changes Made
explainExecutesWithAnalyze. CockroachDB uses thepostgrestype-id, so it shares that reading. Other dialects are unchanged.EXPLAINandANALYZE(or the BritishANALYSE) is enabled, the gate strips a run ofANALYZE/ANALYSE/VERBOSE, or a parenthesised option list, and evaluates the remaining text as its own statement.ANALYZEwith no value, or set totrue,onor1, is on.false,offand0are off. A plainEXPLAINonly plans, so it does not ask, including anUPDATE … SETthe unanchored probe would otherwise find. Words inside strings and comments are not options and are not the statement.Testing
Test Environment
bun tests/run-tests.ts tests/components/QuerySafetyDialog.test.tsx— 524 pass.bun run test:coverage— 1702 files, 62528 pass, 10 skip (18 files not run here because Helm or 7z is absent; CI runs those).bun run coverage:check— 180883/180883 lines (100.00%).bun run typecheckandbun run buildpassed.bun run lintreported 0 errors.Screenshots (if applicable)
Not applicable. The dialog is unchanged; only which statements open it moved.
Checklist
bun run test:coverageandbun run coverage:check)src/lib/db/providers/, I updated the matchingdocs/providers/documentation andtests/integration/db/tests in the same PR (provider triad)AGENT_STATEMENT_LANGUAGErow, I kept its agent leg in step: the engine overlay undersrc/lib/agent/languages/<statement-language>/engines/, theAgent language packsubsection of its provider document, andtests/unit/lib/agent/engine-docs.test.tsAdditional Notes
The grammar flag lives on the PostgreSQL row (
src/lib/db/providers/sql/postgres-sql-grammar.ts) anddocs/providers/postgres.mddescribes it. There is no provider runtime change, so the integration suite was left as it is. The agent overlay is untouched.This PR is stacked on #1872: it contains the #1862 commit plus one new commit for #1863. Please review #1872 first; once it merges this diff reduces to the #1863 commit alone.