Missing bindings on eager loads when using string primary keys

[a-drew]

I've found an edge case for users with string primary keys, where the eager loads won't behave as expected.
When using auto-increment keys, you don't need bindings at all but when your models are using UUIDs the eager load won't build the correct query.

I traced it down to this method:

laravel-many-to-many-self/src/BelongsToManySelf.php

Lines 144 to 163 in 797c689

	/**
	* Set the constraints for an eager load of the relation.
	*
	* @param array $models
	* @return void
	*/
	public function addEagerConstraints(array $models)
	{
	$whereIn = $this->whereInMethod($this->parent, $this->parentKey);
	$this->directJoinWhere->{$whereIn}(
	$this->getQualifiedForeignPivotKeyName(),
	$keys = $this->getKeys($models, $this->parentKey)
	);
	$this->inverseJoinWhere->{$whereIn}(
	$this->getQualifiedRelatedPivotKeyName(),
	$keys
	);
	}

I believe using $this->addBindings($keys) twice on lines 158 / 162, for both whereIn constraints, should solve this.

[kingmaker-agm]

Hi @a-drew,

I believe using $this->addBindings($keys) twice on lines 158 / 162, for both whereIn constraints, should solve this.

Does this solved the Issue in your case?

previously while development, I ran into lots of problems when bindings were manually pushed into the Queries.

[a-drew]

Hey @kingmaker-agm, I've setup an override in one of my project and it solved the issue, but I admit I haven't experimented with particularly complex queries just the following:

        // related is using the custom BelongsToManySelf code below
        $article->loadMissing('related:category_id,cover,title,slug,excerpt');
        $article->loadMissing('related.category:id,title');

BelongsToManySelf Relation, used by Article::related()

<?php

namespace App\Models\Relations;

use Kingmaker\Illuminate\Eloquent\Relations\BelongsToManySelf as BaseBelongsToManySelf;

class BelongsToManySelf extends BaseBelongsToManySelf
{

    /**
     * This override exists only to fix the missing binding issue when using string key types
     * Set the constraints for an eager load of the relation.
     *
     * @param  array  $models
     * @return void
     */
    public function addEagerConstraints(array $models)
    {
        $whereIn = $this->whereInMethod($this->parent, $this->parentKey);

        $this->directJoinWhere->{$whereIn}(
            $this->getQualifiedForeignPivotKeyName(),
            $keys = $this->getKeys($models, $this->parentKey)
        );
        $this->addBinding($keys);

        $this->inverseJoinWhere->{$whereIn}(
            $this->getQualifiedRelatedPivotKeyName(),
            $keys
        );
        $this->addBinding($keys);
    }
}

[kingmaker-agm]

Hi @a-drew, Can you explain your code a bit (DB Schema with Column types, Eloquent Relations & the Queries)?

your input can help me to improve the package. I will try to look into the Issue on my free time.

[a-drew]

hey @kingmaker-agm, first off happy new year 🎉🎉

Here's a basic example of what I was doing. 3 tables and only 2 models: Articles and Categories. Articles have a related relation using the modified belongsToSelf trait version I supplied above. In short, I want to display a list of related articles at the bottom of every article post. Instead of duplicating the entries in the pivot table I'm using your awesome package.

Schema

Models

Article.php

<?php

namespace App\Models;

use App\Models\Traits\HasBelongsToManySelf;

class Article extends Model
{
    use HasBelongsToManySelf;
    
        /**
     * The "type" of the primary key ID.
     *
     * @var string
     */
    protected $keyType = 'string';

    /**
     * Indicates if the IDs are auto-incrementing.
     *
     * @var bool
     */
    public $incrementing = false;

    protected $fillable = ['title', 'content', 'status'];

    public function category() {
        return $this->belongsTo(Category::class);
    }

    public function related() {
        return $this->belongsToManySelf('related_articles', 'article_id', 'related_id');
    }
}

Category.php

<?php

namespace App\Models;

class Category extends Model
{
   ...

    public function articles()
    {
        return $this->hasMany(Article::class);
    }
}

Example Query Usage:

Example queries would look like: Article::where('slug', 'x')->first() or Article::latest()->first(). And these would load any $article->related articles just fine but at the cost of extra queries. In order to reduce that on bigger queries like

Article::where('status', 'published')->get()

I would use eager-loading, turning said query into:

Article::where('status', 'published')->with('related')->get()

// or if I want to limit to only specific columns and even nested relations

Article::where('status', 'published')->with([
   'related:category_id,cover,title,slug,excerpt', 
   'related.category:id,title'
])->get()

This is where we start running into issues. We reach the addEagerConstraints() method. It works fine when the primary key is an int, no binding into the 2 join clauses is necessary. But when switching to string keys, the whereIn constraints are applied but the necessary key bindings are not mapped and the end query is wrong (see examples below). I found its linked to Relation::whereInMethod() method from Laravel Eloquent source.

    /**
     * Get the name of the "where in" method for eager loading.
     *
     * @param  \Illuminate\Database\Eloquent\Model  $model
     * @param  string  $key
     * @return string
     */
    protected function whereInMethod(Model $model, $key)
    {
        return $model->getKeyName() === last(explode('.', $key))
                    && in_array($model->getKeyType(), ['int', 'integer'])
                        ? 'whereIntegerInRaw'
                        : 'whereIn';
    }

When the key type is a string, the normal whereIn() method is used and sets the bindings directly on the join clauses. But these bindings are never passed to the parent query and therefore are not used in performJoin(). Could be something to look at upstream in eloquent's code, seems like pulling bindings of nested joins would be expected behaviour.

Output Of Eagerloaded Queries:

here's some tinker output where I dump the whole query and with its bindings before exiting out of my version of addEagerConstraints():

❯ without addBindings, using integer key type on the models
(uuids get parsed into ints: '0d582b41-05f8-4220-b559-4a9f7410869c' -> 0, '875a923d-...' -> 875)

>>> Article::with('related')->first()
[!] Aliasing 'Article' to 'App\Models\Article' for this Tinker session.
"select * from `articles` inner join `related_articles` on (`articles`.`id` = `related_articles`.`related_id` and `related_articles`.`article_id ` in (0)) or (`articles`.`id` = `related_articles`.`article_id` and `related_articles`.`related_id` in (0))"
...
DOESN'T ERROR OUT BUT RESULTS ARE INCORRECT

❯ without addBindings() calls but setting key type to string

>>> Article::with('related')->first()
[!] Aliasing 'Article' to 'App\Models\Article' for this Tinker session.
"select * from `articles` inner join `related_articles` on (`articles`.`id` = `related_articles`.`related_id` and `related_articles`.`article_id` in (?)) or (`articles`.`id` = `related_articles`.`article_id` and `related_articles`.`related_id` in (?))"
Illuminate\Database\QueryException with message 'SQLSTATE[HY000]: General error: 2031 No data supplied for parameters in prepared statement (SQL: select `articles`.*, `related_articles`.`article_id` as `pivot_article_id`, `related_articles`.`related_id` as `pivot_related_id` from `articles` inner join `related_articles` on (`articles`.`id` = `related_articles`.`related_id` and `related_articles`.`article_id` in (?)) or (`articles`.`id` = `related_articles`.`article_id` and `related_articles`.`related_id` in (?)))'

❯ key type string and one addBindings() call

>>> Article::with('related')->first()
[!] Aliasing 'Article' to 'App\Models\Article' for this Tinker session.
"select * from `articles` inner join `related_articles` on (`articles`.`id` = `related_articles`.`related_id` and `related_articles`.`article_id` in ('0d582b41-05f8-4220-b559-4a9f7410869c')) or (`articles`.`id` = `related_articles`.`article_id` and `related_articles`.`related_id` in (?))"
Illuminate\Database\QueryException with message 'SQLSTATE[HY093]: Invalid parameter number (SQL: select `articles`.*, `related_articles`.`article_id` as `pivot_article_id`, `related_articles`.`related_id` as `pivot_related_id` from `articles` inner join `related_articles` on (`articles`.`id` = `related_articles`.`related_id` and `related_articles`.`article_id` in (0d582b41-05f8-4220-b559-4a9f7410869c)) or (`articles`.`id` = `related_articles`.`article_id` and `related_articles`.`related_id` in (?)))'

❯ key type string with both addBindings() calls

>>> Article::with('related')->first()
[!] Aliasing 'Article' to 'App\Models\Article' for this Tinker session.
"select * from `articles` inner join `related_articles` on (`articles`.`id` = `related_articles`.`related_id` and `related_articles`.`article_id ` in ('0d582b41-05f8-4220-b559-4a9f7410869c')) or (`articles`.`id` = `related_articles`.`article_id` and `related_articles`.`related_id` in ('0d582b41-05f8-4220-b559-4a9f7410869c'))"
...
SUCCESS!