postgres.git / summary / log / commit / refs

commit    b36ee879c54cf42f21c9374aceb5baf65efecbc3
Author:   Michael Paquier <michael@paquier.xyz>
Date:     Wed Sep 18 00:59:26 2024 +0000

    Add missing query ID reporting in extended query protocol
    
    This commit adds query ID reports for two code paths when processing
    extended query protocol messages:
    - 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 (neither is an
    addition in ExecutorRun(), actually, for the second point):
    - 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.  This corresponds to the case where
    execute_is_fetch is set, for example.
    
    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 as the first
    report takes priority except if the report is forced.  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.  This
    uses psql's \bind metacommand, hence this part is backpatched down to
    v16.
    
    Reported-by:  Kaido Vaikla, Erik Wienhold
    Author: Sami Imseih
    Reviewed-by: Jian He, Andrei Lepikhov, Michael Paquier
    Discussion: https://postgr.es/m/CA+427g8DiW3aZ6pOpVgkPbqK97ouBdf18VLiHFesea2jUk3XoQ@mail.gmail.com
    Discussion: https://postgr.es/m/CA+TgmoZxtnf_jZ=VqBSyaU8hfUkkwoJCJ6ufy4LGpXaunKrjrg@mail.gmail.com
    Discussion: https://postgr.es/m/1391613709.939460.1684777418070@office.mailbox.org
    Backpatch-through: 14


 src/backend/executor/execMain.c | 10 ++++++----
 src/backend/tcop/postgres.c     | 24 ++++++++++++++++++++++++
 2 files changed, 30 insertions(+), 4 deletions(-)

diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c
index 42fe8f8fb65..17a5ce61736 100644
--- a/src/backend/executor/execMain.c
+++ b/src/backend/executor/execMain.c
@@ -130,10 +130,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 time, 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 75a36f1b8b1..bd8ad11421a 100644
--- a/src/backend/tcop/postgres.c
+++ b/src/backend/tcop/postgres.c
@@ -1609,6 +1609,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);
@@ -1644,6 +1645,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)
@@ -2067,6 +2079,7 @@ exec_execute_message(const char *portal_name, long max_rows)
 	char		msec_str[32];
 	ParamsErrorCbData params_data;
 	ErrorContextCallback params_errcxt;
+	ListCell   *lc;
 
 	/* Adjust destination to tell printtup.c what to do */
 	dest = whereToSendOutput;
@@ -2113,6 +2126,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;
+		}
+	}
+
 	set_ps_display(GetCommandTagName(portal->commandTag));
 
 	if (save_log_statement_stats)


[parent: 46d277307fbe]