Are there any pitfalls to using an IEnumerable<T> return type for SQL data?

Question

My question is concerning SQL connection status, load, etc. based on the following code:

public IEnumberable<MyType> GetMyTypeObjects()
{
  string cmdTxt = "select * from MyObjectTable";

  using(SqlConnection conn = new SqlConnection(connString))
  {
    using(SqlCommand cmd = new SqlCommand(cmdTxt, conn))
    {
      conn.Open();
      using(SqlDataReader reader = cmd.ExecuteReader())
      {
         while(reader.Read())
         {
            yield return Mapper.MapTo<MyType>(reader);
         }
       }
    }
  }
  yield break;
}

I can see this possibly being a problem if there are many processes running similar code with long execution times between iterations of the IEnumerable object, because the connections will be open longer, etc. However, it also seems plausible that this will reduce the CPU usage on the SQL server because it only returns data when the IEnumerable object is used. It also lowers memory usage on the client because the client only has to load one instance of MyType while it works rather than loading all occurrences of MyType (by iterating through the entire DataReader and returning a List or something).

• Are there any instances you can think of where you would not want to use IEnumerable in this manner, or any instances you think it fits perfectly?

• What kind of load does this put on the SQL server?

• Is this something you would use in your own code (barring any mention of NHibernate, Subsonic, etc)?

-

Answer 1

I wouldn't use it, because it hides what's happening, and it could possibly leave database connections without proper disposal.

The connection object will be closed when you have read past the last record, and if you stop reading before that the connection object won't be disposed. If you for example know that you always have ten records in a result and just have a loop that reads those ten records from the enumerator without making the eleventh Read call that reads beyond the last item, the connection is not closed properly. Also, if you wanted to use only part of the result, you have no way of closing the connection without reading the rest of the records.

Even the built in extensions for the enumerators may cause this even if you seamlingy use them correctly:

foreach (MyType item in GetMyTypeObjects().Take(10)) {
   ...
}

Answer 2

This is not a pattern I would follow. I would not worry as much about load on the server as I would about locks. Following this pattern integrates the data retrieval process into your business logic flow, and that seems like an all-around recipe for trouble; you have no idea what happens on the iteration side, and you're inserting yourself into it. Retrieve your data in one shot, then allow the client code to enumerate over it once you've closed the reader.

Answer 3

I recommend against pre-optimizing. In many situations, the connections will be pooled.

I also don't expect any difference in load on SQL Server - the query will already have been compiled, and will be running.

Answer 4

My concern would be that you're putting yourself completely at the mercy of the client code.

You've already mentioned that the calling code might hold the connection open longer than is strictly necessary, but there's also the possibility that it would never allow the objects to be closed/disposed.

So long as the client code uses foreach, using etc or explicitly calls the enumerator's Dispose method then you're fine, but there's nothing to stop it doing something like this:

var e = GetMyTypeObjects().GetEnumerator();
e.MoveNext();    // open the connection etc

// forget about the enumerator and go away and do something else
// now the reader, command and connection won't be closed/disposed
// until the GC kicks in and calls their finalisers