[DDC-61] OneToOne relation with an entity using Class Table Inheritance fails Created: 27/Oct/09  Updated: 04/Nov/09  Resolved: 03/Nov/09

Status: Closed
Project: Doctrine 2 - ORM
Component/s: ORM
Affects Version/s: None
Fix Version/s: 2.0-ALPHA3
Security Level: All

Type: Bug Priority: Critical
Reporter: Ville Väänänen Assignee: Roman S. Borschel
Resolution: Fixed Votes: 0
Labels: None

Attachments: File cti_test.diff     File cti_test.diff     File StandardEntityPersister.php.diff    

 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.



 Comments   
Comment by Ville Väänänen [ 27/Oct/09 ]

The situation is actually worse if the relation is eager. In that case not only is the classname incorrectly resolved, but if the root-entity is an abstract class, the execution ends up in a fatal error.

It would seem to me that there are two ways to deal with the described problem:

1. join all the tables that contain the discriminator-fields, so that the correct subclasses can be inferred
OR
2. change the proxy pattern to a variant which is used in the FLOW3 framework, where there's only one proxy-class

Comment by Ville Väänänen [ 27/Oct/09 ]

A fix to the patch. Use the newer attachment.

Comment by Ville Väänänen [ 02/Nov/09 ]

I have patches for both of the options above. Should this be a configuration option? As it stands now, OneToMany and ManyToMany relationships enjoy the advantage of having the Collection in between the target and source entities. We could even use a hybrid approach, where a general proxy-object is created only when the object type cannot be inferred without joins.

Comment by Roman S. Borschel [ 02/Nov/09 ]

It should of course be Nr. 1. The discriminator column is always stored in the topmost (root) table and tables of base classes must be joined, always. If this is not the case this is a bug. Can you show your patch?

Comment by Ville Väänänen [ 03/Nov/09 ]

It would seem to me that this would cause an inconsistency in how similar associations with different kinds of entities are treated.

From DDC-38: But inverse sides of one-one associations can never be lazy. Now going with option 1 would mean that in case the association is with an entity using class-table inheritance, these lazy associations would be allowed (because if we're joining the base-table anyway, there's no reason to forbid this) . This begs the question that if we're ready to do the joins in this case, should we reconsider doing them also to find out about target-existency when dealing with other types of entities?

Comment by Ville Väänänen [ 03/Nov/09 ]

Describing the changes StandardEntityPersister.php.diff:

  • _getSelectEntitiesSql
    • For every inverse OneToOne relation join the target-table/base-table and find out the existence of the target-entity and the value of the discriminator column if one should exist. These values are given the names of the sourceFieldNames in the result array
  • _createEntity
    • Pass the existence and discrimnator column values to unitOfWork->createEntity in the hints['associations'] array. This part is only speculative.

This selects the existence/discrimator-column information when an entity is queried using EntityManager->find, it's doesn't help if one is using a DQL query.

Comment by Roman S. Borschel [ 03/Nov/09 ]

I understand now what the problem is. Its tricky. Your testcase however has a wrong mapping. The way you mapped it, any event that is fetched of a company will automatically be the main event In this case it would be correct to put a join column on the organization (main_event_id). But I understand the problem and will create a separate new test for it.

It will probably be treated similar to inverse sides of one-one. That means, when a one-one associations references a target entity that is a base type (not an outermost type, that means there are still possible subtypes), whether the association is the owning side or not it must be eagerly fetched.

Comment by Ville Väänänen [ 04/Nov/09 ]

Okay, so not allowing lazy-loading in this case at all. Why is this? As I stated earlier there are at least to ways to solve this without resorting to eager-loading. If using class-table inheritance means forgetting about lazy-load, to me at least it's a huge disadvantage and would be a deal breaker in using this inheritance-type. The problem with eager-load is not the first fetch that follows, but the uncertainty of how many fetches are going to follow in total. If the target-entity is the tip of a huge object-hierarchy full of entities using class-table inheritance, the current strategy is unusable. Having a couple of extra tables joined is nothing compared to the snowball that eager-fetching might get rolling.

Comment by Roman S. Borschel [ 04/Nov/09 ]

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.

Comment by Roman S. Borschel [ 04/Nov/09 ]

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.

Comment by Ville Väänänen [ 04/Nov/09 ]

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?

Comment by Roman S. Borschel [ 04/Nov/09 ]

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.

Comment by Ville Väänänen [ 04/Nov/09 ]

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

Generated at Tue Sep 02 06:45:52 UTC 2014 using JIRA 6.2.3#6260-sha1:63ef1d6dac3f4f4d7db4c1effd405ba38ccdc558.