[5.5] Fix SQL Server handling of DATETIME columns

[paulofreitas]

Consider you're using this column in a migration:

 $table->timestamp('created_at')->nullable()->useCurrent();

Because SQL Server schema grammar is using the DATETIME column type for a timestamp() column, the column will get a fixed precision of 3 fractional seconds which when used with DEFAULT CURRENT_TIMESTAMP constraint would store values like "2017-11-13 06:25:52.150" that will fail to parse by Carbon if you declare this column on the model's $dates property. 👍

This is the SQL being generated for this column: http://sqlfiddle.com/#!6/16af5/1

This is the exception being thrown:

InvalidArgumentException: The format separator does not match.

This is happening on Iluminate\Database\Eloquent\Concerns\HasAttributes->asDateTime() method at line 715 because $this->getDateFormat() is receiving "Y-m-d H:i:s.000" and "2017-11-13 06:25:52.150" as arguments and this should fail: https://3v4l.org/Nu1p4

I've found this with a fresh Laravel 5.5 install and SQL Server 2016 via Amazon RDS. The proposed change fixes the problem. It seems there's not tests for this yet, I'll try writing them later.

[taylorotwell]

Is this not a breaking change for 5.5?

[paulofreitas]

@taylorotwell Actually I was thinking it wasn't, but it was a breaking change for a different reason. Using Y-m-d H:i:s.u fixes the timestamp retrieval from database but also breaks the timestamp storing.

This happens because Illuminate\Database\Eloquent\Concerns\HasAttributes->getDateFormat() is being used both for timestamp retrieval in HasAttributes->asDateTime() and storing in HasAttributes->fromDateTime(). The problem here is that SQL Server's DATETIME type is more restrictive than DATETIME2 and won't allow passing a value with more than 3 fractional seconds (DATETIME2 would just ignore wider precisions instead) - so you can't use Y-m-d H:i:s.u because Carbon will give it 6 fractional seconds at HasAttributes->fromDateTime().

To fix the whole issue we should be using Y-m-d H:i:s.u when retrieving and Y-m-d H:i:s.v when storing (sadly the v format character is only available to the DateTime->format() method, not in DateTime::createFromFormat()) - yeah, this sounds messy, but seems to be the simplest fix. This would make HasAttributes->fromDateTime() return a timestamp with milliseconds in place of microseconds and this should be fine to DATETIME. If we were using DATETIME2 or if DATETIME wasn't so restrictive this wouldn't be required (that's why I'm suggesting changing to DATETIME2 on #22004).

The easier fix for 5.5 that I've found is changing Illuminate\Database\Query\Grammars\SqlServerGrammar->getDateFormat() to use a default of Y-m-d H:i:s.v and changing Illuminate\Database\Eloquent\Concerns\HasAttributes->asDateTime() to replace .v to .u when retrieving a datetime/timestamp value. I've changed this PR to this new solution since it fixed the whole issue in my tests.

I've added some basic tests that I believe are fulfilling both the expected and unexpected scenarios. 👍

Note that this solution shouldn't break anything, it's just a hack to allow retrieving and storing SQL Server's DATETIME columns correctly. 😉

---

Now a technical explanation as to why currently it's hardcoded to Y-m-d H:i:s.000.

When you store a timestamp like "2017-11-13 06:25:52" in a dateTime() or timestamp() column using the SQL Server driver it get stored as "2017-11-13 06:25:52.000" at the database table - this happens because those columns are being mapped to use the DATETIME data type (which has a fixed precision of 3 fractional seconds). This is what we get in most situations.

But there are some edge cases. Using the useCurrent() modifier in a timestamp() column would be an edge case here because using the DEFAULT CURRENT_TIMESTAMP constraint it would allow SQL Server to store any fractional second for the moment a column get updated. This would break Carbon when retrieving the stored column value. The default() column modifier would be another edge case here for the same reason. Manually updating the column's fractional second would also be an edge case that may break the column value retrieval.

It would be sufficient using Y-m-d H:i:s.u if DATETIME wasn't so restrictive. In fact this is what we should use in $dateFormat when using custom columns precisions in MySQL/PostgreSQL/SQL Server.

[soniibrol]

Hi, I get this error when using Laravel 5.5.28 on my Ubuntu server. Any idea how to resolve this problem?

[neclimdul]

Is this not a breaking change for 5.5?

FWIW, this was a breaking change for me. I get "Missing data" errors from Carbon saving an updated record. Currently reverting on my installs.

[vdbelt]

Using .v for fractional seconds in some cases will result in write errors due to a PHP bug which appears to be only resolved in > PHP 7.2.0.

The fractional seconds will jump from 999 to 1000 because of rounding errors, making it 4 digits, which mssql's DATETIME will not accept.

Reference:
https://bugs.php.net/bug.php?id=74753

[neclimdul]

That's not the problem, its every save and I'm running 7.2.