Doctrine 2 - ORM
  1. Doctrine 2 - ORM
  2. DDC-61

OneToOne relation with an entity using Class Table Inheritance fails

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Critical Critical
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 2.0-ALPHA3
    • Component/s: ORM
    • Security Level: All
    • Labels:
      None

      Description

      The bug occurs when there's a lazy bidirectional one-to-one relation where the inverse side only specifies the root entity. In this case, the final type of the target-entity is not resolved correctly. See the attached file for a failing test case.

      1. cti_test.diff
        2 kB
        Ville Väänänen
      2. cti_test.diff
        2 kB
        Ville Väänänen
      3. StandardEntityPersister.php.diff
        6 kB
        Ville Väänänen

        Activity

        Hide
        Roman S. Borschel added a comment -

        Well, option Nr.1 is too complicated and can lead to lots of new issues. You can not always join arbitrary tables, not when it comes to DQL where it alters the overall result. The situation is much more complex than it might look to you right now.

        Nr.2 is no option at all because such a proxy approach does not even hold for instanceof checks. An absolute no-go.

        What it does now is the best thing from my point of view. If performance becomes an issue you can always improve upon that by eager fetching with DQL. Also, you did not yet understand how it works. The eager load does not happen always with class table inheritance, only when the target entity is a base type. If it is a subtype that does not have any further subtypes, then a proxy can be used without fear. Same goes for single table inheritance.

        Show
        Roman S. Borschel added a comment - Well, option Nr.1 is too complicated and can lead to lots of new issues. You can not always join arbitrary tables, not when it comes to DQL where it alters the overall result. The situation is much more complex than it might look to you right now. Nr.2 is no option at all because such a proxy approach does not even hold for instanceof checks. An absolute no-go. What it does now is the best thing from my point of view. If performance becomes an issue you can always improve upon that by eager fetching with DQL. Also, you did not yet understand how it works. The eager load does not happen always with class table inheritance, only when the target entity is a base type. If it is a subtype that does not have any further subtypes, then a proxy can be used without fear. Same goes for single table inheritance.
        Hide
        Roman S. Borschel added a comment -

        If you still feel this is too bad open a new issue as "improvement" and move any old/new patches you have there. There would need to be much more tests for such a change. Also you did not mention whether your patch actually passes all existing unit tests.

        Show
        Roman S. Borschel added a comment - If you still feel this is too bad open a new issue as "improvement" and move any old/new patches you have there. There would need to be much more tests for such a change. Also you did not mention whether your patch actually passes all existing unit tests.
        Hide
        Ville Väänänen added a comment -

        Sure I understood how it works. But to me one of the biggest advantages of class-table inheritance is exactly that the parent doesn't have to know the final type., and so in the configuration the association is pointed to the base-type. Yes, I accept that the issue might be way too complicated in the DQL case.

        Option number two can be improved if the parent model that might have these general proxies invokes a load in it's getters. Exactly the way the generated proxies work now. Of course, this means that the models need to be aware of the possible proxies. It's not perfect, but maybe it could be an option?

        Show
        Ville Väänänen added a comment - Sure I understood how it works. But to me one of the biggest advantages of class-table inheritance is exactly that the parent doesn't have to know the final type., and so in the configuration the association is pointed to the base-type. Yes, I accept that the issue might be way too complicated in the DQL case. Option number two can be improved if the parent model that might have these general proxies invokes a load in it's getters. Exactly the way the generated proxies work now. Of course, this means that the models need to be aware of the possible proxies. It's not perfect, but maybe it could be an option?
        Hide
        Roman S. Borschel added a comment -

        Please open a new issue, I think this has potential as an improvement that can complement the current behavior.

        What I mean is: for normal find/load ... operations that are not triggered by DQL we can probably do the join and in the case of DQL the current behavior is used, if you want to avoid the extra query in that case its as simple as adding the join to DQL.

        So I think the approaches can be complementary.

        Show
        Roman S. Borschel added a comment - Please open a new issue, I think this has potential as an improvement that can complement the current behavior. What I mean is: for normal find/load ... operations that are not triggered by DQL we can probably do the join and in the case of DQL the current behavior is used, if you want to avoid the extra query in that case its as simple as adding the join to DQL. So I think the approaches can be complementary.
        Hide
        Ville Väänänen added a comment -

        Sure like the sound of that! I will be very happy to open this as an improvement. Thanks for your patience .

        Show
        Ville Väänänen added a comment - Sure like the sound of that! I will be very happy to open this as an improvement. Thanks for your patience .

          People

          • Assignee:
            Roman S. Borschel
            Reporter:
            Ville Väänänen
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: