From 1c7f0d0b638d37936d9ad8f8b42bd1b3e992b5ed Mon Sep 17 00:00:00 2001 From: Heikki Linnakangas Date: Sun, 5 Jul 2026 00:43:27 +0300 Subject: [PATCH 1/1] Improvements to psql --single-step mode 1. Switch to using gets_interactive() for reading the user input gets_interactive() reads the whole line, not just the first two characters like the old fgets() call did. That gives nicer behavior if the user types a string longer than just a single character to the prompt. Previously, the extra characters were kept buffered for subsequent prompts. One consequence of that was that if you responded to the prompt with e.g. "xxxxx" while reading a script with multiple commands, the extra 'x' characters were used as inputs to the prompts on subsequent commands from the file. That was not really usable as a feature either, because it was not one character per prompt, but two, which was just confusing. Now we always swallow the whole line. Add an argument to gets_interactive() to disable tab-completion when we use it for the single-step verification prompt and plumb it through to the tab-completion callback. 2. If the user types anything other than 'x' on the prompt, ask again Previously, we treated any unrecognized input as a "go ahead" and executed the query, the same as if you just hit enter. That's a dangerous default; the point of the --single-step mode is to carefully verify each query before executing them, so we should *not* execute the query if it's not clear what the user intended. Now we keep repeating the instructions to hit return or 'x'+return and wait for another response, until the user types either of those. 3. React to Ctrl-C immediately Previously, if you hit Ctrl-C when stopped on the verification prompt, it had no immediate effect. It did prevent the query from running, but you still had to hit enter to exit the prompt. Change it to exit the prompt immediately; that's probably what the user would expect. 4. Don't execute the queries on Ctrl-D Previously, if the user hits Ctrl-D on the verification prompt, we proceeded to execute the command, and all subsequent commands too. We displayed the prompt on each command, but didn't wait for the user to type a response. To make things worse, if you hit Ctrl-D while reading commands from a file with "\i script.sql", and you then ran another "\i script2.sql" command, we continued to execute all the commands from the second file too without stopping. Switching to readline() fixed that too. Ctrl-D is now treated similarly to Ctrl-C, ie. the current script is cancelled but subsequent scripts are not affected. 5. If not a tty, don't execute anything in --single-step mode Previously, you could use --single-step mode when stdin was reading from a file, and the responses to the prompts were read from the same input, which was unlikely what you'd want. The intended use of the single-step mode is to interactively ask the user about each query before executing them, which doesn't make sense when reading from a file. (Reading a script with the -f option still works and makes a lot of sense. The concern is with things like "cat script.sql | psql --single-step".) --- src/bin/psql/common.c | 127 ++++++++++++++++++++++++++++--- src/bin/psql/input.c | 5 +- src/bin/psql/input.h | 4 +- src/bin/psql/mainloop.c | 4 +- src/bin/psql/tab-complete.h | 7 ++ src/bin/psql/tab-complete.in.c | 55 +++++++++---- src/tools/pgindent/typedefs.list | 1 + 7 files changed, 173 insertions(+), 30 deletions(-) diff --git a/src/bin/psql/common.c b/src/bin/psql/common.c index 7db8025d24c..35ace448a61 100644 --- a/src/bin/psql/common.c +++ b/src/bin/psql/common.c @@ -27,8 +27,10 @@ #include "fe_utils/cancel.h" #include "fe_utils/mbprint.h" #include "fe_utils/string_utils.h" +#include "input.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, @@ -1104,6 +1106,117 @@ 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) +{ + 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(sigint_interrupt_jmp, 1) != 0) + { + /* got here with longjmp from gets_interactive() */ + result = false; + cancel_pressed = true; + } + else + { + 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) + { + 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; + } + } + } + } + pg_free(first_prompt); + return result; +} + /* * SendQuery: send the query string to the backend * (and print out result) @@ -1135,18 +1248,7 @@ SendQuery(const char *query) 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) - if (buf[0] == 'x') - goto sendquery_cleanup; - if (cancel_pressed) + if (!singlestep_verify(query)) goto sendquery_cleanup; } else if (pset.echo == PSQL_ECHO_QUERIES) @@ -1294,6 +1396,7 @@ SendQuery(const char *query) /* 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 6d37359fc96..a38be481027 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 sigint_interrupt_jmp 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 1f409a573b7..93c30f0e95a 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 = { @@ -164,7 +165,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/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 e4bc2c93145..30b9b8477bb 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() @@ -1467,6 +1469,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, @@ -1859,6 +1862,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; @@ -2109,21 +2149,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 117e7379f10..fa49aec36fc 100644 --- a/src/tools/pgindent/typedefs.list +++ b/src/tools/pgindent/typedefs.list @@ -4352,6 +4352,7 @@ substitute_grouped_columns_context substitute_phv_relids_context subxids_array_status symbol +tabCompletionMode tablespaceinfo tar_file td_entry -- 2.47.3