http://ayende.comAyende @ RahienCopyright (C) Ayende Rahien 2004 - 2021 (c) 202660Rafal, I wouldn't go quite that far. The reason for this post is that I run into a bit of code that met the "we have select n+1 here" format quite well, and then tracked it down to end up in not having a select n+1. There is still an issue of a superfluous select, and I think that showing suspicious code can be very helpful. http://ayende.com/4093/reviewing-nerddinner-the-select-n-1-pitfall#comment17http://ayende.com/4093/reviewing-nerddinner-the-select-n-1-pitfall#comment17Fri, 31 Jul 2009 12:54:30 GMTYes, all apps have select n+1 problem, like all apps use too much memory and execute inefficient code. But is it a problem if the application does its job and gives acceptable results? Look, you can put any code in a loop so if I followed your logic I would have to declare all components as 'dormant' problems. http://ayende.com/4093/reviewing-nerddinner-the-select-n-1-pitfall#comment16http://ayende.com/4093/reviewing-nerddinner-the-select-n-1-pitfall#comment16Fri, 31 Jul 2009 12:49:32 GMTRafal, _all_ apps have select n+1 You start from that assumption, and you can only prove it to be false under very limited circumstances http://ayende.com/4093/reviewing-nerddinner-the-select-n-1-pitfall#comment15http://ayende.com/4093/reviewing-nerddinner-the-select-n-1-pitfall#comment15Fri, 31 Jul 2009 10:51:49 GMTcowgaR, Yes, you need to eager load http://ayende.com/4093/reviewing-nerddinner-the-select-n-1-pitfall#comment14http://ayende.com/4093/reviewing-nerddinner-the-select-n-1-pitfall#comment14Fri, 31 Jul 2009 10:48:53 GMTLook at the key map ctrl+alt+f7, ctrl+shift+f7 on my machine http://ayende.com/4093/reviewing-nerddinner-the-select-n-1-pitfall#comment13http://ayende.com/4093/reviewing-nerddinner-the-select-n-1-pitfall#comment13Fri, 31 Jul 2009 10:43:46 GMTHendry, Yes, it does. But it does so in a way that make is explicit that something is going on there, in most cases. http://ayende.com/4093/reviewing-nerddinner-the-select-n-1-pitfall#comment12http://ayende.com/4093/reviewing-nerddinner-the-select-n-1-pitfall#comment12Fri, 31 Jul 2009 10:43:01 GMTAren't you going too far in the hunt for Select N+1? I mean, you assume there's Select N+1 problem before even seeing the application at work or looking at database queries. This is not a very valuable review method. http://ayende.com/4093/reviewing-nerddinner-the-select-n-1-pitfall#comment11http://ayende.com/4093/reviewing-nerddinner-the-select-n-1-pitfall#comment11Fri, 31 Jul 2009 08:23:08 GMTexcellent post and great point about spark's html data caching! still, the question pointed by Hendry remains open, so the only remedy is to do an eagear load? (not in L2S but in generall as L2S eager load is flawed anyway) the nhProf note made a little grin on my face though :D http://ayende.com/4093/reviewing-nerddinner-the-select-n-1-pitfall#comment10http://ayende.com/4093/reviewing-nerddinner-the-select-n-1-pitfall#comment10Fri, 31 Jul 2009 08:14:20 GMT@John alt+shift+F12 http://ayende.com/4093/reviewing-nerddinner-the-select-n-1-pitfall#comment9http://ayende.com/4093/reviewing-nerddinner-the-select-n-1-pitfall#comment9Fri, 31 Jul 2009 07:00:03 GMT"ReSharper will help us figure that one out" What 's the shortcut to get that resharper menu open? I only know of the right click Find Usages that opens in a Tool Window http://ayende.com/4093/reviewing-nerddinner-the-select-n-1-pitfall#comment8http://ayende.com/4093/reviewing-nerddinner-the-select-n-1-pitfall#comment8Fri, 31 Jul 2009 02:37:52 GMTI agree and I'm a firm believer of VIewModel. But how does it solve N+1 problem? Doesn't it merely shifts the N+1 problem from View to Controller? http://ayende.com/4093/reviewing-nerddinner-the-select-n-1-pitfall#comment7http://ayende.com/4093/reviewing-nerddinner-the-select-n-1-pitfall#comment7Fri, 31 Jul 2009 02:32:12 GMTHendry, View Models, mostly. Which force me to explicitly marshal the value. More likely, I would simply watch stuff like in NH Prof http://ayende.com/4093/reviewing-nerddinner-the-select-n-1-pitfall#comment6http://ayende.com/4093/reviewing-nerddinner-the-select-n-1-pitfall#comment6Fri, 31 Jul 2009 02:09:40 GMTHaacked, I follow the logic of doing that in the view, in fact, I can argue for it for delayed batch execution purposes (when the later you run it, the better you are). I don't like it very much because it tend to create problems down the wrong, big ones. Changing the view should NOT be a cause to change the database behavior, and that is very often the case in such things. As for output caching, I don't like the way spark is doing that at all. HTML generation, for the most part, is the cheapest part in the whole deal, and not something that I particularily worry about. Data is what I tend to cache, and for that, I _certainly_ don't want to do it in the view. http://ayende.com/4093/reviewing-nerddinner-the-select-n-1-pitfall#comment5http://ayende.com/4093/reviewing-nerddinner-the-select-n-1-pitfall#comment5Fri, 31 Jul 2009 02:02:25 GMTMore interestingly, I'm curious about better approach you would use to tackle that in this particular example. The concept of rich domain-model and lazy-load often colide each other, sometimes causing massive N+1 explossion. As another example, let's say User has a list of RegisteredRsvps, defined as lazy-load association. And the user has instance property (or method): public bool IsRegistered { get { return !this.RegisteredRsvps.IsEmpty(); } } This is a pretty simple and valid domain design (I believe). But it's a dormant N+1. (perhaps all attempts for rich domain abstractions are likely to end up with dormant N+1s). That code works fine for current purposes, until we have a page that displays all users in a grid with a 'Status' column (Registered or Unregistered). It will fire an N+1 call. How would you tackle this? Do all controllers (who're about to loop over User.IsRegistered property) need to know they got to tell UserRepository to force join-load ReservedRsvps property? Why does UI controller even need to know about this knowledge anyway? http://ayende.com/4093/reviewing-nerddinner-the-select-n-1-pitfall#comment4http://ayende.com/4093/reviewing-nerddinner-the-select-n-1-pitfall#comment4Fri, 31 Jul 2009 01:45:27 GMTI love this post. It clearly illustrates the issues that can show up if you do something "immoral". Even if it can seem trivial on each level alone, somewhere down the pipeline, it'll get ya. http://ayende.com/4093/reviewing-nerddinner-the-select-n-1-pitfall#comment3http://ayende.com/4093/reviewing-nerddinner-the-select-n-1-pitfall#comment3Fri, 31 Jul 2009 00:27:02 GMTInteresting point about your moral objection to the view kicking off a database call. In this case, the view doesn't know it's causing a database call. It's merely a consequence of returning an IQueryable which defers the call till it's being accessed. I don't think this is necessarily bad in this case because the view is in process with the controller and very unlikely to be disconnected from the controller. Why do I say that? Well the responsibility for setting up the database call still lies in the controller and model. So I would say that separation of concerns is maintained. After all, if you called ToList() in the controller, the view code wouldn't have to change. However, from another angle, I understand the moral objection from the perspective of it's kind of hidden. What if I dispose the data context in the controller action method. That seems like a reasonable thing to do, but would cause a problem. In any case, Spark View Engine's new output caching feature relies on delayed execution of the model: [whereslou.com/2009/07/28/spark-output-caching](http://whereslou.com/2009/07/28/spark-output-caching) What do you think of that? It solves a particular problem in a very neat manner, but it faces your moral objection. http://ayende.com/4093/reviewing-nerddinner-the-select-n-1-pitfall#comment2http://ayende.com/4093/reviewing-nerddinner-the-select-n-1-pitfall#comment2Thu, 30 Jul 2009 22:39:12 GMTIt looks like the function was designed to see if the currently logged-in user is registered; and it is not like you can have several logged-in users per context at the same time. But I can see where a scenario like this can cause a potential SELECT N+1 problem. Traditional lazy-loading is not the only way to create this problem. http://ayende.com/4093/reviewing-nerddinner-the-select-n-1-pitfall#comment1http://ayende.com/4093/reviewing-nerddinner-the-select-n-1-pitfall#comment1Thu, 30 Jul 2009 21:13:06 GMT