Details

    • Type: Improvement Improvement
    • Status: Open
    • Priority: Minor Minor
    • Resolution: Unresolved
    • Affects Version/s: None
    • Fix Version/s: None
    • Component/s: DQL
    • Security Level: All
    • Labels:

      Description

      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 ~ :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!

        Activity

        Hide
        Guilherme Blanco added a comment -

        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)  }
        
        Show
        Guilherme Blanco added a comment - 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) }
        Hide
        Christophe Coevoet added a comment -

        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.

        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.

        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.

        Show
        Christophe Coevoet added a comment - 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. 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. 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.
        Hide
        Erik A. added a comment -

        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.

        Show
        Erik A. added a comment - 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.
        Hide
        Christophe Coevoet added a comment -

        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)

        Show
        Christophe Coevoet added a comment - 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)
        Hide
        Guilherme Blanco added a comment -

        Christophe Coevoet 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).

        Show
        Guilherme Blanco added a comment - Christophe Coevoet 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).

          People

          • Assignee:
            Benjamin Eberlei
            Reporter:
            Erik A.
          • Votes:
            1 Vote for this issue
            Watchers:
            4 Start watching this issue

            Dates

            • Created:
              Updated: