Fix: Handle mixed-type values in compileInsert

[alipadron]

This pull request addresses a potential issue that could arise when using Model::query()->create() and the first element in the attributes list is an array (representing a JSON column) while subsequent elements are not arrays.

The original code might have encountered type hinting issues due to this structure.

This change ensures that the compileInsert method in Grammar.php correctly handles such scenarios. Additionally, new unit tests in DatabaseQueryGrammarTest.php verify this behavior for various insert conditions.

Benefits:

• Improves robustness of data insertion with mixed value structures
• Enhances code maintainability with clearer handling of edge cases

Notes:

Leverages array_is_list function (available in PHP 8.1+) for improved type checking

Example 1

User::query()->create([
    'possible_json_column' => [1, 2, 3, 4],
    'name' => 'Test',
    'email' => '[email protected]'
]);

Throws:

TypeError Illuminate\Database\Grammar::parameterize(): Argument #1 ($values) must be of type array, string given, called in vendor\laravel\framework\src\Illuminate\Database\Query\Grammars\Grammar.php on line 1180

If you instead try:

User::query()->create([
    'name' => 'Test',
    'possible_json_column' => [1, 2, 3, 4],
    'email' => '[email protected]',
]);

It won't give you the error.

That is because this line when reset($values) is called it returns the first element in the $values array but $values is an associative array so it returns [1, 2, 3, 4] and the next check ! is_array([1, 2, 3, 4]) gives false and it does not wrap the associative array in a list array.

Then in line 1180 when new Collection($values) is called it uses

new Collection([
    'possible_json_column' => [1, 2, 3, 4],
    'name' => 'Test',
    'email' => '[email protected]'
]);

to create the collection and iterates over its values.

When the code tries to execute $this->parameterize($record) fails for the second iteration because $record is not an associative array (it is the 'Test' string).

Example 2

If you try this scenario:

User::query()->toBase()->grammar->compileInsert(User::query()->toBase(), ['possible_json' => [1, 2, 3, 4]]);

Gives:

insert into "users" ("0", "1", "2", "3") values (?, ?, ?, ?)

Example 3

When you try:

User::query()->toBase()->grammar->compileInsert(User::query()->toBase(), ['possible_json' => [1, 2, 3, 4], 'another_column' => 'value']);

It gives you the TypeError mentioned before.

Example 4

Instead, when you use:

User::query()->toBase()->grammar->compileInsert(User::query()->toBase(), ['another_column' => 'value', 'possible_json' => [1, 2, 3, 4]]);

It gives you the expected result:

insert into "users" ("another_column", "possible_json") values (?, ?)

Conclusion

We can conclude that when the first value of the array is an array, the code fails. An easy fix would be to just change the order of the array values, but it will give hard time to anyone who is in a similar situation.

Please review and provide feedback. Thanks!

[alipadron]

I will take a look at unsuccessful checks!

[taylorotwell]

Sorry, can you provide very simple recreation steps for how to trigger the error? Please mark as ready for review when done.

[alipadron]

I have added the reproduction steps in the description of the pull request, i included four examples.

Let me know if it is clear! Thanks!

[fisharebest]

This change breaks my application.

I'm building up data in a nested array before inserting all rows at once. As a result my rows have keys. e.g.

$rows = [
    'key1' => ['name' => 'John Doe', 'email' => '[email protected]'],
    'key2' => ['name' => 'Alice Wong', 'email' => '[email protected]'],
];

DB::table('t')->insert($rows);

Since 5.8 (and probably very much earlier) and until 11.36.1, this worked fine. Since 11.37.0, it fails.

Now I appreciate the issue you are trying to fix, and I appreciate that I have a workaround
(->insert(array_values($rows))), but I don't expect breaking changes in a minor release.

Perhaps this should have been defered to 12.0 where it could be documented?