[12.x] Fix Factory@insert() to allow for array casts

[cosmastech]

This fix stops double quoting JSON arrays when using Factory@insert().

---

Thanks to @zepfietje for reporting this issue and providing reproduction steps. 🙇 #57600 (comment)

@riesjart You had mentioned there were multiple reasons for not using $model->toArray() in your PR. Do you see any issue with this change?

[riesjart]

@riesjart You had mentioned there were multiple reasons for not using $model->toArray() in your PR. Do you see any issue with this change?

I think the cause of the issue can be found within the Factory::insert() method. It's using Eloquent\Builder::fillForInsert() instead of Eloquent\Builder::insert(). This also means that for each record a new model is being instantiated (again!), while there was already a collection of model instances available (see Factory::insert()). Apart from this causing the double quoting JSON, it also seems quite inefficient to me. Especially since one typically uses the insert() functionality for large record sets.

I do understand why Eloquent\Builder::fillForInsert() is being called, since the setUniqueIds() and addTimestampsToUpsertValues() calls are needed. I think it would be better to call these methods directly on the model instances already available from within the Factory::insert(). The latter addTimestampsToUpsertValues() method is currently protected though.

[cosmastech]

@riesjart You had mentioned there were multiple reasons for not using $model->toArray() in your PR. Do you see any issue with this change?

I think the cause of the issue can be found within the Factory::insert() method. It's using Eloquent\Builder::fillForInsert() instead of Eloquent\Builder::insert(). This also means that for each record a new model is being instantiated (again!), while there was already a collection of model instances available (see Factory::insert()). Apart from this causing the double quoting JSON, it also seems quite inefficient to me. Especially since one typically uses the insert() functionality for large record sets.

I do understand why Eloquent\Builder::fillForInsert() is being called, since the setUniqueIds() and addTimestampsToUpsertValues() calls are needed. I think it would be better to call these methods directly on the model instances already available from within the Factory::insert(). The latter addTimestampsToUpsertValues() method is currently protected though.

That's a fair point about instantiating multiple times. Provided that prior to this change, most of us were creating these models via a series of insert statements, I still think we're on the winning end of speed.

My hope is to fix the break we have, and was curious if you see there being anything that wouldn't work, even if sub-optimal on the instantiation front.

[riesjart]

I think trying to cast 'already-insert-ready' (raw) attributes (Model::$attributes) to their cast values back and forth to eventually insert them, is an anti-pattern and should be avoided. It is not what attributes casts and ...toArray() calls are meant for. This is why I introduced #57670.
The anti-pattern can cause all kinds of issues, I think the double quoting of the JSON values is just one that first came to light. Apart from the inefficiency at runtime, it also makes the code harder to understand and maintain.

I created #57799 as an alternative, what do you think?