laravel / framework

The Laravel Framework.
https://laravel.com
MIT License
32.51k stars 11.02k forks source link

Incorrect date format in SQL query from custom date format cast on model #46737

Closed vHeemstra closed 1 year ago

vHeemstra commented 1 year ago

Laravel Version

10.6.2

PHP Version

8.1.10

Database Driver & Version

MySQL 10.4.25-MariaDB (using Xampp with Apache) on Windows 11 Pro

Description

When updating a model using a PATCH request, a custom date attribute is not being correctly transformed to the valid MySQL date value. Resulting in the error:

SQLSTATE[22007]: Invalid datetime format: 1292 Incorrect date value: '20-10-2000' for column `users`.`date_of_birth` at row 1 (Connection: mysql, SQL: update `users` set `date_of_birth` = 20-10-2000, `users`.`updated_at` = 2023-04-10 11:38:19 where `id` = 1)

Steps To Reproduce

Database (MySQL)

Column date_of_birth is type DATE.

Migration

The column was created using $table->date('date_of_birth')

Model

On the default User model, I added the attribute to fillable and set a custom date cast (as described in the docs).

protected $casts = [
    // other attributes
    'date_of_birth' => 'date:d-m-Y',
];

protected $fillable = [
    // other attributes
    'date_of_birth',
];

And according to the documentation about date serialization, this should not change the format used for storing it in the database, only the format used in PHP array/JSON model instances.

Steps/flow

  1. When loading the page with an edit form, the attribute on the User model is in the correct d-m-Y format.
  2. The post/patch request sends the date value in d-m-Y format as well.
    Route definition in `web.php`
Route::patch('/users/{user}', [UsersController::class, 'update'])->name('users.update');

  1. The values are correctly validated by the rule in UserUpdateRequest:
public function rules(): array
{
    return [
        // other attributes' rules 
        'date_of_birth' => 'nullable|date_format:d-m-Y',
    ];
}
  1. The controller fills the values in the model:
`update` method on the controller ```php public function update(UserUpdateRequest $request, User $user) { $user->fill($request->validated()); // dd($user->date_of_birth); <-- Shows it's a Carbon object with the correct value! $user->save(); return response()->json([ 'status' => 'ok', 'user' => $user, ]); } ```
  1. The JSON response (back to the form page) has the error:
SQLSTATE[22007]: Invalid datetime format: 1292 Incorrect date value: '20-10-2000' for column `users`.`date_of_birth` at row 1 (Connection: mysql, SQL: update `users` set `date_of_birth` = 20-10-2000, `users`.`updated_at` = 2023-04-10 11:38:19 where `id` = 1)

Somehow the framework did not correctly turn the Carbon date object into a valid MySQL date representation. So either:

  1. there is something wrong in the Carbon to MySQL transformation
  2. or the date format defined in the model's casts property is wrongly being used here
  3. or the documentation (1 and 2) is incorrect.

Note: Setting the model's attribute cast to date:Y-m-d does not produce this error. Of course, this is also the exact format MySQL's DATE field uses. This also indicates the issue is option 2.

Note 2: Sending/posting the same value as currently in the DB/on the model, does not give an error. (I guess the attribute is skipped in the SQL here because it hasn't changed.)

Update: created example repo

Repo at: https://github.com/vHeemstra/laravel-date-format-bug

To set it up:

github-actions[bot] commented 1 year ago

Thank you for reporting this issue!

As Laravel is an open source project, we rely on the community to help us diagnose and fix issues as it is not possible to research and fix every issue reported to us via GitHub.

If possible, please make a pull request fixing the issue you have described, along with corresponding tests. All pull requests are promptly reviewed by the Laravel team.

Thank you!

cosmastech commented 1 year ago

@vHeemstra could you link to a repo that shows this is failing?

I tried to write a test case and it's not failing. https://github.com/cosmastech/laravel-framework/pull/1/files Do I have some part of the setup wrong? It could be that it's using SQLite instead of MySQL 🤷 but I'll admit I feel that's not the issue.

tttogo commented 1 year ago

Just FYI, I had a very similar thing with Datetime to Timestamp, where casting just wouldn't work.

Workaround was to write a mutator as per https://laravel.com/docs/10.x/eloquent-mutators#defining-a-mutator.

Sorry I can't offer anything more helpful!

vHeemstra commented 1 year ago

@cosmastech I will try to create a working example.

It might be that MySQL is needed, because the Exception is thrown even before any SQL query is sent. Using:

DB::enableQueryLog();
try {
  $user->save();
} catch (\Exception $e) {}
dd(DB::getQueryLog());

The dd'ed result was an empty array, so I guess the DB grammar/adopter (idk) checks all of this before sending the query and there finds the wrong format?

Full stack trace of the Exception ``` local.ERROR: SQLSTATE[22007]: Invalid datetime format: 1292 Incorrect date value: '20-01-2000' for column `familytree`.`users`.`date_of_birth` at row 1 (Connection: mysql, SQL: update `users` set `date_of_birth` = 20-01-2000, `users`.`updated_at` = 2023-04-11 13:49:54 where `id` = 1) {"userId":1,"exception":"[object] (Illuminate\\Database\\QueryException(code: 22007): [...same line as above...] at D:\\...etc...\\vendor\\laravel\\framework\\src\\Illuminate\\Database\\Connection.php:793) [stacktrace] #0 D:\\...etc...\\vendor\\laravel\\framework\\src\\Illuminate\\Database\\Connection.php(753): Illuminate\\Database\\Connection->runQueryCallback('update `users` ...', Array, Object(Closure)) #1 D:\\...etc...\\vendor\\laravel\\framework\\src\\Illuminate\\Database\\Connection.php(610): Illuminate\\Database\\Connection->run('update `users` ...', Array, Object(Closure)) #2 D:\\...etc...\\vendor\\laravel\\framework\\src\\Illuminate\\Database\\Connection.php(543): Illuminate\\Database\\Connection->affectingStatement('update `users` ...', Array) #3 D:\\...etc...\\vendor\\laravel\\framework\\src\\Illuminate\\Database\\Query\\Builder.php(3376): Illuminate\\Database\\Connection->update('update `users` ...', Array) #4 D:\\...etc...\\vendor\\laravel\\framework\\src\\Illuminate\\Database\\Eloquent\\Builder.php(1021): Illuminate\\Database\\Query\\Builder->update(Array) #5 D:\\...etc...\\vendor\\laravel\\framework\\src\\Illuminate\\Database\\Eloquent\\Model.php(1216): Illuminate\\Database\\Eloquent\\Builder->update(Array) #6 D:\\...etc...\\vendor\\laravel\\framework\\src\\Illuminate\\Database\\Eloquent\\Model.php(1133): Illuminate\\Database\\Eloquent\\Model->performUpdate(Object(Illuminate\\Database\\Eloquent\\Builder)) #7 D:\\...etc...\\app\\Http\\Controllers\\UsersController.php(99): Illuminate\\Database\\Eloquent\\Model->save() (Up until here, all is fine.) ```

As stated in the report above, the date is correctly filled in the user model instance and becomes a Carbon instance with the correct date. It is just that when compiling/preparing the DB query, the Carbon object is turned into a string using the wrong format (that from $casts instead of the MySQL one). Possibly because it might use HasAttributes::attributesToArray()? Which calls HasAttributes::addCastAttributesToArray, which calls the format method on the Carbon instance with the format from $casts.

@tttogo Thanks, yeah I might do that for the moment. Or switch to Y-m-d and have front-end rearrange the date. 😄

vHeemstra commented 1 year ago

@vHeemstra could you link to a repo that shows this is failing?

I tried to write a test case and it's not failing. https://github.com/cosmastech/laravel-framework/pull/1/files Do I have some part of the setup wrong? It could be that it's using SQLite instead of MySQL 🤷 but I'll admit I feel that's not the issue.

Repo at: https://github.com/vHeemstra/laravel-date-format-bug

To set it up:

cosmastech commented 1 year ago

The application worked fine for me.

image image

edit: note, I am running PHP 8.2.4 on Mac OS X & MySQL 8.0.27

vHeemstra commented 1 year ago

@cosmastech First off, thank you so much for taking the time to help!

Second, how strange.. The axios request (when submitting the modal form with a different d-m-Y date) returns with status 200?

I feel OS should not make a difference here. Maybe MySQL versions and/or PHP version, in the way some of the magic getters/setters are implemented??

vHeemstra commented 1 year ago

I just made a new clone from the repo and installed using the steps above. After submitting the edit modal form with only the date set, I still get the described error back from the server:

image

For completeness, these are the installed dependencies' versions:

composer.json

image

cosmastech commented 1 year ago

@tttogo when you were running into this issue, were you using Laravel 10? What version of PHP, DB, etc?

cosmastech commented 1 year ago

Now I'm getting the error! I was editing my profile, not the Personen edit page.

cosmastech commented 1 year ago

Found the issue, will put up a PR for this

driesvints commented 1 year ago

It seems Taylor has decided to not take any action here, sorry.

vHeemstra commented 1 year ago

But that does not address the issue in the bug report. As Taylor says, the cast format should only be used when model instances become arrays or JSON, but it should not interfere with the way it is stored in the database. (See also the referenced docs in my comment there and in the bug report above.)

But it does interfere. So it is still a bug that needs a fix. (Or the docs need changing.)

sandcore-dev commented 7 months ago

I encountered the same problem. Casting with just 'datetime' and $dateFormat = 'U' will ensure that both Unix timestamps and Carbon objects are correctly stored, but if you usedatetime:U and set the value with a Carbon object, it won't be stored correctly.

This is because hasCast() only looks for datetime and a few other datetime strings.

To get around this, I implemented a custom cast that accepts both.