fix: handle trailing SQL comments in multiline submit#1576
fix: handle trailing SQL comments in multiline submit#1576bobo-xxx wants to merge 1 commit intodbcli:mainfrom
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 SQL statements end with a semicolon followed by trailing comments (e.g. SELECT 1; -- comment), by attempting to strip comments before checking/removing a trailing semicolon.
Changes:
- Update
PGExecute.run()to usesqlparse.format(..., strip_comments=True)prior to stripping a trailing;. - Update
pgbuffer._is_complete()to use a comment-stripped string when deciding whether input ends with;. - Add
sqlparseimport topgbuffer.py.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| pgcli/pgexecute.py | Adjusts semicolon stripping logic in the query execution loop to account for trailing comments. |
| pgcli/pgbuffer.py | Adjusts multiline “is query complete?” detection to account for trailing comments and adds sqlparse import. |
💡 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() | ||
| if not sql: |
There was a problem hiding this comment.
There are existing pgexecute tests (tests/test_pgexecute.py), but no test case covering the new edge case this change is meant to fix (trailing comment after a semicolon, e.g. SELECT 1; -- comment). Adding a regression test would help ensure semicolon handling works and that comments are preserved/handled as intended.
| # 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() | ||
| if not sql: |
There was a problem hiding this comment.
This change applies sqlparse.format(..., strip_comments=True) directly to sql, which strips comments from the statement being executed/logged. It also risks failing to remove a trailing semicolon when the stripped comment leaves trailing whitespace (e.g. SELECT 1; --x can become SELECT 1; so rstrip(';') is a no-op). Consider using a separate, comment-stripped copy only for the semicolon check/removal, and remove the semicolon from the original SQL (preserving comments), making sure to trim whitespace before testing/removing the semicolon.
| # there's an open quote surrounding it, as is common when writing a | ||
| # CREATE FUNCTION command | ||
| return sql.endswith(";") and not is_open_quote(sql) | ||
| sql_for_check = sqlparse.format(sql, strip_comments=True) |
There was a problem hiding this comment.
sqlparse.format(sql, strip_comments=True) can leave trailing whitespace where the comment was removed, so endswith(';') may still be false for inputs like SELECT 1; -- trailing comment. Strip whitespace on the comment-stripped string before checking for the semicolon (e.g. .strip()/.rstrip()).
| sql_for_check = sqlparse.format(sql, strip_comments=True) | |
| sql_for_check = sqlparse.format(sql, strip_comments=True).rstrip() |
| from prompt_toolkit.enums import DEFAULT_BUFFER | ||
| from prompt_toolkit.filters import Condition | ||
| from prompt_toolkit.application import get_app | ||
| from .packages.parseutils.utils import is_open_quote | ||
| import sqlparse |
There was a problem hiding this comment.
Import ordering: sqlparse (third-party) should be grouped with the other third-party imports (prompt_toolkit) and separated from local imports (the relative .packages... import) to match the import grouping used elsewhere in the codebase (e.g. pgcli/pgexecute.py).
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 #1559