Skip to content

fix: handle trailing SQL comments in multiline submit #1576

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
bobo-xxx wants to merge 1 commit into
base: main
Choose a base branch
from
+4 −2
Open

fix: handle trailing SQL comments in multiline submit #1576

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
4 changes: 3 additions & 1 deletion pgcli/pgbuffer.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
from prompt_toolkit.filters import Condition
from prompt_toolkit.application import get_app
from .packages.parseutils.utils import is_open_quote
import sqlparse

_logger = logging.getLogger(__name__)

Expand All @@ -12,7 +13,8 @@ def _is_complete(sql):
# A complete command is an sql statement that ends with a semicolon, unless
# 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)
Copy link

Copilot AI Apr 18, 2026

Choose a reason for hiding this comment

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

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()).

Suggested change
sql_for_check = sqlparse.format(sql, strip_comments=True)
sql_for_check = sqlparse.format(sql, strip_comments=True).rstrip()

Copilot uses AI. Check for mistakes.
return sql_for_check.endswith(";") and not is_open_quote(sql)


"""
Expand Down
2 changes: 1 addition & 1 deletion pgcli/pgexecute.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -361,7 +361,7 @@ def run(
# 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()
if not sql:
Comment on lines 363 to 366
Copy link

Copilot AI Apr 18, 2026

Choose a reason for hiding this comment

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

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.

Copilot uses AI. Check for mistakes.
Comment on lines 363 to 366
Copy link

Copilot AI Apr 18, 2026

Choose a reason for hiding this comment

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

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.

Copilot uses AI. Check for mistakes.
continue
Expand Down
Loading