Fixed an issue where EXPLAIN and EXPLAIN ANALYZE failed when blank lines separate clauses in the SQL query. (#9875)

getQueryAt now uses the syntax tree as the primary check for whether a
blank-line boundary cut through a SQL statement. It is split into two
helpers: _findQueryBoundaries (the original scan, parameterized by
stopAtBlankLine) and _needsExpansion, which detects when a Statement
node straddles the extracted range and re-scans ignoring blank lines.

A STATEMENT_STARTERS keyword list guards the case where the parser
merges semicolon-less queries into one Statement, and WRAPPER_STARTERS
(EXPLAIN, ANALYZE, WITH) force expansion when the Statement extends past
the range. Also fixes a Lezer boundary bug (tree.iterate is inclusive at
boundaries) with a node.to > startPos check. Adds 21 tests covering
EXPLAIN, negative no-merge cases, comments, and boundary edge cases.
This commit is contained in:
Ashesh Vashi authored and Dave Page committed 2026-06-09 15:10:07 +01:00
1 parent 0223e8a026
commit 397be4e70e
3 files changed
+416 -120

No files matched your search

+1
View File
@@ -35,6 +35,7 @@ Bug fixes
*********
| `Issue #9098 <https://github.com/pgadmin-org/pgadmin4/issues/9098>`_ - Fixed an issue where the query tool displayed 'default' instead of 'null' for null text data in the data output.
| `Issue #9875 <https://github.com/pgadmin-org/pgadmin4/issues/9875>`_ - Fixed an issue where EXPLAIN and EXPLAIN ANALYZE failed to execute when blank lines separated clauses in the SQL query.
| `Issue #9988 <https://github.com/pgadmin-org/pgadmin4/issues/9988>`_ - Provide an actionable error when 'openid' is in OAUTH2_SCOPE but OAUTH2_SERVER_METADATA_URL is not set, instead of a cryptic Authlib failure.
| `Issue #9125 <https://github.com/pgadmin-org/pgadmin4/issues/9125>`_ - Fixed an issue where the pgAdmin configuration database wasn't being created on a fresh install when an external database was used for the configuration.
| `Issue #9157 <https://github.com/pgadmin-org/pgadmin4/issues/9157>`_ - Fixed an issue where shortcuts are not working as expected on multiple keyboard layouts.
@@ -21,6 +21,25 @@ import { clearBreakpoints, hasBreakpoint, toggleBreakpoint } from './extensions/
import { autoCompleteCompartment, eol, eolCompartment } from './extensions/extraStates';
// Keywords that can begin a standalone SQL statement. Used by
// _needsExpansion to distinguish "new query after blank line" from
// "clause continuation after blank line".
const STATEMENT_STARTERS = new Set([
'SELECT', 'INSERT', 'UPDATE', 'DELETE', 'CREATE', 'ALTER', 'DROP',
'TRUNCATE', 'GRANT', 'REVOKE', 'COMMIT', 'ROLLBACK', 'BEGIN',
'END', 'SAVEPOINT', 'RELEASE', 'SET', 'SHOW', 'EXPLAIN', 'ANALYZE',
'VACUUM', 'REINDEX', 'CLUSTER', 'COMMENT', 'COPY', 'DO', 'LOCK',
'NOTIFY', 'LISTEN', 'UNLISTEN', 'LOAD', 'RESET', 'DISCARD',
'DECLARE', 'FETCH', 'MOVE', 'CLOSE', 'PREPARE', 'EXECUTE',
'DEALLOCATE', 'WITH', 'TABLE', 'VALUES', 'CALL', 'IMPORT', 'MERGE',
'REFRESH', 'SECURITY', 'REASSIGN', 'ABORT', 'START', 'CHECKPOINT',
]);
// Wrapper keywords that MUST be followed by another statement —
// they can never stand alone, so a blank line after them always
// means the statement was split and needs expansion.
const WRAPPER_STARTERS = new Set(['EXPLAIN', 'ANALYZE', 'WITH']);
function getAutocompLoading({ bottom, left }, dom) {
const cmRect = dom.getBoundingClientRect();
const div = document.createElement('div');
@@ -50,6 +69,187 @@ export default class CustomEditorView extends EditorView {
return this.state.sliceDoc();
}
/* Check whether a blank-line boundary cut through a SQL statement.
*
* Uses two checks:
* 1. Syntax tree — does a Statement node cross either boundary of
* [startPos, endPos]? "Straddles start" means the range begins
* mid-statement; "extends past end" means the range captured only
* a prefix of the statement. If neither, no expansion is needed.
* 2. First-word heuristic — wrapper keywords (EXPLAIN, ANALYZE, WITH)
* always force expansion when the Statement extends past endPos.
* For the straddle-start case, expansion is skipped only when
* the range starts with a standalone statement-starting keyword
* (handles the parser merging semicolon-less queries into one
* Statement).
*
* Returns true when expansion is needed, false otherwise.
*/
_needsExpansion(startPos, endPos) {
const query = this.state.sliceDoc(startPos, endPos).trim();
if (!query) return false;
const tree = syntaxTree(this.state);
let statementStraddlesStart = false;
let statementExtendsPastEnd = false;
tree.iterate({
from: startPos,
to: endPos,
enter: (node) => {
if (node.type.name !== 'Statement') return;
if (node.from < startPos && node.to > startPos) {
statementStraddlesStart = true;
}
if (node.from < endPos && node.to > endPos) {
statementExtendsPastEnd = true;
}
}
});
// No Statement crosses either boundary — range is self-contained.
if (!statementStraddlesStart && !statementExtendsPastEnd) return false;
// Comment-only blocks are self-contained — don't expand.
if (query.startsWith('--') || query.startsWith('/*')) return false;
const firstWord = query.split(/[\s(;]+/)[0].toUpperCase();
// Wrapper keywords (EXPLAIN, ANALYZE, WITH) always need a trailing
// statement; expand when the Statement extends past our range.
if (statementExtendsPastEnd && WRAPPER_STARTERS.has(firstWord)) return true;
// Statement straddles startPos — expand unless the range starts
// with a standalone statement-starting keyword (which indicates a
// new query, not a continuation of the previous one).
if (statementStraddlesStart) return !STATEMENT_STARTERS.has(firstWord);
return false;
}
/* Internal helper to find query boundaries with blank line as boundary */
_findQueryBoundaries(currPos, tree, stopAtBlankLine = true) {
let origLine = this.state.doc.lineAt(currPos);
let startPos = currPos;
// Move the startPos to a known node type or a space
for(;startPos<origLine.to; startPos++) {
let node = tree.resolve(startPos);
if(node.type.name != 'Script') {
break;
}
const currChar = this.state.sliceDoc(startPos, startPos+1);
if(currChar == ' ' || currChar == '\t') {
break;
}
}
let maxEndPos = this.state.doc.length;
let statementStartPos = -1;
let validTextFound = false;
// Go in reverse direction to get the start position
while(startPos >= 0) {
const currLine = this.state.doc.lineAt(startPos);
// If empty line then start with prev line
// If empty line in between then that's it
if(currLine.text.trim() == '') {
if(origLine.number != currLine.number && stopAtBlankLine) {
startPos = currLine.to + 1;
break;
}
startPos = currLine.from - 1;
continue;
}
// Script type doesn't give any info, better skip it
const currChar = this.state.sliceDoc(startPos, startPos+1);
let node = tree.resolve(startPos);
if(node.type.name == 'Script' || (currChar == '\n')) {
startPos -= 1;
continue;
}
// Skip the comments
if(node.type.name == 'LineComment' || node.type.name == 'BlockComment') {
startPos = node.from - 1;
validTextFound = true;
continue;
}
// Sometimes, node type is child of statement
while(node.type.name != 'Statement' && node.parent) {
node = node.parent;
}
// We already had found valid text
if(validTextFound) {
if(statementStartPos >= 0 && statementStartPos < startPos) {
startPos -= 1;
continue;
}
startPos = node.to;
break;
}
// Statement found for the first time
if(node.type.name == 'Statement') {
statementStartPos = node.from;
maxEndPos = node.to;
if(node.from >= currLine.from) {
startPos = node.from;
}
}
validTextFound = true;
startPos -= 1;
}
// Move forward from start position
let endPos = startPos + 1;
maxEndPos = maxEndPos == -1 ? this.state.doc.length : maxEndPos;
while(endPos < maxEndPos) {
const currLine = this.state.doc.lineAt(endPos);
// If empty line in between then that's it
if(currLine.text.trim() == '' && stopAtBlankLine) {
break;
}
let node = tree.resolve(endPos);
// Skip the comments
if(node.type.name == 'LineComment' || node.type.name == 'BlockComment') {
endPos = node.to + 1;
continue;
}
// Skip any other types
if(node.type.name != 'Statement') {
endPos += 1;
continue;
}
// Can't go beyond a statement
if(node.type.name == 'Statement') {
maxEndPos = node.to;
}
if(currLine.to < maxEndPos) {
endPos = currLine.to + 1;
} else {
endPos += 1;
}
}
// Make sure start and end are valid values
if(startPos < 0) startPos = 0;
if(endPos > this.state.doc.length) endPos = this.state.doc.length;
return { startPos, endPos };
}
/* Function to extract query based on position passed */
getQueryAt(currPos) {
try {
@@ -58,129 +258,19 @@ export default class CustomEditorView extends EditorView {
}
const tree = syntaxTree(this.state);
let origLine = this.state.doc.lineAt(currPos);
let startPos = currPos;
// First pass: find boundaries treating blank lines as boundaries
let { startPos, endPos } = this._findQueryBoundaries(currPos, tree, true);
// Move the startPos a known node type or a space.
// We don't want to be in an unknown teritory
for(;startPos<origLine.to; startPos++) {
let node = tree.resolve(startPos);
if(node.type.name != 'Script') {
break;
}
const currChar = this.state.sliceDoc(startPos, startPos+1);
if(currChar == ' ' || currChar == '\t') {
break;
}
// If a blank-line boundary cut through a Statement node, the
// extracted range is a fragment. Retry ignoring blank lines so
// the full statement (e.g. SELECT … FROM … WHERE across blank
// lines, or EXPLAIN followed by SELECT) is returned.
if (this._needsExpansion(startPos, endPos)) {
const expanded = this._findQueryBoundaries(currPos, tree, false);
startPos = expanded.startPos;
endPos = expanded.endPos;
}
let maxEndPos = this.state.doc.length;
let statementStartPos = -1;
let validTextFound = false;
// we'll go in reverse direction to get the start position.
while(startPos >= 0) {
const currLine = this.state.doc.lineAt(startPos);
// If empty line then start with prev line
// If empty line in between then that's it
if(currLine.text.trim() == '') {
if(origLine.number != currLine.number) {
startPos = currLine.to + 1;
break;
}
startPos = currLine.from - 1;
continue;
}
// Script type doesn't give any info, better skip it.
const currChar = this.state.sliceDoc(startPos, startPos+1);
let node = tree.resolve(startPos);
if(node.type.name == 'Script' || (currChar == '\n')) {
startPos -= 1;
continue;
}
// Skip the comments
if(node.type.name == 'LineComment' || node.type.name == 'BlockComment') {
startPos = node.from - 1;
// comments are valid text
validTextFound = true;
continue;
}
// sometimes, node type is child of statement.
while(node.type.name != 'Statement' && node.parent) {
node = node.parent;
}
// We already had found valid text
if(validTextFound) {
// continue till it reaches start so we can check for empty lines, etc.
if(statementStartPos >= 0 && statementStartPos < startPos) {
startPos -= 1;
continue;
}
// don't go beyond this
startPos = node.to;
break;
}
// statement found for the first time
if(node.type.name == 'Statement') {
statementStartPos = node.from;
maxEndPos = node.to;
// if the statement is on the same line, jump to stmt start
if(node.from >= currLine.from) {
startPos = node.from;
}
}
validTextFound = true;
startPos -= 1;
}
// move forward from start position
let endPos = startPos+1;
maxEndPos = maxEndPos == -1 ? this.state.doc.length : maxEndPos;
while(endPos < maxEndPos) {
const currLine = this.state.doc.lineAt(endPos);
// If empty line in between then that's it
if(currLine.text.trim() == '') {
break;
}
let node = tree.resolve(endPos);
// Skip the comments
if(node.type.name == 'LineComment' || node.type.name == 'BlockComment') {
endPos = node.to + 1;
continue;
}
// Skip any other types
if(node.type.name != 'Statement') {
endPos += 1;
continue;
}
// can't go beyond a statement
if(node.type.name == 'Statement') {
maxEndPos = node.to;
}
if(currLine.to < maxEndPos) {
endPos = currLine.to + 1;
} else {
endPos +=1;
}
}
// make sure start and end are valid values;
if(startPos < 0) startPos = 0;
if(endPos > this.state.doc.length) endPos = this.state.doc.length;
return {
value: this.state.sliceDoc(startPos, endPos).trim(),
from: startPos,
@@ -10,6 +10,7 @@
import { withTheme } from '../fake_theme';
import CodeMirror from 'sources/components/ReactCodeMirror';
import { syntaxTree } from '@codemirror/language';
import { render } from '@testing-library/react';
@@ -115,4 +116,208 @@ describe('CodeMirrorCustomEditorView', ()=>{
expect(editor.getQueryAt(29)).toEqual({'value': 'select * from public.actor;', 'from': 0, 'to': 27});
});
it('cursor on WHERE clause with blank lines between SELECT and FROM and WHERE',()=>{
// Query with blank lines between clauses: SELECT *\n\nFROM pg_class\n\nWHERE id = 1;
cmRerender({value: 'SELECT *\n\nFROM pg_class\n\nWHERE id = 1;'});
// Cursor at WHERE clause (position 25), should return full query
expect(editor.getQueryAt(25)).toEqual({'value': 'SELECT *\n\nFROM pg_class\n\nWHERE id = 1;', 'from': 0, 'to': 38});
});
it('cursor on FROM clause with blank lines in query',()=>{
// Query with blank lines between clauses
cmRerender({value: 'SELECT *\n\nFROM pg_class\n\nWHERE id = 1;'});
// Cursor at FROM clause (position 10), should return full query
expect(editor.getQueryAt(10)).toEqual({'value': 'SELECT *\n\nFROM pg_class\n\nWHERE id = 1;', 'from': 0, 'to': 38});
});
it('cursor on condition line with WHERE on separate line',()=>{
// Query: select *\n\nfrom pg_attribute\n\nWHERE\n\nattrelid > 3000;
const query = 'select *\n\nfrom pg_attribute\n\nWHERE\n\nattrelid > 3000;';
cmRerender({value: query});
// Cursor at 'attrelid' condition (position 38), should return full query
expect(editor.getQueryAt(38)).toEqual({'value': query, 'from': 0, 'to': query.length});
});
it('cursor on WHERE keyword with blank lines around it',()=>{
// Query: select *\n\nfrom pg_attribute\n\nWHERE\n\nattrelid > 3000;
const query = 'select *\n\nfrom pg_attribute\n\nWHERE\n\nattrelid > 3000;';
cmRerender({value: query});
// Cursor at WHERE keyword (position 30), should return full query
expect(editor.getQueryAt(30)).toEqual({'value': query, 'from': 0, 'to': query.length});
});
it('EXPLAIN ANALYZE with blank lines, cursor on WHERE',()=>{
const query = 'EXPLAIN ANALYZE SELECT *\n\nFROM pg_class\n\nWHERE oid > 1000;';
cmRerender({value: query});
expect(editor.getQueryAt(42)).toEqual({'value': query, 'from': 0, 'to': query.length});
});
it('EXPLAIN with cursor on FROM clause',()=>{
const query = 'EXPLAIN SELECT *\n\nFROM pg_class\n\nWHERE oid > 1000;';
cmRerender({value: query});
expect(editor.getQueryAt(18)).toEqual({'value': query, 'from': 0, 'to': query.length});
});
it('EXPLAIN with cursor on EXPLAIN keyword returns full query across blank lines',()=>{
const query = 'EXPLAIN SELECT *\n\nFROM pg_class\n\nWHERE oid > 1000;';
cmRerender({value: query});
// Cursor on EXPLAIN (position 4) — must expand past blank lines
expect(editor.getQueryAt(4)).toEqual({'value': query, 'from': 0, 'to': query.length});
});
it('EXPLAIN ANALYZE with cursor on SELECT returns full query',()=>{
const query = 'EXPLAIN ANALYZE SELECT *\n\nFROM pg_class\n\nWHERE oid > 1000;';
cmRerender({value: query});
// Cursor on SELECT (position 16) — before any blank line
expect(editor.getQueryAt(16)).toEqual({'value': query, 'from': 0, 'to': query.length});
});
it('two separate queries with semicolons and blank line are not merged',()=>{
const text = 'SELECT * FROM users;\n\nSELECT * FROM orders;';
cmRerender({value: text});
// Cursor on second query (position 22), should return only the second query
expect(editor.getQueryAt(22)).toEqual({'value': 'SELECT * FROM orders;', 'from': 22, 'to': 43});
});
it('two separate queries without semicolons and blank line are not merged',()=>{
const text = 'SELECT * FROM users\n\nSELECT * FROM orders';
cmRerender({value: text});
// Cursor on second query, should return only the second query
expect(editor.getQueryAt(21)).toEqual({'value': 'SELECT * FROM orders', 'from': 21, 'to': 41});
});
it('comment block between queries does not expand into adjacent query',()=>{
const text = 'SELECT 1;\n\n-- This is a comment\n\nSELECT 2;';
cmRerender({value: text});
// Cursor on comment (position 12), should return only the comment
expect(editor.getQueryAt(12)).toEqual({'value': '-- This is a comment', 'from': 11, 'to': 32});
});
it('first query is not expanded into second when cursor on first',()=>{
const text = 'SELECT * FROM users;\n\nSELECT * FROM orders;';
cmRerender({value: text});
// Cursor on first query, should return only the first query
expect(editor.getQueryAt(5)).toEqual({'value': 'SELECT * FROM users;', 'from': 0, 'to': 20});
});
it('cursor on blank line between clauses returns nearest query above',()=>{
// Cursor exactly on the blank line (position 9 = second \n).
// Blank line is a separator — returns the clause above it.
const text = 'SELECT *\n\nFROM pg_class\n\nWHERE id = 1;';
cmRerender({value: text});
expect(editor.getQueryAt(9)).toEqual({'value': 'SELECT *', 'from': 0, 'to': 9});
});
it('single-line EXPLAIN without blank lines',()=>{
// Regression guard: simple EXPLAIN must still work
const query = 'EXPLAIN SELECT * FROM pg_class;';
cmRerender({value: query});
expect(editor.getQueryAt(10)).toEqual({'value': query, 'from': 0, 'to': query.length});
});
it('multiple consecutive blank lines between clauses',()=>{
// 3 blank lines between SELECT and FROM
const query = 'SELECT *\n\n\n\nFROM pg_class;';
cmRerender({value: query});
expect(editor.getQueryAt(12)).toEqual({'value': query, 'from': 0, 'to': query.length});
});
// ----------------------------------------------------------------
// Diagnostic: verify Lezer Statement node boundaries for key cases
// ----------------------------------------------------------------
function getStatementNodes(editorView) {
const tree = syntaxTree(editorView.state);
const stmts = [];
tree.iterate({
enter: (node) => {
if (node.type.name === 'Statement') {
stmts.push({ from: node.from, to: node.to });
}
}
});
return stmts;
}
it('parser: single query with blank lines between clauses is one Statement',()=>{
cmRerender({value: 'SELECT *\n\nFROM pg_class\n\nWHERE id = 1;'});
const stmts = getStatementNodes(editor);
// Must be a single Statement spanning the entire query
expect(stmts.length).toBe(1);
expect(stmts[0].from).toBe(0);
expect(stmts[0].to).toBe(38);
});
it('parser: two queries with semicolons are separate Statements',()=>{
cmRerender({value: 'SELECT 1;\n\nSELECT 2;'});
const stmts = getStatementNodes(editor);
// Must be two separate Statements
expect(stmts.length).toBe(2);
// Statement 1 must NOT extend into Statement 2's range
expect(stmts[0].to).toBeLessThanOrEqual(stmts[1].from);
});
it('parser: two queries without semicolons — verify Statement layout',()=>{
cmRerender({value: 'SELECT * FROM users\n\nSELECT * FROM orders'});
const stmts = getStatementNodes(editor);
// Parser may merge (1 Statement) or separate (2 Statements) — both
// are handled by _needsExpansion. Pin down the exact current behavior
// so a parser update is noticed.
expect([1, 2]).toContain(stmts.length);
if (stmts.length === 1) {
expect(stmts[0].from).toBe(0);
expect(stmts[0].to).toBe(41);
}
});
it('parser: EXPLAIN SELECT is one Statement',()=>{
cmRerender({value: 'EXPLAIN ANALYZE SELECT *\n\nFROM pg_class\n\nWHERE oid > 1000;'});
const stmts = getStatementNodes(editor);
expect(stmts.length).toBe(1);
expect(stmts[0].from).toBe(0);
});
it('parser: Lezer iterate is inclusive at Statement boundary',()=>{
// Verify: tree.iterate({from: stmt1End}) DOES visit Statement 1
// (Lezer boundaries are inclusive). Our _needsExpansion guards
// against this with an additional node.to > startPos check.
cmRerender({value: 'SELECT 1;\n\nSELECT 2;'});
const tree = syntaxTree(editor.state);
const stmts = getStatementNodes(editor);
const stmt1End = stmts[0].to;
// Raw iterate at stmt1End DOES see Statement 1 (Lezer is inclusive)
let seenStmt1AtBoundary = false;
tree.iterate({
from: stmt1End,
to: stmt1End + 10,
enter: (node) => {
if (node.type.name === 'Statement' && node.from === stmts[0].from) {
seenStmt1AtBoundary = true;
}
}
});
expect(seenStmt1AtBoundary).toBe(true); // Lezer IS inclusive
// But _needsExpansion must NOT treat this as needing expansion.
// The query at Statement 2's position should return only Statement 2.
const stmt2Start = stmts[1].from;
const result = editor.getQueryAt(stmt2Start);
expect(result.value).toBe('SELECT 2;');
});
it('query starting exactly at previous Statement boundary is not expanded',()=>{
// Edge case: cursor right after the previous Statement.
// _needsExpansion must not fire because the previous Statement
// does not straddle (contain) startPos.
cmRerender({value: 'SELECT 1;\nSELECT 2;'});
const stmts = getStatementNodes(editor);
expect(stmts.length).toBe(2);
// Cursor on second query — must return only the second query
const result = editor.getQueryAt(stmts[1].from);
expect(result.value).toBe('SELECT 2;');
// from may include the preceding newline (trimmed from value)
expect(result.to).toBe(stmts[1].to);
});
});