Re: pg_stat_transaction patch

Itagaki Takahiro <itagaki.takahiro@gmail.com> writes:

> "Accessor functions to get so far collected statistics for the current
> transaction"
> https://commitfest.postgresql.org/action/patch_view?id=301

> The only issue in the patch is too long view and function names:
>   - pg_stat_transaction_user_tables (31 chars)
>   - pg_stat_get_transaction_tuples_hot_updated (42 chars)
>   - pg_stat_get_transaction_function_self_time (42 chars)

> Since we've already used _xact_ in some system objects, we could replace
> _transaction_ parts with _xact_. It will save 7 key types per query ;-)

Applied, with assorted corrections -

* Renamed *_transaction_* to *_xact_* as suggested by Itagaki-san.

* Removed functions and view columns for delta live/dead tuple counts.

* Marked functions as volatile ... they certainly aren't stable.

* Got rid of use of get_tabstat_entry() to fetch table entries.  That
function forcibly creates tabstat entries if they weren't there before,
which was absolutely not what we want here: it'd result in bloating the
tabstat arrays with entries for tables the current transaction actually
never touched.  Worse, since you weren't passing the correct isshared
flag for the particular relation, the entries could be created with the
wrong isshared setting, leading to misbehavior if they did get used later
in the transaction.  We have to use a find-don't-create function here.

* Fixed bogus handling of inserted/updated/deleted counts --- you need to
add on the pending counts for all open levels of subtransaction.

* Assorted docs improvement and other minor polishing.

BTW, I notice that the patch provides pg_stat_get_xact_blocks_fetched()
and pg_stat_get_xact_blocks_hit(), but doesn't build any views on top of
them.  Was this intentional?  Providing a full complement of
pg_statio_xact_* views seems like overkill to me, but maybe that was where
you were intending to go and forgot.  If the functions are there then
anyone who needs the functionality can easily build their own views atop
them, so this might be an intentional compromise position, but I'm not
sure.  Or maybe we should decide that intratransaction statio numbers
aren't likely to be of interest to anybody, and drop the functions too.

                       regards, tom lane