From 6921a7b6e25c2471d64f05ce136af741598381c0 Mon Sep 17 00:00:00 2001 From: Michael Paquier Date: Fri, 13 Sep 2024 14:54:17 +0900 Subject: [PATCH v6 1/2] Add missing query ID reporting in extended query protocol This commit adds query ID reports for two code paths for the extended query protocol: - When receiving a bind message, setting it to the first Query retrieved from a cached cache. - When receiving an execute message, setting it to the first PlannedStmt stored in a portal. An advantage of this method is that this is able to cover all the types of portals handled in the extended query protocol, particularly these two when the report done in ExecutorStart() is not enough: - Multiple execute messages, with multiple ExecutorRun(). - Portal with execute/fetch messages, like a query with a RETURNING clause and a fetch size that stores the tuples in a first execute message going though ExecutorStart() and ExecuteRun(), followed by one or more execute messages doing only fetches from the tuplestore created in the first message. Note that the query ID reporting done in ExecutorStart() is still necessary, as an EXECUTE requires it. Query ID reporting is optimistic and more calls to pgstat_report_query_id() don't matter. The comment in ExecutorStart() is adjusted to reflect better the reality with the extended query protocol. The test added in pg_stat_statements is a courtesy of Robert Haas. Author: Sami Imseih Discussion: https://postgr.es/m/CA+427g8DiW3aZ6pOpVgkPbqK97ouBdf18VLiHFesea2jUk3XoQ@mail.gmail.com Discussion: https://postgr.es/m/CA+TgmoZxtnf_jZ=VqBSyaU8hfUkkwoJCJ6ufy4LGpXaunKrjrg@mail.gmail.com Backpatch-through: 14 --- src/backend/executor/execMain.c | 10 ++++---- src/backend/tcop/postgres.c | 24 +++++++++++++++++++ .../pg_stat_statements/expected/extended.out | 8 +++++++ contrib/pg_stat_statements/sql/extended.sql | 4 ++++ 4 files changed, 42 insertions(+), 4 deletions(-) diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c index 29e186fa73..7042ca6c60 100644 --- a/src/backend/executor/execMain.c +++ b/src/backend/executor/execMain.c @@ -119,10 +119,12 @@ void ExecutorStart(QueryDesc *queryDesc, int eflags) { /* - * In some cases (e.g. an EXECUTE statement) a query execution will skip - * parse analysis, which means that the query_id won't be reported. Note - * that it's harmless to report the query_id multiple times, as the call - * will be ignored if the top level query_id has already been reported. + * In some cases (e.g. an EXECUTE statement or an execute message with the + * extended query protocol) the query_id won't be reported, so do it now. + * + * Note that it's harmless to report the query_id multiple times, as the + * call will be ignored if the top level query_id has already been + * reported. */ pgstat_report_query_id(queryDesc->plannedstmt->queryId, false); diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c index 8bc6bea113..e394f1419a 100644 --- a/src/backend/tcop/postgres.c +++ b/src/backend/tcop/postgres.c @@ -1654,6 +1654,7 @@ exec_bind_message(StringInfo input_message) char msec_str[32]; ParamsErrorCbData params_data; ErrorContextCallback params_errcxt; + ListCell *lc; /* Get the fixed part of the message */ portal_name = pq_getmsgstring(input_message); @@ -1689,6 +1690,17 @@ exec_bind_message(StringInfo input_message) pgstat_report_activity(STATE_RUNNING, psrc->query_string); + foreach(lc, psrc->query_list) + { + Query *query = lfirst_node(Query, lc); + + if (query->queryId != UINT64CONST(0)) + { + pgstat_report_query_id(query->queryId, false); + break; + } + } + set_ps_display("BIND"); if (save_log_statement_stats) @@ -2111,6 +2123,7 @@ exec_execute_message(const char *portal_name, long max_rows) ErrorContextCallback params_errcxt; const char *cmdtagname; size_t cmdtaglen; + ListCell *lc; /* Adjust destination to tell printtup.c what to do */ dest = whereToSendOutput; @@ -2157,6 +2170,17 @@ exec_execute_message(const char *portal_name, long max_rows) pgstat_report_activity(STATE_RUNNING, sourceText); + foreach(lc, portal->stmts) + { + PlannedStmt *stmt = lfirst_node(PlannedStmt, lc); + + if (stmt->queryId != UINT64CONST(0)) + { + pgstat_report_query_id(stmt->queryId, false); + break; + } + } + cmdtagname = GetCommandTagNameAndLen(portal->commandTag, &cmdtaglen); set_ps_display_with_len(cmdtagname, cmdtaglen); diff --git a/contrib/pg_stat_statements/expected/extended.out b/contrib/pg_stat_statements/expected/extended.out index bc8cb3f141..04a0594337 100644 --- a/contrib/pg_stat_statements/expected/extended.out +++ b/contrib/pg_stat_statements/expected/extended.out @@ -1,5 +1,13 @@ -- Tests with extended query protocol SET pg_stat_statements.track_utility = FALSE; +-- This test checks that an execute message sets a query ID. +SELECT query_id IS NOT NULL AS query_id_set + FROM pg_stat_activity WHERE pid = pg_backend_pid() \bind \g + query_id_set +-------------- + t +(1 row) + SELECT pg_stat_statements_reset() IS NOT NULL AS t; t --- diff --git a/contrib/pg_stat_statements/sql/extended.sql b/contrib/pg_stat_statements/sql/extended.sql index 5ba0678e63..1af0711020 100644 --- a/contrib/pg_stat_statements/sql/extended.sql +++ b/contrib/pg_stat_statements/sql/extended.sql @@ -2,6 +2,10 @@ SET pg_stat_statements.track_utility = FALSE; +-- This test checks that an execute message sets a query ID. +SELECT query_id IS NOT NULL AS query_id_set + FROM pg_stat_activity WHERE pid = pg_backend_pid() \bind \g + SELECT pg_stat_statements_reset() IS NOT NULL AS t; SELECT $1 \parse stmt1 SELECT $1, $2 \parse stmt2 -- 2.45.2