Skip to content

PG-2395 Make it clear we do not do any catcache looksups in ExecutorEnd #811

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
jeltz wants to merge 2 commits into
base: main
Choose a base branch
from
Draft

PG-2395 Make it clear we do not do any catcache looksups in ExecutorEnd #811

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
82c7e77
PG-2395 Remove code for colecting query stats in ExectuorEnd
jeltz Aug 3, 2026
85ce181
PG-2395 Add changelog item about how hopefully an RDS bug is fixed
jeltz Aug 3, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,3 +33,4 @@
- Do not access `PlannedStmt` after we call `standard_ProcessUtility()` ([PG-2486](https://perconadev.atlassian.net/browse/PG-2486))
- Various improvements to the stability of our test suite
- Make sure that for prepared statements utility statement exec info read at the executor start hook, where data is not yet modified by query itself
- Attempt to fix use after free when running on RDS ([PG-2395](https://perconadev.atlassian.net/browse/PG-2395))
44 changes: 28 additions & 16 deletions src/pg_stat_monitor.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -234,7 +234,8 @@ static void pgsm_fill_query_exec_info(pgsmQueryExecInfo *info);
static pgsmQueryStats *pgsm_add_query_stats(int64 queryid, int64 planid, int64 pgsm_query_id, const char *query_text, int query_len, CmdType cmd_type);
static void pgsm_fill_query_stats(pgsmQueryStats *stats, const pgsmQueryExecInfo *info, int64 queryid, int64 planid, int64 pgsm_query_id, const char *query_text, CmdType cmd_type);
static void pgsm_delete_query_stats(uint64 queryid);
static pgsmQueryStats *pgsm_get_query_stats(int64 queryid, int64 planid, const char *query_text, CmdType cmd_type);
static pgsmQueryStats *pgsm_get_query_stats(int64 queryid);
static pgsmQueryStats *pgsm_get_or_add_query_stats(int64 queryid, int64 planid, const char *query_text, CmdType cmd_type);
static int64 get_pgsm_query_id_hash(const char *norm_query, int len);

static void pgsm_cleanup_callback(void *arg);
Expand Down Expand Up @@ -550,8 +551,8 @@ pgsm_ExecutorStart(QueryDesc *queryDesc, int eflags)
* snapshot of the execution info (application_name, user) reflects
* the state at statement start.
*/
(void) pgsm_get_query_stats(queryDesc->plannedstmt->queryId, 0,
queryDesc->sourceText, queryDesc->operation);
(void) pgsm_get_or_add_query_stats(queryDesc->plannedstmt->queryId, 0,
queryDesc->sourceText, queryDesc->operation);

/*
* Set up to track total elapsed time in ExecutorRun. Make sure the
Expand Down Expand Up @@ -700,7 +701,9 @@ pgsm_ExecutorEnd(QueryDesc *queryDesc)
SysInfo sys_info;
int64 planid = plan_ptr ? plan_ptr->planid : 0;

stats = pgsm_get_query_stats(queryId, planid, queryDesc->sourceText, queryDesc->operation);
/* We know at least pgsm_ExecutorStart() has created query stats */
stats = pgsm_get_query_stats(queryId);
Assert(stats != NULL);

@jeltz jeltz Aug 4, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: this is likely wrong. We should probably move to a stack instead.


if (stats->key.planid == 0 && planid != 0)
stats->key.planid = planid;
Expand Down Expand Up @@ -865,7 +868,7 @@ pgsm_planner_hook(Query *parse, const char *query_string, int cursorOptions, Par
walusage_start = pgWalUsage;
INSTR_TIME_SET_CURRENT(start);

stats = pgsm_get_query_stats(queryId, 0, query_string, parse->commandType);
stats = pgsm_get_or_add_query_stats(queryId, 0, query_string, parse->commandType);

#if PG_VERSION_NUM >= 170000
nesting_level++;
Expand Down Expand Up @@ -1628,23 +1631,37 @@ pgsm_delete_query_stats(uint64 queryid)
}
}

/*
* Function to get a pgsmQueryStats structure from the local list.
*/
static pgsmQueryStats *
pgsm_get_query_stats(int64 queryid, int64 planid, const char *query_text, CmdType cmd_type)
pgsm_get_or_add_query_stats(int64 queryid, int64 planid, const char *query_text, CmdType cmd_type)
{
pgsmQueryStats *stats;
int query_len;

Assert(query_text != NULL);

stats = pgsm_get_query_stats(queryid);
if (stats != NULL)
return stats;

query_len = strlen(query_text);
return pgsm_add_query_stats(queryid, planid,
get_pgsm_query_id_hash(query_text, query_len),
query_text, query_len, cmd_type);
}

/*
* Function to get a pgsmQueryStats structure from the local list.
*/
static pgsmQueryStats *
pgsm_get_query_stats(int64 queryid)
{
if (lentries != NIL)
{
pgsmQueryStats *stats;
ListCell *lc;

/* First bet is on the last item */
stats = (pgsmQueryStats *) llast(lentries);
stats = llast(lentries);
if (stats->key.queryid == queryid)
return stats;

Expand All @@ -1656,12 +1673,7 @@ pgsm_get_query_stats(int64 queryid, int64 planid, const char *query_text, CmdTyp
}
}

query_len = strlen(query_text);
stats = pgsm_add_query_stats(queryid, planid,
get_pgsm_query_id_hash(query_text, query_len),
query_text, query_len, cmd_type);

return stats;
return NULL;
}

static void
Expand Down
Loading