using afterQuery on a relationship does not end up retaining any set custom keys

[Jade-GG]

Laravel Version

12.36.1

PHP Version

8.3.24

Database Driver & Version

No response

Description

When you're using afterQuery on an Eloquent relationship, with something like:

return $this->hasMany(Product::class)
    ->afterQuery(fn ($result) => $result->keyBy('product_id'));

The custom keys will be thrown away and replaced with standard incrementing keys (i.e. it turns back into a standard array). After a lot of searching I found out that this is seemingly caused by the buildDictionary function used in every relationship, which ignores custom keys completely and re-keys every array it comes across.

For most relationships a fix for this would be fairly straightforward, for example BelongsToMany could be changed like this:

-        foreach ($results as $result) {
+        foreach ($results as $key => $result) {
            $value = $this->getDictionaryKey($result->{$this->accessor}->{$this->foreignPivotKey});

-            $dictionary[$value][] = $result;
+            $dictionary[$value][$key] = $result;
        }

For HasOneOrMany it's a bit more involved, but still possible.

However, I do not know whether there would be any relevant knock-on effects from changing this behavior, which makes me uncertain whether it's a good idea to go through and fix all of them and create a pull request with that. Plus, there might be a better solution here that I don't know about.

Steps To Reproduce

1. Create two Eloquent models that relate to each other in some way (e.g. a hasMany relationship)
2. Relate them with an Eloquent relationship
3. Try to use ->afterQuery(...) on this relationship with a ->keyBy(...)
4. Dump what comes out of this relationship and observe that the custom keys you tried to add have been removed.

[macropay-solutions]

Please define custom keys? What do you mean by that?

[crynobone]

Hey there, thanks for reporting this issue.

We'll need more info and/or code to debug this further. Can you please create a repository with the command below, commit the code that reproduces the issue as one separate commit on the main/master branch and share the repository here?

Please make sure that you have the latest version of the Laravel installer in order to run this command. Please also make sure you have both Git & the GitHub CLI tool properly set up.

laravel new bug-report --github="--public"

Do not amend and create a separate commit with your custom changes. After you've posted the repository, we'll try to reproduce the issue.

Thanks!

[Jade-GG]

I've created a repository here: https://github.com/Jade-GG/laravel-bug-report-after-query

Here I've added a simple test relation using HasMany:

public function tests(): HasMany
{
    return $this
        ->hasMany(Test::class)
        ->afterQuery(fn($data) => $data->keyBy('test_number'));
}

---

To reproduce the bug:

php artisan migrate
php artisan db:seed

Enter the tinker console:

php artisan tinker

Check that the afterQuery works correctly when retrieving the relation I've created directly:

User::first()->tests

This will return a collection keyed by test_number. Then, note the missing keys when eager loading the model:

User::with('tests')->first()->tests

[rodrigopedra]

@Jade-GG what is the use case for this?

In your proposed fix, what happens if there are colliding keys?

For example, in your seeder, instead of generating 10 test records with test_number ranging from 1 to 1000, generate 1000 records with test_number ranging from 1 to 10.

for ($i = 0; $i < 1000; $i++) {
    Test::create([
        'user_id' => $user->id,
        'test_number' => rand(1, 10),
    ]);
}

The Builder@afterQuery call will key the resulting collection by test_number reducing it to 10 records. And discarding the other 990 results.

I don't think this would make a good default.

[Jade-GG]

@Jade-GG what is the use case for this?

In your proposed fix, what happens if there are colliding keys?

What do you mean? My proposed fix is making eager loading here work the same as lazy loading.

I don't think this would make a good default.

Of course not, because that is not what I'm suggesting at all.

It's just a piece of example code that I created solely to show the bug I'm reporting. My example here only exists to show the inconsistency between eager loading and lazy loading relationships when you try to use afterQuery on keys specifically.

My proposed (partial) fix would only fix that part, retaining the keys that came directly from the query. If you then choose to not add the afterQuery, the behavior would be the same as before.

I'm not sure where the confusion is here.

[macropay-solutions]

@Jade-GG the confusion is generated by the fact that your keys may not always be the primary key or a unique key.