On 2018-09-28 09:35:48 +0200, Peter Eisentraut wrote:
> On 26/09/2018 23:48, Peter Eisentraut wrote:
> > That's certainly a good argument. Note that if we implemented that the
> > transaction timestamp is advanced inside procedures, that would also
> > mean that the transaction timestamp as observed in pg_stat_activity
> > would move during VACUUM, for example. That might or might not be
> > desirable.
I think it doesn't hurt, although it's also not a huge advantage.
> diff --git a/src/backend/access/transam/xact.c b/src/backend/access/transam/xact.c
> index 9aa63c8792..245735420c 100644
> --- a/src/backend/access/transam/xact.c
> +++ b/src/backend/access/transam/xact.c
> @@ -1884,14 +1884,19 @@ StartTransaction(void)
> TRACE_POSTGRESQL_TRANSACTION_START(vxid.localTransactionId);
>
> /*
> - * set transaction_timestamp() (a/k/a now()). We want this to be the same
> - * as the first command's statement_timestamp(), so don't do a fresh
> - * GetCurrentTimestamp() call (which'd be expensive anyway). Also, mark
> - * xactStopTimestamp as unset.
> - */
> - xactStartTimestamp = stmtStartTimestamp;
> - xactStopTimestamp = 0;
> + * set transaction_timestamp() (a/k/a now()). Normally, we want this to
> + * be the same as the first command's statement_timestamp(), so don't do a
> + * fresh GetCurrentTimestamp() call (which'd be expensive anyway). But
> + * for transactions started inside statements (e.g., procedure calls), we
> + * want to advance the timestamp.
> + */
> + if (xactStartTimestamp < stmtStartTimestamp)
> + xactStartTimestamp = stmtStartTimestamp;
> + else
> + xactStartTimestamp = GetCurrentTimestamp();
> pgstat_report_xact_timestamp(xactStartTimestamp);
> + /* Mark xactStopTimestamp as unset. */
> + xactStopTimestamp = 0;
It's a bit weird to make this decision based on these two timestamps
differing. For one, it only indirectly seems to be guaranteed that
xactStartTimestamp is even set to anything here (to 0 by virtue of being
a global var).
Greetings,
Andres Freund