Add support for -f/--file option to execute SQL from files - #1543
Add support for -f/--file option to execute SQL from files#1543DiegoDAF wants to merge 8 commits into
Conversation
This commit adds support for the -f/--file option to pgcli, similar to psql's behavior. Users can now execute SQL commands from files and exit immediately after execution. Features: - Single file execution: pgcli -f file.sql - Multiple files: pgcli -f file1.sql -f file2.sql - Long form: pgcli --file file.sql - Files are executed sequentially - Pager is automatically disabled in file mode - Proper error handling and exit codes Tests included for all scenarios. Made with ❤️ and 🤖 Claude Code Co-Authored-By: Claude <noreply@anthropic.com>
j-bennet
left a comment
There was a problem hiding this comment.
This looks ready to merge, however, I'm wondering, what will happen if the file contains a destructive command (DROP table_name) and destructive_warning is enabled?
Thanks for the review! Short answer: Concretely, with
One detail worth calling out: Happy to add a behave scenario covering the destructive-file case if you'd like it |
The style gate started failing with ruff 0.15.x: the TimeoutExpired handlers bound `as e` without using it (F841), and step_see_command_output decoded cmd_output into `output` but never used it. The command-output step now asserts on the \dt column headers, which are rendered whether or not any tables exist, so it verifies the special command actually produced its listing.
|
There seems to be a persistent failure in the integration scenario: |
Is the same flake has hit other PRs recently (#1544 and #1609 saw it on different py versions and went green on the next run), so a re-run of the failed job should clear it, I don't have permission to re-trigger it myself. If it keeps biting, I'd be happy to send a tiny separate PR bumping that expect_exact timeout (2s -> 10s); it only affects how long the test waits, not how fast it passes. |
The "edit sql in file with external editor" scenario errors intermittently on slow CI runners: pexpect's expect_exact waited only 2 seconds for the ex-mode banner. It has bitten upstream's CI repeatedly (PRs dbcli#1543/dbcli#1544/dbcli#1609) and now our fork's CI on main (cee716d run: unit 3139 passed, 61 scenarios passed, the only error was this scenario on 3.10 while 3.11 was fully green). All expect timeouts in iocommands.py bumped to 10s. Passing runs are not slowed: pexpect returns as soon as the expected text appears; the timeout only bounds how long a FAILING wait lasts. Verified locally: behave features/iocommands.feature green against a throwaway PG (2 scenarios, 12 steps). Same patch offered to upstream in the dbcli#1543 review thread.
…new PRs triaged
|
Sorry for the slow reply, and thanks for your patience! Yes, it passes locally. I dug into it, and that scenario is flaky rather than Evidence that it is timing and not this PR:
I bumped those Happy to send that as a separate one-line-ish PR against main if you want it |
# Conflicts: # pgcli/main.py
| # This matches psql behavior where the file is treated as one unit | ||
| if file_content.strip(): | ||
| logger.debug("Executing commands from file: %s", input_file) | ||
| self.handle_watch_command(file_content) |
There was a problem hiding this comment.
Why are you using handle_watch_command here, rather than execute_command?
There was a problem hiding this comment.
handle_watch_command is what the interactive loop calls (main.py:1081 on main), so -f takes exactly the same path as typing the file's contents at the prompt. -c in #1542 does the same, for consistency between the two.
Concretely it adds two things over execute_command:
\watchworks inside a file, the way it does inpsql -f.- The query is appended to
self.query_history, which a bare\watch(no command) relies on to pick up the previous query.
Calling execute_command directly would silently drop both. Happy to switch if you would rather -f not support \watch, but then I would make -c match.
There was a problem hiding this comment.
I see. If you want to handle \watch inside a file, that's fine.
I tested the changes briefly. It looks like files with multiple lines aren't handled as expected.
Try this example:
-- save this as ~/tmp/select3.txt
select 1;
select 'hello';
select 2; \watch 2
then if you run psql -f ~/tmp/select3.txt, it will output 1, then hello, then start repeating 2.
However, pgcli -f ~/tmp/select3.txt will start repeating the whole block.
There was a problem hiding this comment.
Good catch, thank you for testing it. Fixed.
The file now goes through sqlparse.split() and runs one statement at a time, each through the same path as the interactive prompt. So \watch repeats only its own statement, and a bare \watch picks up the statement before it through the query history. Your example now behaves exactly like psql -f: 1, hello, then only 2 repeating.
A failed statement stops the rest of the file unless on_error is RESUME, which matches what the single-block execution already did internally.
7 new tests, 5 of which fail without the fix (including one asserting that the repeated output contains only the last statement, and one checking that semicolons inside string literals are not treated as statement boundaries).
There was a problem hiding this comment.
How does sqlparse.split() handle metacommands?
There was a problem hiding this comment.
The same way the code already did, plus one rule it was missing. pgexecute.run() already splits its input with sqlparse.split() internally, so metacommands were going through this exact splitter before the fix too.
But your question made me check the edge, and there was a real pre-existing gap: sqlparse.split only cuts at semicolons, so a metacommand followed by SQL on the next line traveled as one chunk, and the metacommand swallowed the SQL. \echo hi followed by a select turned the select into echo arguments, in a file, on main as well. Interactively it never shows because the buffer submits as soon as it starts with a backslash.
Fixed in the last commit with psql's rule: a backslash command spans only its own line, and the rest of the chunk goes back through the splitter. 3 more tests, all failing without it. Verified against a live server that \d between statements, \dt; with a trailing semicolon, and consecutive metacommands behave identically to the interactive prompt.
get_watch_command()'s regex captures ALL the text before a \watch, so feeding a whole file to handle_watch_command made \watch repeat every statement in it (found by j-bennet reviewing upstream dbcli#1543). Files and -c blocks now go through sqlparse.split() and run statement by statement, like psql: \watch repeats only its own statement, and a bare \watch picks up the previous one through query_history. A failed statement now also makes -f/-c exit 1 when on_error is STOP, like psql with ON_ERROR_STOP; RESUME keeps exit code 0. 9 tests; 5 fail without the fix.
# Conflicts: # changelog.rst
get_watch_command()'s regex captures all the text before a \watch, so feeding the whole file to handle_watch_command made \watch repeat every statement in it. Files now go through sqlparse.split() and run statement by statement, like psql: \watch repeats only its own statement, and a bare \watch picks up the previous one through query_history. A failed statement stops the rest of the file unless on_error is RESUME, matching what the single-block execution already did through pgexecute.run(). 7 tests; 5 fail without the fix.
sqlparse.split only cuts at semicolons, so a metacommand followed by SQL on the next line traveled as one chunk and the metacommand swallowed the SQL. Interactively this never happens because the buffer submits as soon as it starts with a backslash; files now follow psql's rule: a backslash command spans only its own line, and the rest of the chunk goes back through the splitter. 3 tests; all fail without the fix.
Summary
This PR adds support for the
-f/--fileoption to pgcli, implementing psql-compatible behavior for executing SQL commands from files.Features
pgcli -f file.sqlpgcli -f file1.sql -f file2.sqlpgcli --file file.sqlImplementation Details
-f/--filethat accepts multiple file pathsrun_cli()to check for file mode and execute file contents before entering interactive modeecho_via_pager()to disable pager when in file modehandle_watch_command()methodTesting
Comprehensive BDD tests included covering:
-fflag--fileflag-foptions for different filesCompatibility
This implementation follows psql's behavior and maintains backward compatibility with existing functionality.
Made with ❤️ and 🤖 Claude Code
Part of the feature list in discussion #1603: this is item 7 (
-c/--commandand-f/--file), the-fhalf. The-chalf is #1542.