Why does my successfully-read Npgsql data disappear?

Question

I have the following code shape. It seems that I'm misunderstanding the C# method return values. How is it possible that a "full" enumerator gets returned as an empty one?

class ThingDoer
{
    public NpgsqlDataReader DoQuery()
    {
        NpgsqlCommand c = new NpgsqlCommand(...);
        NpgsqlDataReader dataread = c.ExecuteReader();
        return dataread;  // Debugger confirms that six data are enumerable here.
    }
}

...

class OtherThing
{
    public void higherLevelFunction()
    {
        NpgsqlDataReader result = myThingDoer.DoQuery();
        result.Read();  // No data! result's enumerable returns nothing!
    }
}

How does the debugger "confirm" this? What are you calling in the debugger? An empty result still has fields, but no rows. Also, where is the NpgsqlConnection in all this? — Jon Hanna, Oct 26 '11 at 13:44
@Jon Hanna I've eliminated all of the extra work around this, including mundane details like the connection, in favor of highlighting the actual problem. By setting a breakpoint, I was able to inspect data read before and after return. — Andres Jaan Tack, Oct 26 '11 at 15:51

score 3 · Accepted Answer · answered Oct 26 '11 at 13:52

3

You don't detail where your connection is coming from. Assuming it's something like:

public NpgsqlDataReader DoQuery()
{
    using(NpgsqlConnection = GetConnectionCode())
    {
        NpgsqlCommand c = new NpgsqlCommand(...);
        NpgsqlDataReader dataread = c.ExecuteReader();
        return dataread;
    }//Connection closes at this using-scope being left because that triggers Dispose()
}

Then change it to:

public NpgsqlDataReader DoQuery()
{
    bool ownershipPassed = false;
    NpgsqlConnection conn = GetConnectionCode();
    try
    {
        NpgsqlCommand c = new NpgsqlCommand(...);
        NpgsqlDataReader dataread = c.ExecuteReader(CommandBehavior.CloseConnection);
        ownershipPassed = true;
        return dataread;
    }
    finally
    {
        if(!ownershipPassed)//only if we didn't create the reader than takes charge of the connection
          conn.Dispose();
    }
}

Then where you use the reader, you have to dispose it to in turn dispose the connection's underlying connection to the database:

public void higherLevelFunction()
{
    using(NpgsqlDataReader result = myThingDoer.DoQuery())
      result.Read();
}

answered Oct 26 '11 at 13:52

Jon Hanna

110,372
10
146
251

Ah! I see why you asked about the connection now. Sorry for being snarky. :-) – Andres Jaan Tack Oct 26 '11 at 15:52
In fact I was explicitly Close()ing the connection, thinking it had run its course. – Andres Jaan Tack Oct 26 '11 at 15:54
I didn't read it as snarky. Deciding something is extraneous to a problem when it is in fact crucial happens to every one of us. Incidentally, with an earlier version of Npgsql, calling Close() would have worked fine, but this was actually a flaw because it led to large datasets being read in entirely rather than as-needed which affected performace as per http://npgsql.projects.postgresql.org/docs/manual/preloadScalabilty.html but since some old code (including one of the NUnit tests) took your approach, the `Preload Reader` commandstring option goes back to the old approach... – Jon Hanna Oct 26 '11 at 16:15
...however, while that option would make your previous code work, it has a nasty performance impact (as per the graph linked to). Also, since the IDataReader spec doesn't promise to retain results in such a case, you'd hit the same problem if you ever moved to a different database provider. – Jon Hanna Oct 26 '11 at 16:18
See also: http://stackoverflow.com/questions/3845764/what-does-opening-a-connection-actually-mean/3845924#3845924 which has some points relevant to this. – Jon Hanna Oct 26 '11 at 16:32
To clarify, do we need to explicitly call `result.Dispose()` in the higherLevelFunction? – Andres Jaan Tack Oct 27 '11 at 07:58
2

You explicitly or implicitly `Close()` it. You'll know all the reasons on why you should close connections, and about `using` making it easier to not have slips in this. When you call `ExecuteDataReader` with the `CommandBehavior.CloseConnection` then the `IDataReader` "takes ownership" of the connection, and closing it will close the connection. Hence while in this case we don't close or dispose the connection ourselves, we do still have to makes sure we close the reader. This can be through `Close()`, `Dispose()` or having `using` call `Dispose()`. See the answer I linked to above for more. – Jon Hanna Oct 27 '11 at 08:43

score 1 · Answer 2 · edited Jan 22 '13 at 02:23

1

NpgsqlCommand c = new NpgsqlCommand(...);
        NpgsqlDataReader dataread = c.ExecuteReader();

The above lines are very local to the method DoQuery. So as soon as the control comes out of the method, every object created inside to this method loses its scope. Hence you're losing the data because it's a reference type you're referring to in the caller method.

edited Jan 22 '13 at 02:23

xpda

15,585
8
51
82

answered Oct 26 '11 at 13:36

Zenwalker

1,883
1
14
27

Do how do I avoid losing the object, then? – Andres Jaan Tack Oct 26 '11 at 13:43
Make the connection object a class variable. That way it wont looses its scope. Even the same with DataReader too, you may instantiate it when it is required, i mean when you can DoQuery, then intantiate it i.e new SqlCommand(), but let the variables be in class scope. ANy ways your creating the DoQuery's class object in your consumer code, so its ok! – Zenwalker Oct 26 '11 at 13:45
No, you most certainly can return an NpgsqlDataReader from a method. You'd want to make sure the NpgsqlConnection didn't close (best by using CommandBehaviour.CloseConnection to ensure that it did eventually close with the reader). It's possible they are closing their connection too (it's not mentioned in the question), so you might be on the right track as to the problem, but even then scope isn't the issue per se. – Jon Hanna Oct 26 '11 at 13:46
1

Dear gods! Don't make the connection a class variable. – Jon Hanna Oct 26 '11 at 13:47
@JonHanna Your right if at all if the OP has created the connection in a global scope, but in his code it appears that he is doing some thing like new SqlCommand(new SqlConnection....) code, hence obviously it looses its scope and automatically may or may not closes the connection. And i dont think the connection automatically closes just because its opened in the local scope unless your using Using(){}. Correct me if i am wrong. – Zenwalker Oct 26 '11 at 13:49
@AndresJaanTack Yes Jon is right here, dont make the connection global, rather make the data collection global. Sorry, what was i thinking! Apologies! – Zenwalker Oct 26 '11 at 13:51
Don't make anything global or class-scoped at all. Rather, use CommandBehavior.CloseConnection so that the reader controls the lifetime of the underlying connection to the database rather than having NpgsqlConnection in a using block. (Personally I prefer to go further and have a wrapper I put in using that doesn't dispose if it's been called with such a behaviour, but the gist is the same). – Jon Hanna Oct 26 '11 at 13:55
Yep thats a nice hack, but lets see how OP digests it. – Zenwalker Oct 26 '11 at 13:58
Actually I really like it. Doesn't seem hacky at all. – Andres Jaan Tack Oct 26 '11 at 15:56

Why does my successfully-read Npgsql data disappear?

2 Answers2

Linked