Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use statement_timestamp() instead of now() #19

Merged
merged 9 commits into from
Jan 29, 2025
Merged

Conversation

andrepiske
Copy link
Collaborator

@andrepiske andrepiske commented Jan 28, 2025

Postgresql has a handful of functions to get the current timestamp, with varying exact meanings for each one.

So far we've been using the NOW() function which returns the timestamp of the beginning of a transaction block rather than the actual clock time. This PR changes it to use the STATEMENT_TIMESTAMP() function instead, which returns the timestamp of the beginning of a statement. Postgres documentation does a better job of explaining how exactly each of those functions work: https://www.postgresql.org/docs/current/functions-datetime.html#FUNCTIONS-DATETIME-CURRENT

In rails, a common pattern is to do

ActiveRecord::Base.transaction do
  ...
end

and in this case, the timestamp returned by NOW() will be set by when ActiveRecord::Base.transaction is called and not when statements inside the block are issued.

There are ups and downs to using each one and no right answer as to which one to use. But we feel that with this change, timestamps will behave more like a wall clock, especially in long or slow transactions.


Related tasks:

@andrepiske andrepiske changed the title use statement timestamp Use statement_timestamp() instead of now() Jan 28, 2025
@andrepiske andrepiske marked this pull request as ready for review January 28, 2025 17:25
@andrepiske andrepiske self-assigned this Jan 28, 2025
@andrepiske andrepiske requested a review from a team January 28, 2025 17:29
Base automatically changed from use-model-timestamps to main January 29, 2025 18:11
@andrepiske andrepiske merged commit 5388701 into main Jan 29, 2025
7 checks passed
@andrepiske andrepiske deleted the use-statement-time branch January 29, 2025 18:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants