1

Can someone please suggest me what wrong with this query? How can I improve the performance and decrease the time taken to execute it?

IQueryable<Mapper> query = null;

query = (from c in entities.Users
         where c.UserEmailAddress == emailAddress
         && c.UserPassword == password 
         && c.IsAccountVerified == true
         select new Mapper()
         {
             UserId= c.UserID,
             Name = c.UserName

         });
custObj = query.ToList<Mapper>().FirstOrDefault();

I am using EF profiler it alerts me following warning:-

  1. Query on unindexed column
  2. Column Type mismatch
  3. More than one session per request

FYI:-

  • EmailAddress - varchar(50) - Non ClusteredIndex
  • Password - varchar(max) - No Index
  • IsAccountVerified - bool - No Index

Even in local, I notice its taking 2-4 seconds to execute?

Apart from it, is there can someone suggest imp guidelines to fine tune the queries in EF.

I am using EF6.0

DavidG
  • 113,891
  • 12
  • 217
  • 223
abc
  • 7
  • 6
  • 4
    `ToList()` wil return *every* record. Then you only want the first... I would highly recommend removing the `ToList()` method. It also appears you are storing clear text passwords, [Please don't do that](https://www.youtube.com/watch?v=8ZtInClXe1Q). – Erik Philips Oct 16 '15 at 15:25
  • @ErikPhilips.. I am not storing clear text password..this is DAL layer snippet..the password user provides is first encrypted in BLL and then passed to DAL layer.. – abc Oct 16 '15 at 16:07
  • @ErikPhilips.. as far as I know since I am using IQuerable it will filter the records on db server itself and returns only the filtered records and as there will be only one record per email(_emailaddress is UQ key_) so I don't think removing toList() will affect.. currently I have only 10-20records in the local db – abc Oct 16 '15 at 16:09
  • I'm very glad to hear all of that. Don't take any of that personal, there are many novice developers asking questions and I like to make sure basic's are covered, especially for security concerns! :) – Erik Philips Oct 16 '15 at 16:56
  • @ErikPhilips..no problem..mate...thanks for your inputs – abc Oct 16 '15 at 17:07

1 Answers1

1

I think the problem is that you're using unnecessary complex query as EmailAddress is probably unique. Now you are checking three conditions to select your record, but using only email address should be fairly enough. I would rather select user basing on EmailAddress (and maybe IsAccountVerified) and later checked password hash in code.

The code would be something like this (I haven't checked it):

var user = entities.Users.FirstOrDefault(u => u.EmailAddress == emailAddress)
Mapper custObj = null;
if(user != null && user.IsAccountVerified && user.Password == password)
    custObj = new Mapper
    {
        UserId = user.UserID,
        Name = user.UserName
    };

Now you are not making a query on non indexed column, and results will be the same.

I checked simillar case on MS SQL database. Select based on one condition using indexed column boosted the query nicely (0ms instead of 13ms in my case).

  • thanks..your suggestion really make sense. and I see there is improvement in the query but still I am continously getting a sugesstion in ef profiler **Multiple Object Context in a single request lead to poor performance** plus in the same transaction I need to update the login date.. can you share your thought on the best way to do it.. – abc Oct 16 '15 at 16:29
  • I haven't used EF Profiler, but it seems the warning is connected with more code than you posted. Maybe you are creating and using multiple contexts. AFAIK it's not a very big deal: [Look here for more info](http://stackoverflow.com/questions/1671334/performance-cost-of-creating-objectcontext-in-every-method-in-entity-framework-v). – Patryk Spytkowski Oct 16 '15 at 16:38
  • In another query.. I am in need to load data from 3 unassociate different tables.So what I am doing is using Mutlitasking(C#) fetching data in three separate queries..is it a good approach? – abc Oct 16 '15 at 16:41
  • If loading data in separate threads I would use separate data contexts. – Patryk Spytkowski Oct 16 '15 at 16:46
  • yes..separate data context..1 v.imp thing to check.. long back I watched video by MVP on entity framework, he suggested to use toList() after every query.. I don't remember for what good reason.. did I misunderstood his point? is it necessary to use .ToList() if using IQueryable or nothing such? – abc Oct 16 '15 at 16:49
  • @abc It's because IQueryable. All operations you make on IQueryable will change the query used to obtain data from DB. When you're using ToList() query is executed and result is put into a memory. Every operation you do after IQueryable.ToList() is made on objects in memory instead of database. – Patryk Spytkowski Oct 16 '15 at 17:27
  • @abc more on IQueryable on [MSDN](https://msdn.microsoft.com/en-us/library/system.linq.iqueryable(v=vs.100).aspx) and [StackOverflow](http://stackoverflow.com/questions/252785/what-is-the-difference-between-iqueryablet-and-ienumerablet) (especially second answer). – Patryk Spytkowski Oct 16 '15 at 17:34