Re: now() vs transaction_timestamp() - Mailing list pgsql-hackers

On 06.10.2018 00:25, Tom Lane wrote:
> I wrote:
>> Konstantin Knizhnik <k.knizhnik@postgrespro.ru> writes:
>>> Postgres documentation says that |"now()| is a traditional PostgreSQL
>>> equivalent to |transaction_timestamp()|".
>>> Also both use the same implementation.
>> Right.
>>> But them have different parallel safety property:
>> That seems like a bug/thinko.  I am not sure which property setting is
>> correct though.  It'd only be safe if the parallel-query infrastructure
>> transfers the relevant timestamp to workers, and I don't know if it does.
> The answer is that it doesn't; indeed, xactStartTimestamp is local inside
> xact.c, and it's easy to see by inspection that there is nothing at all
> that sets it except StartTransaction().  What is happening, if you do
>
> set force_parallel_mode to 1;
> select transaction_timestamp();
>
> is that the value you get back is the time that the parallel worker did
> StartTransaction().  It is easy to show that this is utterly broken:
> transaction_timestamp() holds still within a transaction until you set
> force_parallel_mode, and then it does not.
>
> regression=# begin;
> BEGIN
> regression=# select transaction_timestamp();
>       transaction_timestamp
> -------------------------------
>   2018-10-05 17:00:11.764019-04
> (1 row)
>
> regression=# select transaction_timestamp();
>       transaction_timestamp
> -------------------------------
>   2018-10-05 17:00:11.764019-04
> (1 row)
>
> regression=# select transaction_timestamp();
>       transaction_timestamp
> -------------------------------
>   2018-10-05 17:00:11.764019-04
> (1 row)
>
> regression=# set force_parallel_mode to 1;
> SET
> regression=# select transaction_timestamp();
>       transaction_timestamp
> -------------------------------
>   2018-10-05 17:00:21.983122-04
> (1 row)
>
> Looking at the related functions, I see that now() and
> statement_timestamp() are marked stable/restricted, which is OK, while
> clock_timestamp() and timeofday() are marked volatile/safe which seems odd
> but on reflection I think it's OK.  Their values wouldn't hold still in
> the parent process either, so there's no reason to disallow workers from
> running them.
>
> So transaction_timestamp() is definitely buggy, but we're not out of the
> woods yet: SQLValueFunction is treated as parallel-safe, but it also has
> some instances that are equivalent to transaction_timestamp and so do not
> work correctly.
>
> regression=# begin;
> BEGIN
> regression=# select current_time;
>      current_time
> --------------------
>   17:12:35.942968-04
> (1 row)
>
> regression=# select current_time;
>      current_time
> --------------------
>   17:12:35.942968-04
> (1 row)
>
> regression=# set force_parallel_mode to 1;
> SET
> regression=# select current_time;
>      current_time
> --------------------
>   17:12:55.462141-04
> (1 row)
>
> regression=# set force_parallel_mode to 0;
> SET
> regression=# select current_time;
>      current_time
> --------------------
>   17:12:35.942968-04
> (1 row)
>
> Ain't that fun?
>
> My initial thought was that we should just re-mark transaction_timestamp()
> as parallel-restricted and call it a day, but we'd then have to do the
> same for SQLValueFunction, which is not much fun because it does have
> variants that are parallel safe (and teaching max_parallel_hazard_walker
> which is which seems like a recipe for bugs).
>
> Also, while it might not be quite too late to force a catversion bump
> in v11, this is demonstrably also broken in v10, and we can't do that
> there.
>
> So maybe the right answer is to change the parallel mode infrastructure
> so it transmits xactStartTimestamp, making transaction_timestamp()
> retroactively safe, and then in HEAD only we could re-mark now() as
> safe.  We might as well do the same for statement_timestamp as well.
>
> Thoughts?
>
>             regards, tom lane
Attached please find very small patch fixing the problem (propagating 
transaction and statement timestamps to workers).