laravel / framework

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

MySQL zero fill makes eager loading impossible #53500

Closed JapSeyz closed 2 weeks ago

JapSeyz commented 2 weeks ago

Laravel Version

v10.48.23

PHP Version

8.3.13

Database Driver & Version

11.5.2-MariaDB-ubu2204-log

Description

We have an ID column defined as bigint(13) unsigned zerofill because it stores 13-digit EAN barcodes.

When eager loading this table, it provides the follow, fully functional query:

select * from `table`.`barcodes` where `table`.`barcodes`.`id` in (978001, 978002, 978003)

I have validated that this fetches the correct items, but they are somehow never mapped onto the model they're being eagerloaded onto. Presumably because ->id is gonna be prefixed by 0s to pad it to 13 digits, which then makes the barcode_id on the parent-model != id on the eagerloaded model

Steps To Reproduce

Try to eager load a model where the id column is zerofill and the id is less than what it will be zero filled to.

JapSeyz commented 2 weeks ago

As a workaround, adding a getter fixes the issue, as it strips the prefixed 0s

public function getIdAttribute()
{
    return (int) $this->attributes['id'];
}
macropay-solutions commented 2 weeks ago

Did you try to put in your model:

protected $keyType = 'string';
JapSeyz commented 2 weeks ago

No, but I doubt that would solve it. That wouldn't remove the extra zero filled 0s, right?

Edit: I tried it, and it does not work.

macropay-solutions commented 2 weeks ago

https://stackoverflow.com/questions/5256469/what-is-the-benefit-of-zerofill-in-mysql Using ZEROFILL and a display width has no effect on how the data is stored. It affects only how it is displayed, so the string type does not solve it.

JapSeyz commented 2 weeks ago

I know, but data is returned prefixed by 0s to the defined length. in this case 13.

It means when we look up id = 9876, the id returned will be 0000000009876 (which is correct). The id will then no longer match, when laravel tries to attach the model to the parent.

The issue is not within the database, but how Laravel handles zero fill. I am not sure it is a bug, but it was something that took a while to track down, so creating an issue for it, at least might help someone find it in the future.

I am not sure what your comment is trying to say.

macropay-solutions commented 2 weeks ago

It explains why the string type does not solve it, just like you pointed out. A solution could be casting the pk to the $keyType type when eager loading.

Update. Casting the foreign key not the pk because the pk is already casted from 0001 to 1, no?

Update 2: In here could be the casting(example for belongs to):


    public function match(array $models, Collection $results, $relation)
    {
        $foreign = $this->foreignKey;

        $owner = $this->ownerKey;

        // First we will get to build a dictionary of the child models by their primary
        // key of the relationship, then we can easily match the children back onto
        // the parents using that dictionary and the primary key of the children.
        $dictionary = [];

        foreach ($results as $result) {
            $attribute = $this->getDictionaryKey($result->getAttribute($owner));

            $dictionary[$attribute] = $result;
        }

        // Once we have the dictionary constructed, we can loop through all the parents
        // and match back onto their children using these keys of the dictionary and
        // the primary key of the children to map them onto the correct instances.
        foreach ($models as $model) {
            $attribute = $this->getDictionaryKey($model->{$foreign});

            if (isset($dictionary[$attribute])) {
                $model->setRelation($relation, $dictionary[$attribute]);
            }
        }

        return $models;
    }

    /**
     * Get a dictionary key attribute - casting it to a string if necessary.
     *
     * @param  mixed  $attribute
     * @return mixed
     *
     * @throws \Doctrine\Instantiator\Exception\InvalidArgumentException
     */
    protected function getDictionaryKey($attribute)
    {
        if (is_object($attribute)) {
            if (method_exists($attribute, '__toString')) {
                return $attribute->__toString();
            }

            if ($attribute instanceof UnitEnum) {
                return $attribute instanceof BackedEnum ? $attribute->value : $attribute->name;
            }

            throw new InvalidArgumentException('Model attribute value is an object but does not have a __toString method.');
        }

        return $attribute;
    }

Update 3: But the foreign key does not have a type, only the pk has one so... your solution is the solution.