From 9e3e1e0ba8ed8b35c570c2d6ac145e6aa8044f61 Mon Sep 17 00:00:00 2001 From: Heikki Linnakangas Date: Sun, 5 Jul 2026 00:43:27 +0300 Subject: [PATCH 3/3] Improvements to psql --single-step mode Use gets_interactive to read the whole response to a single-step confirmation, instead of reading at most two characters with fgets. Previously, a longer response left characters buffered for subsequent confirmations. Disable tab completion while asking for confirmation. Accept an empty line to execute the query and x to skip it. Ask again for other responses, rather than treating unexpected input as permission to execute. Refuse single-step execution when stdin is not a terminal. Let Ctrl-C cancel confirmation immediately, without requiring the user to press Enter afterward. Use a local sigjmp_buf in singlestep_verify and save and restore PG_exception_stack. This preserves MainLoop's saved context for later input instead of leaving siglongjmp to use the context of singlestep_verify after it has returned. Retain the preceding EOF fix when moving confirmation from SendQuery to singlestep_verify. On EOF, clear stdin's EOF indicator and cancel the script, so a session without readline can still accept another command. Add TAP tests for Ctrl-C while confirming a query and during a later \prompt on the same script line. Reuse the preceding EOF tests to check cancellation, subsequent confirmation, and both readline modes. --- src/bin/psql/common.c | 148 +++++++++++++++++++++++++++---- src/bin/psql/input.c | 5 +- src/bin/psql/input.h | 4 +- src/bin/psql/mainloop.c | 4 +- src/bin/psql/meson.build | 1 + src/bin/psql/t/042_singlestep.pl | 75 ++++++++++++++++ src/bin/psql/tab-complete.h | 7 ++ src/bin/psql/tab-complete.in.c | 55 ++++++++---- src/tools/pgindent/typedefs.list | 1 + 9 files changed, 264 insertions(+), 36 deletions(-) create mode 100644 src/bin/psql/t/042_singlestep.pl diff --git a/src/bin/psql/common.c b/src/bin/psql/common.c index 9600c468aa2..db16a3b7c15 100644 --- a/src/bin/psql/common.c +++ b/src/bin/psql/common.c @@ -27,9 +27,11 @@ #include "fe_utils/cancel.h" #include "fe_utils/mbprint.h" #include "fe_utils/string_utils.h" +#include "input.h" #include "mainloop.h" #include "portability/instr_time.h" #include "settings.h" +#include "tab-complete.h" static bool DescribeQuery(const char *query, double *elapsed_msec); static int ExecQueryAndProcessResults(const char *query, @@ -1116,6 +1118,132 @@ PrintQueryResult(PGresult *result, bool last, return success; } +/* + * Ask the user if the query should be executed, for --single-step mode. + * + * Returns true if it should be executed, false otherwise. + */ +static bool +singlestep_verify(const char *query) +{ + sigjmp_buf local_sigjmp_buf; + sigjmp_buf *save_exception_stack = PG_exception_stack; + const char *header; + const char *instructions; + char *first_prompt; + const char *prompt; + bool result; + + /* + * --single-step mode only makes sense when there's a human at the + * terminal approving the queries. Perhaps it could also be used by a + * program connected to psql's stdin and stdout, while reading the SQL + * from a file, but it seems far-fetched. The point of --single-step mode + * is to be extra careful before executing commands, so let's play it safe + * and refuse to execute anything if the input is not an interactive + * terminal, to prevent accidentally running commands that the user didn't + * really mean to. + */ + if (pset.notty) + { + pg_log_error("single-step mode is active but not a tty, skipping command: %s", query); + return false; + } + + /* + * Construct the prompt. On first call, print the query we're about to + * execute and instructions. If the user types something invalid, we'll + * ask again with just the instructions. + */ + header = _("/**(Single step mode: verify command)******************************************/"); + instructions = _("/**(press return to proceed or enter x and return to cancel)*******************/\n"); + first_prompt = psprintf("%s\n%s\n%s", + header, query, instructions); + prompt = first_prompt; + + /* + * Establish longjmp destination for exiting from wait-for-input. (This + * is only effective while sigint_interrupt_enabled is TRUE.) + */ + if (sigsetjmp(local_sigjmp_buf, 1) != 0) + { + /* got here with longjmp from gets_interactive() */ + result = false; + cancel_pressed = true; + } + else + { + PG_exception_stack = &local_sigjmp_buf; + + for (;;) + { + char *line; + + if (cancel_pressed) + { + result = false; + break; + } + + line = gets_interactive(prompt, NULL, COMPLETE_DISABLE); + + /* + * gets_interactive returns NULL if the user hits Ctrl-D. Treat + * it the same as Ctrl-C. (NULL means "end-of-file", but in + * interactive mode we're not really reading from a file.) + */ + if (line == NULL) + { + /* + * Without readline, fgets leaves stdin's EOF indicator set. + * Clear it so MainLoop can read the next command from stdin. + */ + clearerr(stdin); + result = false; + cancel_pressed = true; + break; + } + else + { + /* Return on an empty line executes the query */ + if (strcmp(line, "") == 0) + { + pg_free(line); + result = true; + break; + } + + /* 'x' + return skips the query */ + else if (strcmp(line, "x") == 0) + { + pg_free(line); + result = false; + pg_log_info("skipping"); + break; + } + + /* + * Any other response is invalid. Ask again, but prompt with + * just the instruction without the query. + */ + else + { + pg_free(line); + prompt = instructions; + continue; + } + } + } + } + + sigint_interrupt_enabled = false; + PG_exception_stack = save_exception_stack; + + pg_free(first_prompt); + + return result; +} + /* * SendQuery: send the query string to the backend * (and print out result) @@ -1151,24 +1279,7 @@ SendQuery(const char *query, int num_copy_from_stdin) if (pset.singlestep) { - char buf[3]; - - fflush(stderr); - printf(_("/**(Single step mode: verify command)******************************************/\n" - "%s\n" - "/**(press return to proceed or enter x and return to cancel)*******************/\n"), - query); - fflush(stdout); - if (fgets(buf, sizeof(buf), stdin) == NULL) - { - /* Cancel the script, but allow subsequent input from stdin. */ - clearerr(stdin); - cancel_pressed = true; - goto sendquery_cleanup; - } - if (buf[0] == 'x') - goto sendquery_cleanup; - if (cancel_pressed) + if (!singlestep_verify(query)) goto sendquery_cleanup; } else if (pset.echo == PSQL_ECHO_QUERIES) @@ -1320,6 +1431,7 @@ SendQuery(const char *query, int num_copy_from_stdin) /* perform cleanup that should occur after any attempted query */ sendquery_cleanup: + sigint_interrupt_enabled = false; /* global cancellation reset */ ResetCancelConn(); diff --git a/src/bin/psql/input.c b/src/bin/psql/input.c index 2df1133b101..0f0635b9669 100644 --- a/src/bin/psql/input.c +++ b/src/bin/psql/input.c @@ -58,13 +58,14 @@ static void finishInput(void); * prompt: the prompt string to be used * query_buf: buffer containing lines already read in the current command * (query_buf is not modified here, but may be consulted for tab completion) + * completion_mode: context for tab-completion * * The result is a malloc'd string. * * Caller *must* have set up PG_exception_stack before calling. */ char * -gets_interactive(const char *prompt, PQExpBuffer query_buf) +gets_interactive(const char *prompt, PQExpBuffer query_buf, tabCompletionMode completion_mode) { #ifdef USE_READLINE if (useReadline) @@ -82,6 +83,8 @@ gets_interactive(const char *prompt, PQExpBuffer query_buf) rl_reset_screen_size(); #endif + tab_completion_mode = completion_mode; + /* Make current query_buf available to tab completion callback */ tab_completion_query_buf = query_buf; diff --git a/src/bin/psql/input.h b/src/bin/psql/input.h index 2a47166347e..fb92289ebee 100644 --- a/src/bin/psql/input.h +++ b/src/bin/psql/input.h @@ -46,8 +46,10 @@ #include "pqexpbuffer.h" +enum tabCompletionMode; -extern char *gets_interactive(const char *prompt, PQExpBuffer query_buf); +extern char *gets_interactive(const char *prompt, PQExpBuffer query_buf, + enum tabCompletionMode completion_mode); extern char *gets_fromFile(FILE *source); extern void initializeInput(int flags); diff --git a/src/bin/psql/mainloop.c b/src/bin/psql/mainloop.c index db02bdde178..014e5e1ee77 100644 --- a/src/bin/psql/mainloop.c +++ b/src/bin/psql/mainloop.c @@ -15,6 +15,7 @@ #include "mb/pg_wchar.h" #include "prompt.h" #include "settings.h" +#include "tab-complete.h" /* callback functions for our flex lexer */ const PsqlScanCallbacks psqlscan_callbacks = { @@ -166,7 +167,8 @@ MainLoop(FILE *source) } /* Now we can fetch a line */ line = gets_interactive(get_prompt(prompt_status, cond_stack), - query_buf); + query_buf, + COMPLETE_MAINLOOP); } else { diff --git a/src/bin/psql/meson.build b/src/bin/psql/meson.build index e78d53faeb6..6d5aa2d9e6e 100644 --- a/src/bin/psql/meson.build +++ b/src/bin/psql/meson.build @@ -80,6 +80,7 @@ tests += { 't/030_pager.pl', 't/040_interrupt.pl', 't/041_singlestep_eof.pl', + 't/042_singlestep.pl', ], }, } diff --git a/src/bin/psql/t/042_singlestep.pl b/src/bin/psql/t/042_singlestep.pl new file mode 100644 index 00000000000..26065327daa --- /dev/null +++ b/src/bin/psql/t/042_singlestep.pl @@ -0,0 +1,75 @@ +# Copyright (c) 2026, PostgreSQL Global Development Group + +use strict; +use warnings FATAL => 'all'; + +use PostgreSQL::Test::Cluster; +use PostgreSQL::Test::Utils; +use Test::More; + +if ($windows_os) +{ + plan skip_all => 'sending SIGINT on Windows terminates the test itself'; +} + +eval { require IO::Pty; }; +if ($@) +{ + plan skip_all => 'IO::Pty is needed to run this test'; +} + +my $node = PostgreSQL::Test::Cluster->new('main'); +$node->init; +$node->start; + +my $script = $node->basedir . '/prompt.sql'; +append_to_file($script, "SELECT 111; \\prompt 'ANSWER: ' answer\n"); + +my $confirm = qr/return to cancel[^\r\n]*\r?\n/; +my $ready = qr/postgres=# /; + +for my $no_readline (1, 0) +{ + # Respect the same readline opt-out as 010_tab_completion.pl. + next + if !$no_readline + && ((!defined($ENV{with_readline}) || $ENV{with_readline} ne 'yes') + || defined($ENV{SKIP_READLINE_TESTS})); + + my $mode = $no_readline ? 'without readline' : 'with readline'; + my $h = $node->interactive_psql('postgres', + extra_params => $no_readline ? ['-n'] : []); + $h->set_query_timer_restart; + $h->query_until(qr/step-ready\r?\npostgres=# /, + "\\set SINGLESTEP on\n\\echo step-ready\n"); + + # After singlestep_verify returns, Ctrl-C during \prompt on the same + # line must use the context saved by sigsetjmp in MainLoop. + for my $response ("\n", "x\n") + { + my $action = $response eq "\n" ? 'accepting' : 'skipping'; + $h->query_until($confirm, "\\i $script\n"); + $h->query_until(qr/ANSWER: /, $response); + $h->{run}->signal('INT'); + $h->query_until($ready, ''); + + $h->query_until($confirm, "SELECT 222;\n"); + like($h->query_until($ready, "\n"), + qr/\r?\n222\r?\n/, + "session survives canceling \\prompt after $action, $mode"); + } + + # Canceling confirmation must leave the session usable for another query. + $h->query_until($confirm, "SELECT 333;\n"); + $h->{run}->signal('INT'); + $h->query_until($ready, ''); + $h->query_until($confirm, "SELECT 444;\n"); + like($h->query_until($ready, "\n"), + qr/\r?\n444\r?\n/, + "session survives canceling single-step confirmation, $mode"); + + ok($h->quit, "psql exits normally, $mode"); +} + +$node->stop; +done_testing(); diff --git a/src/bin/psql/tab-complete.h b/src/bin/psql/tab-complete.h index fda737bfcaa..e6f38a1f69d 100644 --- a/src/bin/psql/tab-complete.h +++ b/src/bin/psql/tab-complete.h @@ -10,6 +10,13 @@ #include "pqexpbuffer.h" +typedef enum tabCompletionMode +{ + COMPLETE_DISABLE = 0, + COMPLETE_MAINLOOP, +} tabCompletionMode; + +extern tabCompletionMode tab_completion_mode; extern PQExpBuffer tab_completion_query_buf; extern void initialize_readline(void); diff --git a/src/bin/psql/tab-complete.in.c b/src/bin/psql/tab-complete.in.c index f86684edc08..7221d6a3ca9 100644 --- a/src/bin/psql/tab-complete.in.c +++ b/src/bin/psql/tab-complete.in.c @@ -85,6 +85,8 @@ /* word break characters */ #define WORD_BREAKS "\t\n@><=;|&() " +tabCompletionMode tab_completion_mode; + /* * Since readline doesn't let us pass any state through to the tab completion * callback, we have to use this global variable to let get_previous_words() @@ -1459,6 +1461,7 @@ static const char *const view_optional_parameters[] = { /* Forward declaration of functions */ static char **psql_completion(const char *text, int start, int end); +static char **mainloop_completion(const char *text, int start, int end); static char **match_previous_words(int pattern_id, const char *text, int start, int end, char **previous_words, @@ -1851,6 +1854,43 @@ psql_completion(const char *text, int start, int end) /* This is the variable we'll return. */ char **matches = NULL; + /* + * Dispatch to the correct completion function depending on the context + * we're being called in. + */ + switch (tab_completion_mode) + { + case COMPLETE_MAINLOOP: + matches = mainloop_completion(text, start, end); + break; + case COMPLETE_DISABLE: + break; + } + + /* + * If we still don't have anything to match we have to fabricate some sort + * of default list. If we were to just return NULL, readline automatically + * attempts filename completion, and that's usually no good. + */ + if (matches == NULL) + { + COMPLETE_WITH_CONST(true, ""); + /* Also, prevent Readline from appending stuff to the non-match */ + rl_completion_append_character = '\0'; +#ifdef HAVE_RL_COMPLETION_SUPPRESS_QUOTE + rl_completion_suppress_quote = 1; +#endif + } + return matches; +} + +/* The main completion function, for when we are in the main prompt */ +static char ** +mainloop_completion(const char *text, int start, int end) +{ + /* This is the variable we'll return. */ + char **matches = NULL; + /* Workspace for parsed words. */ char *words_buffer; @@ -2101,21 +2141,6 @@ psql_completion(const char *text, int start, int end) } } - /* - * If we still don't have anything to match we have to fabricate some sort - * of default list. If we were to just return NULL, readline automatically - * attempts filename completion, and that's usually no good. - */ - if (matches == NULL) - { - COMPLETE_WITH_CONST(true, ""); - /* Also, prevent Readline from appending stuff to the non-match */ - rl_completion_append_character = '\0'; -#ifdef HAVE_RL_COMPLETION_SUPPRESS_QUOTE - rl_completion_suppress_quote = 1; -#endif - } - /* free storage */ pg_free(previous_words); pg_free(words_buffer); diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list index a7a8dbdd23f..0c8a85d4ab1 100644 --- a/src/tools/pgindent/typedefs.list +++ b/src/tools/pgindent/typedefs.list @@ -4338,6 +4338,7 @@ substitute_grouped_columns_context substitute_phv_relids_context subxids_array_status symbol +tabCompletionMode tablespaceinfo tar_file td_entry -- 2.43.7