doctrine / mongodb-odm

The Official PHP MongoDB ORM/ODM
https://www.doctrine-project.org/projects/doctrine-mongodb-odm/en/latest/
MIT License
1.09k stars 502 forks source link

Support to non-primitive types in queries #1179

Open marcospassos opened 9 years ago

marcospassos commented 9 years ago

In addition to #1178, would be great to support the use of non-primitive values in queries, exactly as ORM does. Auto-converting parameters to database values makes things look more consistent and avoid a lot boilerplate code for casting non-primitive types into primitive ones.

Currently, the execution of the following snippet results in an exception:

public function findById(UserId $id)
{
        return $this->createQueryBuilder()
            ->find()
            ->field('id')
            ->equals($id)
            ->execute();
}

Error:

zero-length keys are not allowed, did you use $ with double quotes?

It looks weird because the field is mapped to a custom type, so why the parameter is not being converted to the specified value?

The workaround is casting the value whenever you want to use it, what exposes some implementation details that, IMHO, should be handled by the ODM:

public function findById(UserId $id)
{
        return $this->createQueryBuilder()
            ->find()
            ->field('id')
            ->equals((string) $id)
            ->execute();
}

Is there any possibilities to schedule it to be in 1.x release? If so, I'll try to provide a PR as well.

malarzm commented 9 years ago

For this particular case using $qb->references($user) would be more adequate (however that requires having User object)

marcospassos commented 9 years ago

I just updated the examples to make it more explicit.

malarzm commented 9 years ago

Yes, I agree that now it makes more sense, however I'm not sure if this can be easily supported - it's easy in simple cases like this, but you might not know mapping of deeply embedded documents as it may vary on discriminator values or even have different types of id if documents share the same collection (single collection inheritance).

marcospassos commented 9 years ago

Another use case:

Multiple users can share the same SupportId (type support_id):

public function findBySupportId(SupportId $supportId)
{
     return $this->findBy(['supportId' => $supportId]);
}

Currently:

public function findBySupportId(SupportId $supportId)
{
     return $this->findBy(['supportId' => (string) $supportId]);
}
dkarlovi commented 6 years ago

If it's any consolation, ORM has exactly the same problem, https://github.com/doctrine/doctrine2/issues/4632 Both of which came up because using UUIDs as IDs.

In both cases, a fix might be to use the field's type's convertToDatabaseValue to normalize the value before binding?

juliusstoerrle commented 5 years ago

What is the reason fot not converting normal fields in DocumentPersister::prepareQueryElement called in DocumentPersister::prepareQueryOrNewObj() if there is an entry in the ClassMetadata?

If my criteria contains the name of a property/field of a document I would expect that the ORM will automatically convert the type.

As this is a BC Break, is there any way we could change this for the 2.0 release?

alcaeus commented 5 years ago

As this is a BC Break, is there any way we could change this for the 2.0 release?

Short answer: no. Long answer below.

So, what we currently do is that for the built-in types we convert PHP values to their appropriate MongoDB counterparts (e.g., an ObjectID instance is just represented as string for easier use, but needs to be converted whenever we work with it in the database). To do this, we fetch mapping information for the field in question, then convert it.

For complex fields, this isn't possible yet. I would certainly like to do it when types are involved, but we have to first consider making changes to the type system as it doesn't support this as of now. Since we want the core things between ORM and MongoDB ODM to be very similar, this isn't something I want to just tackle without prior coordination, especially not when 2.0 is already in Beta.

Since this is a BC break, we won't be forcing the behaviour upon someone in 2.0, but we can certainly allow users to enable it if they wish. What I'm currently considering is a strict mode for query builder (which will be turned on by default in 3.0) which enforces type safety for fields where we have mapping information. For unmapped fields, we will infer the type from the value given and apply the correct type conversion (if applicable). However, this is just a rough idea and I haven't started sketching out anything. I appreciate your feedback on the above idea, but as-is this isn't something we're doing in 2.0.

juliusstoerrle commented 5 years ago

Thanks for your long reply! Thats ok, I'm totally on your side in terms of consistency across doctrine. I will find a temporary solution to get my project forward.

Could you tell me what mappings you count as complex fields? References, Embedding?

The strict mode sounds good. If there is anything I can do to support with this, please let me know.

alcaeus commented 5 years ago

Complex fields => everything other than a scalar value, so references, embedded documents, value objects, etc.