Ayende @ Rahien

Hi!
My name is Ayende Rahien
Founder of Hibernating Rhinos LTD and RavenDB.
You can reach me by phone or email:

ayende@ayende.com

+972 52-548-6969

@

Posts: 5,947 | Comments: 44,541

filter by tags archive

ChallengeThe lazy loaded inheritance many to one association OR/M conundrum


Here is an interesting problem that pop up with any OR/M that supports inheritance and lazy loading.

Let us say that we have the following entity model:

image

Backed by the following data model:

image

As you can see, we map the Animal hierarchy to the Animals table, and we have a polymorphic association between Animal Lover and his/her animal. Where does the problem starts?

Well, let us say that we want to load the animal lover. We do that using the following SQL:

SELECT Name,
	 Animal,
	 Id
FROM AnimalLover
WHERE Id = 1 /* @p0 */

And now we have an animal lover instance:

var animalLover = GetAnimalLoverById(1);
var isDog = animalLover.Animal is Dog;
var isCat = animalLover.Animal is Cat;

Can you guess what would be the result of this code?

I’ll post the actual answer tomorrow…

More posts in "Challenge" series:

  1. (28 Apr 2015) What is the meaning of this change?
  2. (26 Sep 2013) Spot the bug
  3. (27 May 2013) The problem of locking down tasks…
  4. (17 Oct 2011) Minimum number of round trips
  5. (23 Aug 2011) Recent Comments with Future Posts
  6. (02 Aug 2011) Modifying execution approaches
  7. (29 Apr 2011) Stop the leaks
  8. (23 Dec 2010) This code should never hit production
  9. (17 Dec 2010) Your own ThreadLocal
  10. (03 Dec 2010) Querying relative information with RavenDB
  11. (29 Jun 2010) Find the bug
  12. (23 Jun 2010) Dynamically dynamic
  13. (28 Apr 2010) What killed the application?
  14. (19 Mar 2010) What does this code do?
  15. (04 Mar 2010) Robust enumeration over external code
  16. (16 Feb 2010) Premature optimization, and all of that…
  17. (12 Feb 2010) Efficient querying
  18. (10 Feb 2010) Find the resource leak
  19. (21 Oct 2009) Can you spot the bug?
  20. (18 Oct 2009) Why is this wrong?
  21. (17 Oct 2009) Write the check in comment
  22. (15 Sep 2009) NH Prof Exporting Reports
  23. (02 Sep 2009) The lazy loaded inheritance many to one association OR/M conundrum
  24. (01 Sep 2009) Why isn’t select broken?
  25. (06 Aug 2009) Find the bug fixes
  26. (26 May 2009) Find the bug
  27. (14 May 2009) multi threaded test failure
  28. (11 May 2009) The regex that doesn’t match
  29. (24 Mar 2009) probability based selection
  30. (13 Mar 2009) C# Rewriting
  31. (18 Feb 2009) write a self extracting program
  32. (04 Sep 2008) Don't stop with the first DSL abstraction
  33. (02 Aug 2008) What is the problem?
  34. (28 Jul 2008) What does this code do?
  35. (26 Jul 2008) Find the bug fix
  36. (05 Jul 2008) Find the deadlock
  37. (03 Jul 2008) Find the bug
  38. (02 Jul 2008) What is wrong with this code
  39. (05 Jun 2008) why did the tests fail?
  40. (27 May 2008) Striving for better syntax
  41. (13 Apr 2008) calling generics without the generic type
  42. (12 Apr 2008) The directory tree
  43. (24 Mar 2008) Find the version
  44. (21 Jan 2008) Strongly typing weakly typed code
  45. (28 Jun 2007) Windsor Null Object Dependency Facility

Comments

marco

2 false becouse the type it's a proxy that inherits directly from Animal

Frans Bouma

Can you guess what would be the result of this code?

depends on the O/R mapper you're using. If animalLover 1 has a dog, isDog is true for some O/R mappers, and otherwise it's false as Marco said.

Even though 'persistence ignorance' is thought to be used here, it's hardly 'ignorance', it's very tied to the underlying o/r mapper.

Tran Ngoc Van

I think both isDog and isCat are false.

Due to lazy loading, the Animal instance has to be a proxy, and thus cannot be Dog or Cat. At the time the Animal proxy is instantiated, we have not known its actual time.

Steve Powell

This has been the only real problem we have been having with nHibernate.

We have a parallel class hierarchy problem where we have Animal, Cat, Dog and AnimalHome, Kennel, CatBasket. Animal has a property LivesIn which is of type AnimalHome and Dog and Cat override this property with the 'new' keyword returning Kennel and CatBasket. If you try and go Cath.LivesIn you get a cast exception.

We have found a work around if anyone wants to know it..... but it aint pretty!

Steve Powell

oops. Should have been Cat.LivesIn not Cath.LivesIn. Not sure how the wife managed to creep into my comment there....

Anders Ivner

This is one of the few (only?) design choices that nHibernate does wrong, imho. Alternately you could:

  1. take the penalty of joining the associated table to determine the actual type of Animal when loading the AnimalLover.

  2. Let the AnimalLover lazy load the animal when accessing the Animal property.

Alas, nhibernate sacrifices correctness for performance.

Ayende Rahien

Anders,

Liskov would be rolling in her grave, if she had one

Anders Ivner

Not all valid usage scenarios are classic OO. You may want to use the object for e.g. serialization or data binding using reflection (similar to how NH itself works).

John Chapman

Ok,

Let's make this more interesting. What if the base Animal had a method called ComeHere(). A cat ignores you because cats don't listen, and the dog comes. Calling that method instead of checking the type, would you get the correct polymorphic behavior? Assuming that this does behave correctly, you can work around this issue as long as you are aware of it.

While using NHibernate I never used lazy loaded properties to try to avoid situations like these. It was perfectly acceptable in our application.

Ayende Rahien

John,

You will get the right behavior, absolutely!

Schrödinger

no, the cat is dead when you check for it... so it must be dog

Ayende Rahien

Schrödinger,

That is aweseom

Roger Alsing

Anders,

Liskov would be rolling in her grave, if she had one

What does his statements have to do with Liskov substitution?

NPersist did load the related entities on property access (when using lazy load that is)

And thus, we always got an object of the correct type.

So your statement that this is try for all mappers is simply not correct.

You eiter lazy load using ghosts, or lazy load on property or field access.

(And I assume that POCO or not POCO and how to accomplish POCO is out of the scope of this post)

Ayende Rahien

Roger,

Loading on property access means that you kill a bunch of very important optimizations:

animalLover.Animal.Id <-- no load

session.Save(new TreatmentRecord{

Animal = animalLover.Animal <-- no load

});

Roger Alsing

That doesnt make the initial statement true.

animalLover.Animal.Id <-- no load

You could accomplish this using correct proxies also depending on how the mapping is created.

If the FK from animal lover to animal contains the type descriminator then you could very well instance a ghost proxy of the correct type.

Also, assigning objects of the wrong type to an association might be an optimization but it will also enable a bunch of problems.

And IMO, those problems are more related to Liskov.

Let's say that you'r setters of the .Animal property contains logic that makes decisions based on the type of the assigned object.

e.g.

set{

if (value is dog)

SendEmailIfDogLacksPuppyVaccination()

this.animal = value;

}

One can ofcourse argue about how valid that code is, but it does prove that a mapper that uses ghost proxies would break rule of least suprise and that it would potentially break pre/post conditions of your domain logic .

All of the above is somewhat offtopic, I'm just trying to say that your initial statement is not true and thus you shouldnt emphasise "any"

Roger Alsing

hehe, I'll bite even more on this.

This introduces what we called "Leaky this" problem.

The actual animal instance and the ghost proxy that is assigned to the Animal property will not be the same instance.

As you explained, the ghost proxy will delegate any call to the real instance.

So if the Animal class would contain a method called "SetMeAsFavoriteAnimal(AnimalLover lover)"

e.g.

void SetMeAsFavoriteAnimal(AnimalLover lover)

{

lover.Animal = this;

}

then consider the following code:

AnimalLover lover1 = LoverRepo.GetLoverById(1);

AnimalLover lover2 = new AnimalLover();

lover1.Animal.SetMeAsFavoriteAnimal(lover2);

if (lover1.Animal == lover2.Animal)

//THIS WILL NOT EXECUTE!!!

Because lover1.Animal is a ghost proxy and lover2.Animal is a real animal instance.

And this kind of problems are a PAIN to find.

I do understand you have chosen this approach from pragmatic reasons, but I think it is conceptually wrong.

kork

roger hibernate works with the concept of entity and 2 entity can be compared only by their id, so in your example you must compare using

lover1.Animal.Id == lover2.Animal.Id or redefine Equals and the == operator to works using only the Id.

conceptually the choise it's the better choise available

Ayende Rahien

Roger,

Nope, you can't do that without hitting the actual table container the data. That affect the general case vs. the special case.

And you code still violate Liskov

Ayende Rahien

Roger,

Yep, leaky this is a real and annoying issue.

We run around this a few times trying to resolve that, but there isn't a good solution that doesn't kill the general case for the special case.

However, NH says that entities SHOULD implement Equals, GetHashCode & Equality operators, because of that exact reason.

Tyler Burd

This is why I stopped using inheritance with NH a year ago. I tried using the interface proxies to get around the issues, but it never felt right having to create interfaces for my persistent classes and remember to use them only within an inheritance hierarchy. I switched to simple many-to-ones, where Animal would have Dog and Cat properties, and I've never been happier! The team understands what's going on, they can change the type of animal from cat to dog, and they don't get any of these "is it or isn't it a dog?!" issues.

I think the inheritance is very well done in NH, I just don't like having to consider so many special cases.

Mr_Simple

@Tyler

I switched to simple many-to-ones, where Animal would have Dog and Cat properties, and I've never been happier!

Mr_Simple approves of your coding style!

gunteman

"And you code still violate Liskov "

Why? How?

Ayende Rahien

See Replace conditional with polymorphism.

gunteman

Oh,you mean the "is" check? I agree about that one, but I fail to see how Anders' (not Richard's) statement has anything to do with that.

NHibernate has made a conscious decision in this matter. It serves a very real and valuable purpose. Doesn't make it "the right" decision though.

Fabio Maulo

Guessing that a Cat can't be a lover of a Dog I would imagine a different implementation.

ivos

hey, if you have an animal, you don't have a cat or a dog. you have an animal. you shouldn't make any decision based on the type. Instead, you should use a polymorphic solution (like the one John said, a Visitor, etc). And what about overriding methods with "new"?!!? I think all this problems will be solved once the people that uses ORMs understands the O.

Frank

How much of a real problem is it?

Unfortunately at my current company, we engage in anemic models, and to make it even more problematic, data sets with a custom written data layer for each system. And open source libraries like NHibernate are a no-go, because too many people think they need to be in full control of everything and open source is not to be trusted.

But back to the question. In my very limited experience with models, I don't see why one would check what the type of the property is. The reason why one uses inheritance is to provide different behavior. Checking for a type looks like behavior at the wrong place. John Chapman seems to be spot on with his example.

Roger Alsing

@Frank

Its way to easy to say "use polymorphism instead of conditionals and every problem will go away"

It does solve some problems, yes..

But what if you pass the object to some custom serializer?

What if you place it in a hashtable?

Also, never forget that people are people and will most likely do the most quick n dirty solution they can come up with.

And don't get me wrong, Im not trying to bash NHibernate or anything.

I'm just saying that O/R mapping is hard enough any way for most people.

So adding extra friction in terms of object comparison and identity problems will not make it easier for anyone.

Alex Yakunin

Just read this post (after reading & commenting a subsequent one about the same). I'm fully agree, such a practice is quite bad.

Moreover, this is completely contradictorily to Liskov principal (in fact, you're talking about inverse principal at all!): this principal does not imply you must expose absolutely the same logic as in base type. It implies any subclass instance can replace superclass instance everywhere. But you're doing COMPLETELY INVERSE THING!

Do you understand that such imagination of Liskov substitution principal makes polymorphism impossible at all?

Alex Yakunin

And... I also surprised to hear this: "However, NH says that entities SHOULD implement Equals, GetHashCode & Equality operators, because of that exact reason.".

I'm curious, what is the typical implementation? I.e. 2 things are clear for me:

  • Such instances are frequently used in sets & dictionaries

  • Thus their hash code shouldn't change

  • Likely, two instances with the same key must be equal

Thus I see the only good implementation of these methods: they must rely only on key comparison & key hash code.

If there will be anything else, you'll get mutable GetHashCode = forget about dictionaries.

Mats Helander

I switched to simple many-to-ones, where Animal would have Dog and Cat properties, and I've never been happier!

"Mr_Simple approves of your coding style!"

Same here.

Alex Yakunin

I switched to simple many-to-ones, where Animal would have Dog and Cat properties, and I've never been happier!

In some cases this is possible (and likely, desirable), although in general such a design won't work. I.e. it is a solution working in particular cases.

Comment preview

Comments have been closed on this topic.

FUTURE POSTS

No future posts left, oh my!

RECENT SERIES

  1. RavenDB Sharding (3):
    22 May 2015 - Adding a new shard to an existing cluster, splitting the shard
  2. The RavenDB Comic Strip (2):
    20 May 2015 - Part II – a team in trouble!
  3. Challenge (45):
    28 Apr 2015 - What is the meaning of this change?
  4. Interview question (2):
    30 Mar 2015 - fix the index
  5. Excerpts from the RavenDB Performance team report (20):
    20 Feb 2015 - Optimizing Compare – The circle of life (a post-mortem)
View all series

RECENT COMMENTS

Syndication

Main feed Feed Stats
Comments feed   Comments Feed Stats