Hi,
On 2/14/23 7:11 AM, Kyotaro Horiguchi wrote:
Isn't it ignoring the second call to pgstat_fetch_pending_entry?
Oh right, my bad (the issue has been introduced in V2).
Fixed in V4.
I thought that we might be able to return entry_ref->pending since the
callers don't call pfree on the returned pointer, but it is not great
that we don't inform the callers if the returned memory can be safely
pfreed or not.
Thus what I have in mind is the following.
if (!entry_ref)
+ {
entry_ref = pgstat_fetch_pending_entry(PGSTAT_KIND_RELATION,
InvalidOid, rel_id);
+ if (!entry_ref)
+ return NULL;
+ }
LGTM, done that way in V4.
So, since we want to hide the internal from pgstatfuncs, the
additional flag should be gone.
I think there is pros and cons for both but I don't have a strong
opinion about that.
So also proposing V3 attached without the flag in case this is the
preferred approach.
That part looks good to me.
Thanks!
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..91b0eee7a4 100644
--- a/src/backend/utils/activity/pgstat_relation.c
+++ b/src/backend/utils/activity/pgstat_relation.c
@@ -477,14 +477,36 @@ 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);
}
