Hi,
On 2/15/23 1:56 AM, Kyotaro Horiguchi wrote:
At Tue, 14 Feb 2023 15:43:26 +0100, "Drouvot, Bertrand"
<bertranddrouvot...@gmail.com> wrote in
The comment assumed that the function directly returns an entry from
shared memory, but now it copies the entry's contents into a palloc'ed
memory and stores the sums of some counters for the current
transaction in it. Do you think we should update the comment to
reflect this change?
Good point, thanks! Yeah, definitively, done in V5 attached.
Regards,
--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
diff --git a/src/backend/utils/activity/pgstat_relation.c
b/src/backend/utils/activity/pgstat_relation.c
index f793ac1516..b26e2a5a7a 100644
--- a/src/backend/utils/activity/pgstat_relation.c
+++ b/src/backend/utils/activity/pgstat_relation.c
@@ -471,20 +471,46 @@ pgstat_fetch_stat_tabentry_ext(bool shared, Oid reloid)
* Find any existing PgStat_TableStatus entry for rel_id in the current
* database. If not found, try finding from shared tables.
*
+ * If an entry is found, copy it and increment the copy's counters with their
+ * subtransactions counterparts. Then return the copy. There is no need for the
+ * caller to pfree the copy as the MemoryContext will be reset soon after.
+ *
* If no entry found, return NULL, don't create a new one
*/
PgStat_TableStatus *
find_tabstat_entry(Oid rel_id)
{
PgStat_EntryRef *entry_ref;
+ PgStat_TableXactStatus *trans;
+ PgStat_TableStatus *tabentry = NULL;
+ PgStat_TableStatus *tablestatus = NULL;
entry_ref = pgstat_fetch_pending_entry(PGSTAT_KIND_RELATION,
MyDatabaseId, rel_id);
if (!entry_ref)
+ {
entry_ref = pgstat_fetch_pending_entry(PGSTAT_KIND_RELATION,
InvalidOid, rel_id);
+ if(!entry_ref)
+ return tablestatus;
+ }
+
+ tabentry = (PgStat_TableStatus *) entry_ref->pending;
+ tablestatus = palloc(sizeof(PgStat_TableStatus));
+ *tablestatus = *tabentry;
+
+ /*
+ * Live subtransactions' counts aren't in t_counts yet. This is not a
hot
+ * code path so it sounds ok to reconcile for tuples_inserted,
+ * tuples_updated and tuples_deleted even if this is not what the caller
+ * is interested in.
+ */
+ for (trans = tabentry->trans; trans != NULL; trans = trans->upper)
+ {
+ tablestatus->t_counts.t_tuples_inserted +=
trans->tuples_inserted;
+ tablestatus->t_counts.t_tuples_updated += trans->tuples_updated;
+ tablestatus->t_counts.t_tuples_deleted += trans->tuples_deleted;
+ }
- if (entry_ref)
- return entry_ref->pending;
- return NULL;
+ return tablestatus;
}
/*
diff --git a/src/backend/utils/adt/pgstatfuncs.c
b/src/backend/utils/adt/pgstatfuncs.c
index 9d707c3521..405d080daa 100644
--- a/src/backend/utils/adt/pgstatfuncs.c
+++ b/src/backend/utils/adt/pgstatfuncs.c
@@ -1548,17 +1548,11 @@ pg_stat_get_xact_tuples_inserted(PG_FUNCTION_ARGS)
Oid relid = PG_GETARG_OID(0);
int64 result;
PgStat_TableStatus *tabentry;
- PgStat_TableXactStatus *trans;
if ((tabentry = find_tabstat_entry(relid)) == NULL)
result = 0;
else
- {
- result = tabentry->t_counts.t_tuples_inserted;
- /* live subtransactions' counts aren't in t_tuples_inserted yet
*/
- for (trans = tabentry->trans; trans != NULL; trans =
trans->upper)
- result += trans->tuples_inserted;
- }
+ result = (int64) (tabentry->t_counts.t_tuples_inserted);
PG_RETURN_INT64(result);
}
@@ -1569,17 +1563,11 @@ pg_stat_get_xact_tuples_updated(PG_FUNCTION_ARGS)
Oid relid = PG_GETARG_OID(0);
int64 result;
PgStat_TableStatus *tabentry;
- PgStat_TableXactStatus *trans;
if ((tabentry = find_tabstat_entry(relid)) == NULL)
result = 0;
else
- {
- result = tabentry->t_counts.t_tuples_updated;
- /* live subtransactions' counts aren't in t_tuples_updated yet
*/
- for (trans = tabentry->trans; trans != NULL; trans =
trans->upper)
- result += trans->tuples_updated;
- }
+ result = (int64) (tabentry->t_counts.t_tuples_updated);
PG_RETURN_INT64(result);
}
@@ -1590,17 +1578,11 @@ pg_stat_get_xact_tuples_deleted(PG_FUNCTION_ARGS)
Oid relid = PG_GETARG_OID(0);
int64 result;
PgStat_TableStatus *tabentry;
- PgStat_TableXactStatus *trans;
if ((tabentry = find_tabstat_entry(relid)) == NULL)
result = 0;
else
- {
- result = tabentry->t_counts.t_tuples_deleted;
- /* live subtransactions' counts aren't in t_tuples_deleted yet
*/
- for (trans = tabentry->trans; trans != NULL; trans =
trans->upper)
- result += trans->tuples_deleted;
- }
+ result = (int64) (tabentry->t_counts.t_tuples_deleted);
PG_RETURN_INT64(result);
}