Skip to content

Comments

[12.x] Adjust freshTimestamp for SQL Server#58614

Merged
taylorotwell merged 1 commit intolaravel:12.xfrom
aimeos:12.x
Feb 4, 2026
Merged

[12.x] Adjust freshTimestamp for SQL Server#58614
taylorotwell merged 1 commit intolaravel:12.xfrom
aimeos:12.x

Conversation

@aimeos
Copy link
Contributor

@aimeos aimeos commented Feb 4, 2026

Contrary to other RDBMS, SQL Server stores microseconds in timestamps regardless of the timestamp precision and rounds the timestamp according to the microsecond value. This often leads to "off by one second" timestamps esp. in tests because timestamps stored as "2026-02-04 11:39:54.742" are rounded to "2026-02-04 11:39:55" while the original timestamp in Laravel is still "2026-02-04 11:39:54".

This fix ensures that all timestamps are stored with "000" as microseconds to ensure compatibility with all other RDBMS and to avoid the "off by one" problem in tests. The fix isn't breaking and if microseconds for timestamps are required, they can still be enabled in the booted() method of the model.

@taylorotwell
Copy link
Member

Feels like this could be breaking.

@aimeos
Copy link
Contributor Author

aimeos commented Feb 4, 2026

@taylorotwell Could you please explain why "it feels breaking"?

@jackbayliss
Copy link
Contributor

jackbayliss commented Feb 4, 2026

@aimeos He means you should essentially target the master branch (13.x), as it feels risky for 12.x :)

So, probably worth opening a new branch targeting 13.x

@AndrewMast
Copy link
Contributor

@aimeos I created an alternative PR that could help fix this: #58620

@taylorotwell taylorotwell reopened this Feb 4, 2026
@taylorotwell taylorotwell merged commit 7fa1c8f into laravel:12.x Feb 4, 2026
140 of 141 checks passed
@taylorotwell
Copy link
Member

Let's merge it and find out then. :)

@aimeos
Copy link
Contributor Author

aimeos commented Feb 5, 2026

@taylorotwell Should I create a PR for 11.x too?

@calebdw
Copy link
Contributor

calebdw commented Feb 11, 2026

@taylorotwell, @aimeos

This is breaking our CI. If ->touch() is called shortly after a model was created then it doesn't register the touch and events aren't fired. We use Postgres and we store timestamps with the default precision of 6---the timestamp SHOULD NOT be arbitrarily truncated to the beginning of the second.

If anything, there needs to be a callback that can be registered, such as what @AndrewMast proposed

@dylanbr
Copy link
Contributor

dylanbr commented Feb 11, 2026

There are tests now failing for me too, due to this change.

For now I have worked around it by adding the old version of freshTimestamp() to a base Model class, but hopefully there is a better solution, which doesn't change the default behavior.

the timestamp SHOULD NOT be arbitrarily truncated to the beginning of the second.

Agreed.

@jackbayliss
Copy link
Contributor

jackbayliss commented Feb 11, 2026

I would potentially send a revert PR to 12.x for now as seems a B/C and then a solution can be done in a separate PR.

calebdw added a commit to calebdw/laravel-framework that referenced this pull request Feb 11, 2026
@aimeos
Copy link
Contributor Author

aimeos commented Feb 11, 2026

@calebdw Default timestamps have a precision of 0 by default in Eloquent models. If you've changed the default timestamp format in the model to use microseconds, you should overwrite freshTimestamp() to return Date::now().

@taylorotwell As it does break more as I thought, reverting the PR in 12.x is OK but I would like to get that into 13.x nevertheless to streamline default behavior across all supported databases.

@calebdw
Copy link
Contributor

calebdw commented Feb 11, 2026

@aimeos, I would prefer @AndrewMast solution to make it easy to override for all models, not just models that extend a particular base class

@dylanbr
Copy link
Contributor

dylanbr commented Feb 11, 2026

Should this not be controlled by getDateFormat() in the Query\Grammar?

I am overriding this to 'Y-m-d H:i:s.uO' so that timestamps are written with microsecond precision for Postgres.

Wouldn't it make sense to use this when dealing with freshTimestamp() as well?

I see SqlServerGrammar is using a date format of 'Y-m-d H:i:s.v', so it includes milliseconds. Couldn't the same thing be accomplished by overriding this to 'Y-m-d H:i:s', and leaving freshTimestamp as it was before?

When I said "default" previously, I meant the default for your setup, which would be getDateFormat(). And if you want your freshTimestamp() specifically to differ from default, you could to override it to something other than now(). But having it as now() without any changes, means it will work for any date format chosen.

Edit: Maybe getDateFormat() should be removed from SqlServerGrammar? It is the only one that overrides the default date format out of the box. I don't use SQL Server, but this seems confusing, although maybe there is some historical context where this makes sense.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants