doctrine / orm

Doctrine Object Relational Mapper (ORM)
https://www.doctrine-project.org/projects/orm.html
MIT License
9.93k stars 2.52k forks source link

DDC-3154: Conditions with Value Objects #3933

Open doctrinebot opened 10 years ago

doctrinebot commented 10 years ago

Jira issue originally created by user erik-am:

I'm quite enthousiastic about Embeddables being added to Doctrine, but it's a pity that true Value Objects, which are compared by their properties, are not supported yet.

Given a Value Object Address with properties street and house number that you can instantiate with new Address("High Street", 1), and a User class that has an Address as Embeddable.

Then, the following is now supported: DQL1:

SELECT u FROM User u WHERE (u.address.street = :street AND u.address.nr = :number)
    with { 'street' => 'High Street', 'number' => 1 }

Disadvantage: you should know the internal properties when writing your query. That's not how Value Objects usually are compared.

Instead, I expect to be able to do this: DQL2:

SELECT u FROM User u WHERE (u.address = :address)
    with { 'address' => new Address("High Street", 1)  }

Internally, DQL2 could simply be transformed to the equivalent DQL1 by replacing the condition with conditions for each internal property. The advantage is that the one writing the query does not have to refer to the internal fields; the transformation is hidden.

A complicating factor is that Value Objects are Embeddables, but not every Embeddable is a Value Object. So there is always the question if objects need to be compared by reference or by their properties.

So, perhaps it's an idea to introduce a special operator for comparing objects by their value to make the distinction explicit? Like so: DQL3:

SELECT u FROM User u WHERE (u.address </sub> :address)
    with { 'address' => new Address("High Street", 1)  }.

I created a pull request that contains an idea how the same concept (the ~ operator) might be applied to criterias on in-memory collections.

Just some thoughts and ideas. I'd love to hear some discussion on this as I think it would make Doctrine really powerful in supporting rich, expressive domain models. It would be great if both in-memory collections and DQL supported this!

doctrinebot commented 10 years ago

Comment created by @guilhermeblanco:

It should be the same operator, not a new one. So this is the intended and desired behavior:

SELECT u FROM User u WHERE (u.address = :address)
    with { 'address' => new Address("High Street", 1)  }
doctrinebot commented 10 years ago

Comment created by stof:

This would require to change the DQL to SQL conversion based on the fact that u.address is the path to an embeddable. It might impact performances Using a separate operator would at least allow to know that it needs a special handling, without having to do complex changes to all places using the = operator.

{quote} A complicating factor is that Value Objects are Embeddables, but not every Embeddable is a Value Object. So there is always the question if objects need to be compared by reference or by their properties. {quote}

Embeddables cannot be compared by reference. They don't have an identity in the database. The only thing we have to compare them are their properties.

doctrinebot commented 10 years ago

Comment created by erik-am:

True, if we look at the database level, we can only compare by reference. However, if you look beyond ORM and also to the Collections package, then if you want to do a matching on a collection of entities that have an Embeddable by using a Criteria on that Embeddable, then you do have both options (see my referenced pull request). Then two operators might come in handy, so that the additional operator can be introduced to both ORM as well as Collections.

If we would go for one operator (=), then I think the Criteria in Collection also needs to be changed so that it performs a loose comparison on objects and a strict comparison only on scalars. Perhaps that is already out of the scope of the current issue, but either way it would be preferrable to have a consistent solution.

doctrinebot commented 10 years ago

Comment created by stof:

No, saying that we can only compare by reference is wrong. We _cannot_ compare by reference (there is no way to reference them).

And talking about the needs of the criteria here is irrelevant, as this discussion is about building the DQL language, not about building the Criteria API. The criteria API can still have a separate operator to deal with value object even if the DQL uses = to compare embeddables. (btw, changing the Criteria comparison to loose on objects would break the comparison of relations, so it is totally impossible as it would be a BC break)

doctrinebot commented 10 years ago

Comment created by @guilhermeblanco:

[~stof] not a performance impact since DQL => SQL is cached. Adding a new operator resolution requires bigger efforts and I'm pretty sure it'll be slower than converting u.address to multiple clauses (we do it already with composite identifiers).

ddegasperi commented 3 months ago

It seems that developers keep falling into the same trap (including me) - see also ticket #8164 and #10898

In cases where only one attribute is used, the behavior could be solved by using custom types, but not in cases where multiple attributes are present on the value object.

With regard to ticket #8164, this feature would also align the behavior of PersistentCollection with that of the in-memory search.

I would really like to see this improvement given more priority as it would visibly add value to the ORM