From e77de23fbb0f4ef27090c144edcfa889bb2a06d5 Mon Sep 17 00:00:00 2001 From: Peter Eisentraut Date: Thu, 9 Jun 2022 08:49:13 +0200 Subject: [PATCH] psql: Show notices immediately (again) The new show-all-results feature in psql (7844c9918) went out of its way to show notices next to the results of the statements (in a multi-statement string) that caused them. This also had the consequence that notices for a single statement were not shown until after the statement had executed, instead of right away. After some discussion, it seems very difficult to satisfy both of these goals, so here we are giving up on the first goal and just show the notices as we get them. This restores the pre-7844c9918 behavior for notices. Reported-by: Alastair McKinley Author: Fabien COELHO Discussion: https://www.postgresql.org/message-id/flat/PAXPR02MB760039506C87A2083AD85575E3DA9%40PAXPR02MB7600.eurprd02.prod.outlook.com --- src/bin/psql/common.c | 64 ------------------------------ src/test/regress/expected/psql.out | 8 ++-- 2 files changed, 4 insertions(+), 68 deletions(-) diff --git a/src/bin/psql/common.c b/src/bin/psql/common.c index 03e6d9ce8f..974959c595 100644 --- a/src/bin/psql/common.c +++ b/src/bin/psql/common.c @@ -1061,44 +1061,6 @@ PrintQueryResult(PGresult *result, bool last, bool is_watch, const printQueryOpt return success; } -/* - * Data structure and functions to record notices while they are - * emitted, so that they can be shown later. - * - * We need to know which result is last, which requires to extract - * one result in advance, hence two buffers are needed. - */ -struct t_notice_messages -{ - PQExpBufferData messages[2]; - int current; -}; - -/* - * Store notices in appropriate buffer, for later display. - */ -static void -AppendNoticeMessage(void *arg, const char *msg) -{ - struct t_notice_messages *notices = arg; - - appendPQExpBufferStr(¬ices->messages[notices->current], msg); -} - -/* - * Show notices stored in buffer, which is then reset. - */ -static void -ShowNoticeMessage(struct t_notice_messages *notices) -{ - PQExpBufferData *current = ¬ices->messages[notices->current]; - - if (*current->data != '\0') - pg_log_info("%s", current->data); - resetPQExpBuffer(current); -} - - /* * SendQuery: send the query string to the backend * (and print out result) @@ -1483,7 +1445,6 @@ ExecQueryAndProcessResults(const char *query, double *elapsed_msec, bool *svpt_g instr_time before, after; PGresult *result; - struct t_notice_messages notices; if (timing) INSTR_TIME_SET_CURRENT(before); @@ -1513,12 +1474,6 @@ ExecQueryAndProcessResults(const char *query, double *elapsed_msec, bool *svpt_g return 0; } - /* intercept notices */ - notices.current = 0; - initPQExpBuffer(¬ices.messages[0]); - initPQExpBuffer(¬ices.messages[1]); - PQsetNoticeProcessor(pset.db, AppendNoticeMessage, ¬ices); - /* first result */ result = PQgetResult(pset.db); @@ -1536,7 +1491,6 @@ ExecQueryAndProcessResults(const char *query, double *elapsed_msec, bool *svpt_g */ const char *error = PQresultErrorMessage(result); - ShowNoticeMessage(¬ices); if (strlen(error)) pg_log_info("%s", error); @@ -1601,8 +1555,6 @@ ExecQueryAndProcessResults(const char *query, double *elapsed_msec, bool *svpt_g if (result_status == PGRES_COPY_IN || result_status == PGRES_COPY_OUT) { - ShowNoticeMessage(¬ices); - if (is_watch) { ClearOrSaveAllResults(); @@ -1610,12 +1562,7 @@ ExecQueryAndProcessResults(const char *query, double *elapsed_msec, bool *svpt_g return -1; } - /* use normal notice processor during COPY */ - PQsetNoticeProcessor(pset.db, NoticeProcessor, NULL); - success &= HandleCopyResult(&result); - - PQsetNoticeProcessor(pset.db, AppendNoticeMessage, ¬ices); } /* @@ -1623,9 +1570,7 @@ ExecQueryAndProcessResults(const char *query, double *elapsed_msec, bool *svpt_g * string, it will return NULL. Otherwise, we'll have other results * to process. We need to do that to check whether this is the last. */ - notices.current ^= 1; next_result = PQgetResult(pset.db); - notices.current ^= 1; last = (next_result == NULL); /* @@ -1647,9 +1592,6 @@ ExecQueryAndProcessResults(const char *query, double *elapsed_msec, bool *svpt_g *elapsed_msec = INSTR_TIME_GET_MILLISEC(after); } - /* notices already shown above for copy */ - ShowNoticeMessage(¬ices); - /* this may or may not print something depending on settings */ if (result != NULL) success &= PrintQueryResult(result, last, false, opt, printQueryFout); @@ -1659,7 +1601,6 @@ ExecQueryAndProcessResults(const char *query, double *elapsed_msec, bool *svpt_g SetResultVariables(result, true); ClearOrSaveResult(result); - notices.current ^= 1; result = next_result; if (cancel_pressed) @@ -1669,11 +1610,6 @@ ExecQueryAndProcessResults(const char *query, double *elapsed_msec, bool *svpt_g } } - /* reset notice hook */ - PQsetNoticeProcessor(pset.db, NoticeProcessor, NULL); - termPQExpBuffer(¬ices.messages[0]); - termPQExpBuffer(¬ices.messages[1]); - /* may need this to recover from conn loss during COPY */ if (!CheckConnection()) return -1; diff --git a/src/test/regress/expected/psql.out b/src/test/regress/expected/psql.out index 2a38a93a3b..60acbd1241 100644 --- a/src/test/regress/expected/psql.out +++ b/src/test/regress/expected/psql.out @@ -5316,13 +5316,13 @@ AS $$ $$; -- show both SELECT 1 AS one \; SELECT warn('1.5') \; SELECT 2 AS two ; +NOTICE: warn 1.5 +CONTEXT: PL/pgSQL function warn(text) line 2 at RAISE one ----- 1 (1 row) -NOTICE: warn 1.5 -CONTEXT: PL/pgSQL function warn(text) line 2 at RAISE warn ------ t @@ -5335,13 +5335,13 @@ CONTEXT: PL/pgSQL function warn(text) line 2 at RAISE -- \gset applies to last query only SELECT 3 AS three \; SELECT warn('3.5') \; SELECT 4 AS four \gset +NOTICE: warn 3.5 +CONTEXT: PL/pgSQL function warn(text) line 2 at RAISE three ------- 3 (1 row) -NOTICE: warn 3.5 -CONTEXT: PL/pgSQL function warn(text) line 2 at RAISE warn ------ t