Saving a model fails if there's too many children in a touches relationship

[creekorful]

• Laravel Version: 8.49.2
• PHP Version: 8.0.11
• Database Driver & Version: mysql

Description:

When saving a model that define touches, the save fails with:

SQLSTATE[HY000]: General error: 1390 Prepared statement contains too many placeholders

If there's too many children in the touched relation.

Steps To Reproduce:

• Have a model with relationship

• Have an high number of child in the relationship.

• Define the relationship in $touches.

• Update the parent and then save it: the request will fails with the given error message.

Steps to Resolve

I think we need to chunk the $ids before updating, to make sure we doesn't reach the placeholder limit. See:

framework/src/Illuminate/Database/Eloquent/Relations/BelongsToMany.php

Line 1102 in 52e2376

$this->getRelated()->newQueryWithoutRelationships()->whereIn($key, $ids)->update($columns);

Notes

In our case, this happens with a BelongsToMany relationship but I suppose other relationship may be affected.

[creekorful]

I'm willing to do the PR of course, if you are interested in getting this fixed upstream :)

[driesvints]

Insert a high number of child (127,268 in our case) in the relationship.

Don't do this. Chunk the insertions into queued jobs.

[creekorful]

@driesvints The children have been created and accumulated over time. The issue is not because of too many inserts, it's because eloquent is trying to set updated_at for too many models at the same time. The inserts here are just a way to recreate this use case, it's not the error.

Please consider opening the issue again.

[creekorful]

@driesvints I have updated the issue to make this more clear.

[driesvints]

@creekorful ah I see, sorry. In this case I wouldn't recommend using $touches if you have that much related records. If you find an optimization for this you're free to PR something 👍

[creekorful]

@driesvints what do you think about chunking the ids if it reaches a defined threshold?

[driesvints]

@creekorful could maybe do that yeah. But wondering how you're going to determine that?

[creekorful]

There are some known limits:

• MySQL: 65,535 (see: https://github.com/mysql/mysql-server/blob/3290a66c89eb1625a7058e0ef732432b6952b435/sql/sql_prepare.cc#L1505)
• PostgreSQL: Looks like there's no real limit (see: https://github.com/postgres/postgres/blob/998d060f3db79c6918cb4a547695be150833f9a4/src/backend/parser/parse_expr.c#L1080)
• SQLite: Looks like there are no real limits other than the classic SQLITE_LIMIT_LENGTH / SQLITE_MAX_LENGTH (see: https://www.sqlite.org/c3ref/bind_blob.html)

So I guess doing a per driver limits system would be the best (Is that possible?) otherwise I would fallback to a magic number.

[creekorful]

Otherwise maybe this settings may be configured inside config/database.php and be set to null by default to not change the current behaviour. What do you think @driesvints ?

[driesvints]

I still think the easiest way is to not use $touches heh... just doesn't feels the correct solution if you're working with that many records. I think it's best we leave things be. But you're free to attempt a PR either at the code or docs to see if Taylor would accept it.

[creekorful]

I guess you're right, we have a kind of special use case here. But I would expect $touches to work as expected so amending to the doc that it shouldn't be used with high volumes relationship would be needed IMO. I can take care of that!

I guess I can always work with Event or Observer to manually update the children updated_at in bulk mode if I really need it.

[creekorful]

If anyone happen to face the same issue one day here's how I solved it:

class ParentSubscriber
{
    public function handleParentSaved(ParentSaved $event)
    {
        // Manually bulk update the children updated_at using chunking
        // We can't use $touches here because we have too many children linked to the parent, and therefore
        // we hit the MySQL placeholder limits (65,535)
        // See: https://github.com/laravel/framework/issues/39259
        $event->parent->children()->chunk(1000, function (Collection $children) {
            Child::whereIn('id', $children->pluck('id'))->update(['updated_at' => Carbon::now()]);
        });
    }
}

class Parent extends Model
{
    protected $dispatchesEvents = [
        'saved' => ParentSaved::class,
    ];
}

@driesvints here's how I've worked around the limitation.

---

On a side note: I'm a bit sad that Taylor has cancelled my MR for the docs (see: laravel/docs#7373) without taking time to consider this issue. IMO this limitation should definitely be documented somewhere since users would expect $touches to work out of the box, but meh.

Have a nice one!