fix: handle trailing SQL comments in multiline submit#1
fix: handle trailing SQL comments in multiline submit#1
Conversation
Fixes two related bugs when a SQL comment follows the semicolon:
- pgbuffer._is_complete(): sql.endswith(';') returned False when a trailing
comment followed the semicolon. In multiline mode, pressing Enter never
submitted the query.
- pgexecute.run(): sql.rstrip(';') couldn't strip the semicolon because
the string ended with comment text. The malformed SQL was sent to
PostgreSQL via psycopg's extended query protocol.
Both locations now use sqlparse.format(sql, strip_comments=True) before
checking for / removing the trailing semicolon.
There was a problem hiding this comment.
Pull request overview
Fixes multiline submission and execution issues when a SQL statement terminator (;) is followed by a trailing SQL comment, by normalizing SQL before semicolon checks/stripping.
Changes:
- Update
PGExecute.run()to strip comments before attempting to remove a trailing semicolon. - Update
pgbuffer._is_complete()to ignore comments when determining whether input is “complete” (ends with;).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| pgcli/pgexecute.py | Alters per-statement cleanup to handle semicolons followed by trailing comments before execution. |
| pgcli/pgbuffer.py | Adjusts multiline completeness detection to treat ; as terminal even when followed by comments. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # Remove spaces, eol and semi-colons. | ||
| sql = sql.rstrip(";") | ||
| sql = sqlparse.format(sql, strip_comments=True).rstrip(";") | ||
| sql = sqlparse.format(sql, strip_comments=False).strip() |
There was a problem hiding this comment.
This change introduces new behavior around semicolon stripping with trailing comments, but there isn’t a test covering the specific case ...; -- trailing comment (or ...; /* trailing */) on the same line. Adding a regression test in tests/test_pgexecute.py would help ensure the query is executed successfully and that only the terminator semicolon is removed (not the comment content).
| sql_for_check = sqlparse.format(sql, strip_comments=True) | ||
| return sql_for_check.endswith(";") and not is_open_quote(sql) |
There was a problem hiding this comment.
sqlparse.format(sql, strip_comments=True) is likely to parse/rewrite the SQL and may leave trailing whitespace/newlines; using it without .strip()/.rstrip() can make endswith(';') unreliable. Also, this adds an additional sqlparse parse/format pass on top of is_open_quote(sql) (which already calls sqlparse.parse), and this Condition can be evaluated frequently in the UI; consider a cheaper approach (e.g., stripping only trailing comments/whitespace before checking for ;) to avoid performance regressions in multiline mode.
| # run each sql query | ||
| for sql in sqlarr: | ||
| # Remove spaces, eol and semi-colons. | ||
| sql = sql.rstrip(";") | ||
| sql = sqlparse.format(sql, strip_comments=True).rstrip(";") | ||
| sql = sqlparse.format(sql, strip_comments=False).strip() |
There was a problem hiding this comment.
sqlparse.format(sql, strip_comments=True) removes all SQL comments, not just the trailing comment after a semicolon. In this function you intentionally re-add leading comments (lines 355-360) so they show up in logs, but this change strips them back out and also drops any in-query comments/hints (which some users rely on for observability or extensions like pg_hint_plan). Consider using a comment-stripped copy only to locate/remove a trailing semicolon, while keeping the original SQL (including comments) for execution/logging.
Fixes two related bugs when a SQL comment follows the semicolon:
Bug 1 — pgbuffer._is_complete(): sql.endswith(';') returned False when a trailing comment followed the semicolon. In multiline mode, pressing Enter never submitted the query.
Bug 2 — pgexecute.run(): sql.rstrip(';') couldn't strip the semicolon because the string ended with comment text. The malformed SQL was sent to PostgreSQL via psycopg's extended query protocol.
Both locations now use sqlparse.format(sql, strip_comments=True) before checking for / removing the trailing semicolon.
Fixes dbcli#1559