PHP DateTime Milliseconds rounding error can cause SqlServer DB Error

[leftbak]

• Laravel Version: v5.5.24
• PHP Version: 7.1.8

Description:

A bug in PHP (below 7.2) causes microsecond values >=999500 to be rounded to '1000' when formatted using 'v'.

Since 3af1858 set the SQL Server date format to 'Y-m-d H:i:s.v' this means that these values are put into a date string in as '2017-11-14 08:23:19.1000' which causes an illegal write exception in SQL Server as a datetime column can only support milliseconds.

This affects the Builder->addUpdatedAtColumn method which is called on all models with timestamps as this method defers to Model->freshTimestampString() which uses getDateFormat() to output the date as a string - meaning every model will occasionally try and set updated_at to an illegal value and causes the SQL write to fail.

Steps To Reproduce:

By changing the below test from framework/tests/Database/DatabaseEloquentIntegrationTest.php you can cause it to fail - proving the string rounding/truncating bug.

    public function testTimestampsUsingDefaultSqlServerDateFormat()
    {
        $model = new EloquentTestUser;
        $model->setDateFormat('Y-m-d H:i:s.v'); // Default SQL Server date format
        $model->setRawAttributes([
            'created_at' => '2017-11-14 08:23:19.000',
            'updated_at' => '2017-11-14 08:23:19.999999',
        ]);

        $this->assertEquals('2017-11-14 08:23:19.000000', $this->getRawDateTimeString($model->getAttribute('created_at')));
        $this->assertEquals('2017-11-14 08:23:19.999999', $this->getRawDateTimeString($model->getAttribute('updated_at')));
        $this->assertEquals('2017-11-14 08:23:19.000', $model->fromDateTime($model->getAttribute('created_at')));
        $this->assertEquals('2017-11-14 08:23:19.999', $model->fromDateTime($model->getAttribute('updated_at')));
    }

[sisve]

Your test does not seem to involve any database at all. Is the mention of SQL Server important at all?

[leftbak]

You're correct, the test doesn't involve a database - mainly because you can prove the incorrect rounding without hitting the database.

However if you use MySQL and write '2017-11-14 08:23:19.1000' into a datetime column then it accepts the value - so although incorrect due to bad rounding, it causes no errors.

Using SqlServer a datetime column cannot accept more than milliseconds and so the database write fails - because this code is used by the updated_at column there's potential for any model update with timestamps to fail on SqlServer (1 in 2000 chance).

[sisve]

Can you update the issue to focus on what you're actually reporting? If a database is irrelevant, then there's no need to mention SQL Server. It looks like you have a problem with a Model method, but it's unclear which. The numbers you're showing has the milliseconds cut off, they're not rounded.

1. What's getRawDateTimeString?
2. Have you specified a $dateFormat yourself, or are you falling back to the current database grammar (which for SQL Server is three decimals)?

[sisve]

I also see that you've filed a bug in the carbon repository. Is this a Carbon bug? If so, wouldn't it be enough for them to fix it?

Okay, okay, I almost feel bad for saying that. They don't fix stuff. briannesbitt/Carbon#863

[leftbak]

I just discovered that it's actually a PHP DateTime bug. Need to close that Carbon issue...

I'll update the issue to hopefully explain it better

[Miguel-Serejo]

I'm not sure I quite understand what your issue here is.
Are you reading values from a MySql database and writing to SqlServer?

Is "08:23:19.1000" with a "H:i:s.v" format even a valid date? I'd expect it to truncate to .100 as milliseconds are only three digits. If it were ".u", I'd expect it to zerofill to .100000, so I don't see how or why you'd get .999 out of it by "truncating".

[leftbak]

Updated issue description - hopefully explains it better.

The problem is caused by a bug in PHP DateTime format - but it's due to a change in the Laravel framework that it's been exposed to users to SQL Server.

[vdbelt]

Can confirm this is also causing issues here.

The bug seems to be solved in PHP 7.2 (https://bugs.php.net/bug.php?id=74753), but current implementation will give database errors in versions lower than that.

[uxweb]

I have the same issue.

I discovered it when I was running some seeders as they insert hundreds of rows that affect the created_at column with the generated date using the sql server grammar date format "Y-m-d H:i:s.v".

[BertCly]

Is there an alternative solution for this issue?
Upgrading to PHP 7.2 seems not possible as there is no SQL Server driver for PHP 7.2 & Ubuntu.

I solved the issue by creating a BaseModel. All models should extend this BaseModel.
class BaseModel extends Model { public function getDateFormat() { return 'Y-m-d H:i:s.000'; } }
All datetimes with milliseconds should be updated:
UPDATE table SET created_at=DATEADD(ms, -DATEPART(ms,created_at), created_at)

[FrittenKeeZ]

According to this, there should be PHP 7.2 drivers for Ubuntu 16.04.
In regards to @BertKUL's response, I opted for a trait instead:

trait DateFormat
{
    /**
     * Get the format for database stored dates.
     *
     * @return string
     */
    public function getDateFormat()
    {
        return str_replace(['.v', '.u'], '.000', parent::getDateFormat());
    }
}

As far as I can see, a proper solution would require additional logic for these code pieces:
Illuminate\Database\Connection#L595
Illuminate\Database\Eloquent\Concerns\HasAttributes#L795
Illuminate\Database\Eloquent\Relations\Concerns\InteractsWithPivotTable#L324
Basically check if PHP is version 7.1 and date format contains .v, checking if it'll round to 1000, and add +1 sec to the datetime instance and zero out the microseconds.

[laurencei]

Closing this, as it seems to be a bug in PHP7.2.

You can use the solution posted by @FrittenKeeZ as a backport workaround if needed.