Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
286 changes: 183 additions & 103 deletions web/pgadmin/static/js/components/ReactCodeMirror/CustomEditorView.js
Original file line number Diff line number Diff line change
Expand Up @@ -50,136 +50,216 @@ export default class CustomEditorView extends EditorView {
return this.state.sliceDoc();
}

/* Function to extract query based on position passed */
getQueryAt(currPos) {
try {
if(typeof currPos == 'undefined') {
currPos = this.state.selection.main.head;
/* Helper to check if extracted SQL is a complete/valid query.
* Returns true if the query is complete (can stand alone).
* Returns false if the query is incomplete (e.g., WHERE without SELECT).
*/
isCompleteQuery(startPos, endPos) {
const query = this.state.sliceDoc(startPos, endPos).trim();
if (!query) return true; // Empty is considered "complete" (nothing to expand)

const tree = syntaxTree(this.state);
let hasStatement = false;
let hasOnlyComments = true;
let hasError = false;

tree.iterate({
from: startPos,
to: endPos,
enter: (node) => {
if (node.type.name === 'Statement') {
hasStatement = true;
hasOnlyComments = false;
}
if (node.type.isError) {
hasError = true;
}
// Check for non-comment content
if (node.type.name !== 'LineComment' &&
node.type.name !== 'BlockComment' &&
node.type.name !== 'Script') {
hasOnlyComments = false;
}
}
const tree = syntaxTree(this.state);
});

let origLine = this.state.doc.lineAt(currPos);
let startPos = currPos;
// Comments alone are considered complete (they're a valid block)
if (hasOnlyComments && !hasStatement) {
return 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') {
// Check if query starts with a comment - if so, consider it complete
// (comments followed by queries should stop at blank line)
const trimmedQuery = query.replace(/^\s+/, '');
if (trimmedQuery.startsWith('--') || trimmedQuery.startsWith('/*')) {
return true;
}

// Get the first word/token of the query (ignoring leading whitespace)
const firstWord = trimmedQuery.split(/[\s\n\r(;]+/)[0].toUpperCase();

// Valid SQL statement starters - a complete query must start with one of these
const validStarters = [
'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',
];

// If the query doesn't start with a valid statement starter, it's incomplete
if (!validStarters.includes(firstWord)) {
return false;
}

// A statement without syntax errors is complete
return hasStatement && !hasError;
}

/* 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;
}

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;
}

// 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')) {
// 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;
}

// Skip the comments
if(node.type.name == 'LineComment' || node.type.name == 'BlockComment') {
startPos = node.from - 1;
// comments are valid text
validTextFound = true;
continue;
}
// Statement found for the first time
if(node.type.name == 'Statement') {
statementStartPos = node.from;
maxEndPos = node.to;

// sometimes, node type is child of statement.
while(node.type.name != 'Statement' && node.parent) {
node = node.parent;
if(node.from >= currLine.from) {
startPos = node.from;
}
}

// 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;
}
validTextFound = true;
startPos -= 1;
}

// statement found for the first time
if(node.type.name == 'Statement') {
statementStartPos = node.from;
maxEndPos = node.to;
// 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 the statement is on the same line, jump to stmt start
if(node.from >= currLine.from) {
startPos = node.from;
}
}
// If empty line in between then that's it
if(currLine.text.trim() == '' && stopAtBlankLine) {
break;
}

validTextFound = true;
startPos -= 1;
let node = tree.resolve(endPos);
// Skip the comments
if(node.type.name == 'LineComment' || node.type.name == 'BlockComment') {
endPos = node.to + 1;
continue;
}

// 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);
// Skip any other types
if(node.type.name != 'Statement') {
endPos += 1;
continue;
}

// If empty line in between then that's it
if(currLine.text.trim() == '') {
break;
}
// Can't go beyond a statement
if(node.type.name == 'Statement') {
maxEndPos = node.to;
}

let node = tree.resolve(endPos);
// Skip the comments
if(node.type.name == 'LineComment' || node.type.name == 'BlockComment') {
endPos = node.to + 1;
continue;
}
if(currLine.to < maxEndPos) {
endPos = currLine.to + 1;
} else {
endPos += 1;
}
}

// Skip any other types
if(node.type.name != 'Statement') {
endPos += 1;
continue;
}
// Make sure start and end are valid values
if(startPos < 0) startPos = 0;
if(endPos > this.state.doc.length) endPos = this.state.doc.length;
Comment on lines +238 to +240
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Clamp startPos to the document length.

Line 239 only guards the lower bound. startPos can become currLine.to + 1 (Line 151) on a trailing blank line, which can exceed doc.length and return an invalid from. Add an upper-bound clamp alongside the existing checks.

🛠️ Proposed fix
-    if(startPos < 0) startPos = 0;
+    if(startPos < 0) startPos = 0;
+    if(startPos > this.state.doc.length) startPos = this.state.doc.length;
     if(endPos > this.state.doc.length) endPos = this.state.doc.length;
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// Make sure start and end are valid values
if(startPos < 0) startPos = 0;
if(endPos > this.state.doc.length) endPos = this.state.doc.length;
// Make sure start and end are valid values
if(startPos < 0) startPos = 0;
if(startPos > this.state.doc.length) startPos = this.state.doc.length;
if(endPos > this.state.doc.length) endPos = this.state.doc.length;
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@web/pgadmin/static/js/components/ReactCodeMirror/CustomEditorView.js` around
lines 238 - 240, Clamp startPos to the document length as well as the lower
bound: after computing startPos (which can be set to currLine.to + 1 in the
codepath around where currLine is used), add an upper-bound check similar to
endPos so if startPos > this.state.doc.length set startPos =
this.state.doc.length; ensure you update the existing bounds block that
currently checks "if(startPos < 0) startPos = 0;" and "if(endPos >
this.state.doc.length) endPos = this.state.doc.length;" to include the startPos
upper clamp.


// can't go beyond a statement
if(node.type.name == 'Statement') {
maxEndPos = node.to;
}
return { startPos, endPos };
}

if(currLine.to < maxEndPos) {
endPos = currLine.to + 1;
} else {
endPos +=1;
}
/* Function to extract query based on position passed */
getQueryAt(currPos) {
try {
if(typeof currPos == 'undefined') {
currPos = this.state.selection.main.head;
}
const tree = syntaxTree(this.state);

// make sure start and end are valid values;
if(startPos < 0) startPos = 0;
if(endPos > this.state.doc.length) endPos = this.state.doc.length;
// First pass: find boundaries with blank lines as boundaries
let { startPos, endPos } = this._findQueryBoundaries(currPos, tree, true);

// Check if the result is a complete query
if (!this.isCompleteQuery(startPos, endPos)) {
// Query is incomplete, try expanding by ignoring blank lines
const expanded = this._findQueryBoundaries(currPos, tree, false);
startPos = expanded.startPos;
endPos = expanded.endPos;
}

return {
value: this.state.sliceDoc(startPos, endPos).trim(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -115,4 +115,34 @@ 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});
});

});
Loading