Skip to content

Nested ids remapped even if renameEmbeddedIdField is false #3351

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
NickHuijgen opened this issue Apr 14, 2025 · 4 comments
Closed

Nested ids remapped even if renameEmbeddedIdField is false #3351

NickHuijgen opened this issue Apr 14, 2025 · 4 comments

Comments

@NickHuijgen
Copy link

  • Laravel-mongodb Version: 5.3
  • PHP Version: 8.3
  • Database Driver & Version: -

Description:

I'm trying to query based on the id field of a nested array document. When attempting to query the id field is remapped to _id regardless of the renameEmbeddedIdField setting.

If I modify my query to include another where() clause, the id field is not remapped to _id
Model::query()->where('id', 1)->where('nested.id', 1)->get() -> does not remap nested.id
Model::query()->where('nested.id', 1)->get() -> remaps nested.id

Steps to reproduce

  1. 'rename_embedded_id_field' => false or DB::connection('mongodb')->setRenameEmbeddedIdField(false);
  2. Model::query()->where('nested.id', 1)->get();

Expected behaviour

nested.id should not be remapped to nested._id in the query because renameEmbeddedIdField is set to false

Actual behaviour

In the query that is executed nested.id is remapped to nested._id in the query

@GromNaN
Copy link
Member

GromNaN commented Apr 14, 2025

I'm surprised by your issue because there is a test that covers this scenario:

public function testRenameEmbeddedIdFieldCanBeDisabled()
{
$builder = $this->getBuilder(false);
$this->assertFalse($builder->getConnection()->getRenameEmbeddedIdField());
$mql = $builder
->where('id', '=', 10)
->where('nested.id', '=', 20)
->where('embed', '=', ['id' => 30])
->toMql();
$this->assertEquals([
'find' => [
[
'$and' => [
['_id' => 10],
['nested.id' => 20],
['embed' => ['id' => 30]],
],
],
['typeMap' => ['root' => 'object', 'document' => 'array']],
],
], $mql);
}

Perhaps the problem only arises when the query is created from a Model.

@NickHuijgen
Copy link
Author

I believe that test only covers this:
Model::query()->where('id', 1)->where('nested.id', 1)->get();

And not this:
Model::query()->where('nested.id', 1)->get();

To make sure this wasn't an issue in my project I cloned this project and added a test that fails.

public function testRenameEmbeddedIdFieldCanBeDisabledWithSingleWhere()
{
    $builder = $this->getBuilder(false);
    $this->assertFalse($builder->getConnection()->getRenameEmbeddedIdField());

    $mql = $builder
        ->where('nested.id', '=', 20)
        ->toMql();

    $this->assertEquals([
        'find' => [
            [
                'nested.id' => 20,
            ],
            ['typeMap' => ['root' => 'object', 'document' => 'array']],
        ],
    ], $mql);
}

Output:

Image

@NickHuijgen
Copy link
Author

Correct me if I'm wrong but I believe this can be fixed by updating this if statement in src/Query/Builder::aliasIdForQuery():

if (str_ends_with($key, '.id') && ($root || $this->connection->getRenameEmbeddedIdField())) {
...
}

To this:

if (str_ends_with($key, '.id') && $this->connection->getRenameEmbeddedIdField()) {
...
}

Because if the key includes a .id it is always a nested document.

@GromNaN
Copy link
Member

GromNaN commented Apr 15, 2025

Thank you @NickHuijgen for reporting and investigating this issue. The fix is merged and released in 5.3.1.

@GromNaN GromNaN closed this as completed Apr 15, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants