Returning an IEnumerable using Linq-to-SQL which is then bound to a GridView

This function works, but it's really really slow and the generated SQL is gargantuan and horrible to look at. It's also very expensive to run when it shouldn't be.

public static IEnumerable TrackingBoardPCsgroup(string groupName)
{
    var pingresult = from p in db.GetTable()
                     group p by p.ComputerAsset into g
                     select g.OrderByDescending(t => t.HealthPingTime).FirstOrDefault();

    var healthresult = from p in db.GetTable()
                       group p by p.ComputerAsset into g
                       select g.OrderByDescending(t => t.HealthHeartbeatTime).FirstOrDefault();

    var query = (from t in TrackingBoardPCs()
                 where t.TrackingGroup == groupName
                 select new TrackingComputer
                 {
                     ComputerName = t.ComputerAsset,
                     IPAddress = t.ComputerIP,
                     Location = t.Location,
                     Pingable = (from p in pingresult where p.ComputerAsset == t.ComputerAsset select p.HealthPingResult).FirstOrDefault() ?? false,
                     PingTime = (from p in pingresult where p.ComputerAsset == t.ComputerAsset select p.HealthPingTime).FirstOrDefault() ?? DateTime.Now.AddDays(-100),
                     Username = (from p in healthresult where p.ComputerAsset == t.ComputerAsset select p.HealthCurrentUser).FirstOrDefault(),
                     CurrentWindow = (from p in healthresult where p.ComputerAsset == t.ComputerAsset select p.HealthCurrentWindow).FirstOrDefault(),
                     Uptime = (from p in healthresult where p.ComputerAsset == t.ComputerAsset select p.HealthUptime).FirstOrDefault()
                 }).OrderBy(t => t.Pingable);
    return query;  
}

TrackingBoardPCs function:

public static IQueryable TrackingBoardPCs()
    {

            var query = (from tbpcs in db.GetTable()
                         select tbpcs);

        return query;

    }

First, the fact that your methods are static, and accessing what appears to be a static, disposable field, scares me: your application quite possibly has structural problems much more important than a slow query - but you haven't included nearly enough code for us to review that aspect.

All I can say, is that db instances should be as short-lived as possible, and properly disposed once you're done with them; you might want to look into patterns like Repository and Unit-of-Work, to wrap up your LINQ-to-SQL dependencies.

As you may or may not know, LINQ-to-SQL queries don't hit the database immediately; deferred execution makes the provider translate your expressions into proper SQL when the results are iterated... which means... your UI is actually what's triggering query execution.

Let's see...

var pingresult = from p in db.GetTable()
                 group p by p.ComputerAsset into g
                 select g.OrderByDescending(t => t.HealthPingTime).FirstOrDefault();

I had to read twice and think more than I should have done so, to parse exactly what's going on here and make sure I was reading it right - the FirstOrDefault applies to each grouping, right?

Sometimes method notation is easier to read:

var pingResult = db.GetTable()
                   .GroupBy(p => p.ComputerAsset)
                   .Select(g => g.OrderByDescending(t => t.HealthPingTime)
                                 .FirstOrDefault());

If I read this correctly (did I?), pingResult is an IGrouping where each group contains the greatest HealthPingTime, and each group represents a ComputerAsset.

I would materialize this query immediately, into a dictionary:

var pingResults = db.GetTable()
                    .GroupBy(ping => ping.ComputerAsset)
                    .AsEnumerable()
                    .ToDictionary(
                        grouping => grouping.Key,
                        grouping => grouping.OrderByDescending(t => t.HealthPingTime)
                                            .FirstOrDefault());

I'd do the same for the other one:

var healthResults = db.GetTable()
                      .GroupBy(beat => beat.ComputerAsset)
                      .AsEnumerable()
                      .ToDictionary(
                          grouping => grouping.Key,
                          grouping => grouping.OrderByDescending(t => t.HealthHeartbeatTime)
                                              .FirstOrDefault());

Now you have two materialized dictionaries, and you're armed for \$O(1)\$ lookups that you don't need to query anymore - I believe your code makes a bunch of \$O(n)\$ lookups for every single row.

var query = db.GetTable()
              .Where(t => t.TrackingGroup == groupName)
              .ToList()
              .Select(t => CreateTrackingComputerItem(t, pingResults, healthResults)
              .OrderBy(t => t.Pingable);

Keep things clean, make a separate function for the messy projection:

private TrackingComputer CreateTrackingComputerItem(
    tblTrackingBoardPC row, 
    IDictionary pingResults, 
    IDictionary healthResults)
{
    tblTBHealthPing pingResult = null;
    pingResults.TryGetValue(row.ComputerAsset, out pingResult);

    tblTBHealthHeartbeat healthResult = null;
     healthResults.TryGetValue(row.ComputerAsset, out healthResult);

    return new TrackingComputer
        {
            ComputerName = row.ComputerAsset,
            IPAddress = row.ComputerIP,
            Location = row.Location,
            Pingable = pingResult != null
                ? pingResult.HealthPingResult
                : false,
            PingTime = pingResult != null
                ? pingResult.HealthPingTime
                : DateTime.Now.AddDays(-100),
            Username = healthResult != null
                ? healthResult.HealthCurrentUser
                : null,
            CurrentWindow = healthResult != null
                ? healthResult.HealthCurrentWindow
                : null,
            Uptime = healthResult != null
                ? healthResult.HealthUptime
                : null
        };
}

Calling ToList will materialize the query - yes, server-side sorting is generally faster, but you have complex projections going on here, and it's not because LINQ-to-SQL can do it all SQL-side that it should.

With SQL doing the filtering, and LINQ-to-Objects doing the complex projection, IMO you're getting the best of both worlds, and you're returning materialized query results to your UI.

That said - avoid exposing IQueryable (and unmaterialized queries in general) outside your data access code. The reason is deferred execution: because the query didn't run yet, your client code is "free" to extend the query and potentially add things that the LINQ provider can't translate into SQL, resulting in unexpected exceptions

StackExchange Code Review Q#122788, answer score: 4