From 9dde7db053972d51819358eca38e0a02657cf7df Mon Sep 17 00:00:00 2001 From: Diego Date: Fri, 16 Oct 2026 14:26:39 -0300 Subject: [PATCH] Exit -c and -f with psql's status codes pgcli -c "select nope" exited 0, so a script calling pgcli could not tell a failed statement from a good one. psql uses the status of the last -c or -f that ran: a failed -c exits 1, a script exits 0 even with errors in it, and with ON_ERROR_STOP the first failure stops everything after it and a failed script exits 3. on_error = STOP (the default) now plays ON_ERROR_STOP and RESUME plays psql's default. The same loop had two more departures from psql: under RESUME a failed -c kept the next -c from running, and under STOP it did not stop the -f files. Both follow psql now. Nine cases, each measured with psql 18 against a live server, plus two tests through a real connection; ten of the eleven fail without the change (the remaining case already matched). The two behave scenarios that ran an invalid query expected a successful exit; they now expect an error, and fail without the change. --- changelog.rst | 10 +++++ pgcli/main.py | 24 ++++++++--- tests/features/command_option.feature | 2 +- tests/features/file_option.feature | 2 +- tests/test_main.py | 62 +++++++++++++++++++++++++++ 5 files changed, 91 insertions(+), 9 deletions(-) diff --git a/changelog.rst b/changelog.rst index 81fbefe78..612eeb82e 100644 --- a/changelog.rst +++ b/changelog.rst @@ -6,6 +6,16 @@ Features: * Add a ``\service`` prompt token for connections made using ``service=...`` or ``PGSERVICE``. For example, set ``prompt = '\service> '``. +Bug fixes: +---------- +* ``-c`` and ``-f`` now exit with psql's status codes instead of always 0. A + failed ``-c`` exits 1. With ``on_error = STOP`` (the default, psql's + ``ON_ERROR_STOP``) the first failure also stops every ``-c`` and ``-f`` after + it, and a failed script exits 3. With ``RESUME`` everything runs and the + status is that of the last ``-c`` or ``-f``, where a script counts as a + success, as in psql. Before, a failed ``-c`` also kept the next ``-c`` from + running under ``RESUME``, and did not stop the ``-f`` files under ``STOP``. + 4.7.1 (2026-09-20) ================== diff --git a/pgcli/main.py b/pgcli/main.py index 994f116e5..9f4a72f97 100644 --- a/pgcli/main.py +++ b/pgcli/main.py @@ -1060,6 +1060,12 @@ def _check_ongoing_transaction_and_allow_quitting(self): def run_cli(self): logger = self.logger + # Exit status, like psql: that of the last -c or -f that ran. A failed + # -c exits 1, while a script exits 0 even with errors in it, unless + # on_error = STOP (psql's ON_ERROR_STOP): then the first failure stops + # everything after it, and a failed script exits 3. + exit_code = 0 + # Handle command mode (-c flag) - similar to psql behavior # Multiple -c options are executed sequentially if hasattr(self, 'commands') and self.commands: @@ -1068,8 +1074,12 @@ def run_cli(self): logger.debug("Running command: %s", command) # Statement by statement, like psql -c: \watch only repeats # its own statement, not the whole -c block. - if not self._execute_statements(command): - break + if self._execute_statements(command): + exit_code = 0 + else: + exit_code = 1 + if self.on_error != "RESUME": + sys.exit(1) except PgCliQuitError: # Normal exit from quit command sys.exit(0) @@ -1081,7 +1091,7 @@ def run_cli(self): # psql runs both -c and -f when they are given together, so only # exit here when there is no file left to run. if not (hasattr(self, 'input_files') and self.input_files): - sys.exit(0) + sys.exit(exit_code) # Handle file mode (-f flag) - similar to psql behavior # Multiple -f options are executed sequentially @@ -1096,8 +1106,9 @@ def run_cli(self): logger.debug("Executing commands from file: %s", input_file) # Statement by statement, like psql -f: \watch only # repeats its own statement, not the whole file. - if not self._execute_statements(file_content): - break + if not self._execute_statements(file_content) and self.on_error != "RESUME": + sys.exit(3) + exit_code = 0 except PgCliQuitError: # Normal exit from quit command @@ -1107,8 +1118,7 @@ def run_cli(self): logger.error("traceback: %r", traceback.format_exc()) click.secho(str(e), err=True, fg="red") sys.exit(1) - # Exit successfully after executing all commands - sys.exit(0) + sys.exit(exit_code) history_file = self.config["main"]["history_file"] if history_file == "default": diff --git a/tests/features/command_option.feature b/tests/features/command_option.feature index 38f5fef51..238d2d334 100644 --- a/tests/features/command_option.feature +++ b/tests/features/command_option.feature @@ -20,7 +20,7 @@ Feature: run the cli with -c/--command option, Scenario: run pgcli with -c and an invalid query When we run pgcli with -c "SELECT invalid_column FROM nonexistent_table" then we see an error message - and pgcli exits successfully + and pgcli exits with error Scenario: run pgcli with -c and multiple statements When we run pgcli with -c "SELECT 1; SELECT 2" diff --git a/tests/features/file_option.feature b/tests/features/file_option.feature index 4533c7c11..63179c907 100644 --- a/tests/features/file_option.feature +++ b/tests/features/file_option.feature @@ -30,4 +30,4 @@ Feature: run the cli with -f/--file option, When we create a file with "SELECT invalid_column FROM nonexistent_table" and we run pgcli with -f and the file then we see an error message - and pgcli exits successfully + and pgcli exits with error diff --git a/tests/test_main.py b/tests/test_main.py index decae97b1..3e2679096 100644 --- a/tests/test_main.py +++ b/tests/test_main.py @@ -602,6 +602,68 @@ def test_file_mode_runs_statements(tmpdir): mock_exec.assert_called_once_with("select 1;\nselect 2;") +# Each case is psql 18 measured against a live server, with on_error = STOP +# standing for psql's ON_ERROR_STOP and RESUME for its default. +@pytest.mark.parametrize( + "on_error, commands, files, expected_code, expected_runs", + [ + # psql -c "select nope" -> 1 + ("STOP", ["bad"], [], 1, ["bad"]), + # psql -v ON_ERROR_STOP=1 -c "select nope" -c "select 9" -> 1, 9 never runs + ("STOP", ["bad", "good"], [], 1, ["bad"]), + # psql -c "select 1" -c "select nope" -> 1, the last -c failed + ("RESUME", ["good", "bad"], [], 1, ["good", "bad"]), + # psql -c "select 1" -c "select nope" -c "select 2" -> 0, the last -c worked + ("RESUME", ["good", "bad", "good"], [], 0, ["good", "bad", "good"]), + # psql -v ON_ERROR_STOP=1 -f err.sql -> 3 + ("STOP", [], ["bad"], 3, ["bad"]), + # psql -v ON_ERROR_STOP=1 -f err.sql -f ok.sql -> 3, ok.sql never runs + ("STOP", [], ["bad", "good"], 3, ["bad"]), + # psql -f err.sql -f ok.sql -> 0, both run + ("RESUME", [], ["bad", "good"], 0, ["bad", "good"]), + # psql -v ON_ERROR_STOP=1 -c "select nope" -f err.sql -> 1, the file never runs + ("STOP", ["bad"], ["good"], 1, ["bad"]), + # psql -c "select nope" -f err.sql -> 0, the script ran last + ("RESUME", ["bad"], ["bad"], 0, ["bad", "bad"]), + ], +) +def test_exit_status_matches_psql(tmpdir, on_error, commands, files, expected_code, expected_runs): + paths = [] + for i, content in enumerate(files): + sql_file = tmpdir.join(f"script{i}.sql") + sql_file.write(content) + paths.append(str(sql_file)) + cli = PGCli(pgclirc_file=str(tmpdir.join("rcfile"))) + cli.on_error = on_error + cli.commands = commands or None + cli.input_files = paths or None + with mock.patch.object(cli, "_execute_statements", side_effect=lambda text: text != "bad") as mock_exec: + with pytest.raises(SystemExit) as e: + cli.run_cli() + assert e.value.code == expected_code + assert [c[0][0] for c in mock_exec.call_args_list] == expected_runs + + +@dbtest +def test_failed_command_exits_1(executor, tmpdir): + cli = PGCli(pgexecute=executor, pgclirc_file=str(tmpdir.join("rcfile"))) + cli.commands = ["select nope"] + with pytest.raises(SystemExit) as e: + cli.run_cli() + assert e.value.code == 1 + + +@dbtest +def test_failed_script_exits_3(executor, tmpdir): + sql_file = tmpdir.join("script.sql") + sql_file.write("select 1;\nselect nope;\nselect 2;\n") + cli = PGCli(pgexecute=executor, pgclirc_file=str(tmpdir.join("rcfile"))) + cli.input_files = [str(sql_file)] + with pytest.raises(SystemExit) as e: + cli.run_cli() + assert e.value.code == 3 + + def test_missing_rc_dir(tmpdir): rcfile = str(tmpdir.join("subdir").join("rcfile"))