Commit 0f2f5645 authored by Tom Lane's avatar Tom Lane

pg_stat_statements: fetch stmt location/length before it disappears.

When executing a utility statement, we must fetch everything
we need out of the PlannedStmt data structure before calling
standard_ProcessUtility.  In certain cases (possibly only ROLLBACK
in extended query protocol), that data structure will get freed
during command execution.  The situation is probably often harmless
in production builds, but in debug builds we intentionally overwrite
the freed memory with garbage, leading to picking up garbage values
of statement location and length, typically causing an assertion
failure later in pg_stat_statements.  In non-debug builds, if
something did go wrong it would likely lead to storing garbage
for the query string.

Report and fix by zhaoqigui (with cosmetic adjustments by me).
It's an old problem, so back-patch to all supported versions.

Discussion: https://postgr.es/m/17663-a344fd0675f92128@postgresql.org
Discussion: https://postgr.es/m/1667307420050.56657@hundsun.com
parent 5a30d43f
...@@ -1077,6 +1077,8 @@ pgss_ProcessUtility(PlannedStmt *pstmt, const char *queryString, ...@@ -1077,6 +1077,8 @@ pgss_ProcessUtility(PlannedStmt *pstmt, const char *queryString,
{ {
Node *parsetree = pstmt->utilityStmt; Node *parsetree = pstmt->utilityStmt;
uint64 saved_queryId = pstmt->queryId; uint64 saved_queryId = pstmt->queryId;
int saved_stmt_location = pstmt->stmt_location;
int saved_stmt_len = pstmt->stmt_len;
/* /*
* Force utility statements to get queryId zero. We do this even in cases * Force utility statements to get queryId zero. We do this even in cases
...@@ -1142,6 +1144,16 @@ pgss_ProcessUtility(PlannedStmt *pstmt, const char *queryString, ...@@ -1142,6 +1144,16 @@ pgss_ProcessUtility(PlannedStmt *pstmt, const char *queryString,
} }
PG_END_TRY(); PG_END_TRY();
/*
* CAUTION: do not access the *pstmt data structure again below here.
* If it was a ROLLBACK or similar, that data structure may have been
* freed. We must copy everything we still need into local variables,
* which we did above.
*
* For the same reason, we can't risk restoring pstmt->queryId to its
* former value, which'd otherwise be a good idea.
*/
INSTR_TIME_SET_CURRENT(duration); INSTR_TIME_SET_CURRENT(duration);
INSTR_TIME_SUBTRACT(duration, start); INSTR_TIME_SUBTRACT(duration, start);
...@@ -1166,8 +1178,8 @@ pgss_ProcessUtility(PlannedStmt *pstmt, const char *queryString, ...@@ -1166,8 +1178,8 @@ pgss_ProcessUtility(PlannedStmt *pstmt, const char *queryString,
pgss_store(queryString, pgss_store(queryString,
saved_queryId, saved_queryId,
pstmt->stmt_location, saved_stmt_location,
pstmt->stmt_len, saved_stmt_len,
PGSS_EXEC, PGSS_EXEC,
INSTR_TIME_GET_MILLISEC(duration), INSTR_TIME_GET_MILLISEC(duration),
rows, rows,
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment