Review documentation (and confirm behavior) for SqlDataReader.Close and Dispose

[roji]

The remarks section of the SqlDataReader.Close docs says:

You must explicitly call the Close method when you are through using the SqlDataReader to use the associated SqlConnection for any other purpose.

The Close method fills in the values for output parameters, return values and RecordsAffected, increasing the time that it takes to close a SqlDataReader that was used to process a large or complex query. When the return values and the number of records affected by a query are not significant, the time that it takes to close the SqlDataReader can be reduced by calling the Cancel method of the associated SqlCommand object before calling the Close method.

Some notes:

• If Dispose is called, Close isn't needed. This allows SqlDataReader to be used with the preferred using pattern.
• Output parameters seem to be filled in before Close is called.
• I'm not too sure avoiding filling in output/return parameter and RecordsAffected has any significance perf-wise. On the other hand, if the reader still has a big pending resultset which isn't needed, closing/disposing would need to consume it, which indeed has a perf impact, justifying a possible command cancellation instead. However, especially since cancellation has its own problems, the best practice should be to simply not to fetch rows that you don't need (e.g. use TOP(n)).
• Grammar-wise, in When the return values and the number of records affected by a query are not significant, the not seems like it should be removed.

[Wraith2]

Output parameters seem to be filled in before Close is called.
I'm not too sure avoiding filling in output/return parameter and RecordsAffected has any significance perf-wise.

Output parameters are filled in as they are encountered in the result stream. I don't see any relationship between close and filling in parameters. If the execution is cancelled/closed before the return values are passed back they won't be filled in. You can probably say in general that if a reader has been closed and not cancelled that at that time the output parameters will be filled in. I agree that it seems strange to tie performance considerations to them.

On the other hand, if the reader still has a big pending resultset which isn't needed, closing/disposing would need to consume it, which indeed has a perf impact, justifying a possible command cancellation instead.

I haven't looked into it yet but if close is called and the command simply then consumes results until the query is finished that would seem like an odd thing to do. I'd have thought it would use cancel in this case.

[roji]

I haven't looked into it yet but if close is called and the command simply then consumes results until the query is finished that would seem like an odd thing to do. I'd have thought it would use cancel in this case.

That could be more complex than it sounds, since cancelling a command could have effects; for example, if several statements are batched in the same command and the command is cancelled at some point, could that mean that later statements are skipped? If so (and it depends on the particular database and driver), then it seems highly undesirable to me that closing a reader would do that. This is the main reason Npgsql doesn't do something like this: closing a reader will always consume the resultset, and cancellation is only ever done if the user explicitly requests it. In any case I don't think things need to be very sophisticated here - the user should ultimately not select a huge resultset if they're not going to consume it etc.

In any case, I opened this issue more to check the docs and possibly align them more with the real behavior of SqlClient - although the actual behavior is also interesting.

[David-Engel]

@roji is correct on why the driver does not call cancel during close on a reader. This is common across all the SQL Server drivers.

I agree the remark needs adjusting. Since output parameters are filled in as they are encountered in the stream (which could be large), I think we should leave that info in about perf impact. Here's my proposal:

You must ensure the Close method is called when you are through using the SqlDataReader before using the associated SqlConnection for any other purpose. The Close method may either be called directly or through the Dispose method, disposing directly or in the context of a using block.

The Close method fills in the values for output parameters, return values and RecordsAffected, increasing the time that it takes to close a SqlDataReader that was used to process a large or complex query. When the return values and the number of records affected by a query are not important, the time that it takes to close the SqlDataReader can be reduced by calling the Cancel method of the associated SqlCommand object before the Close method is called.

Original version for comparison:

You must explicitly call the Close method when you are through using the SqlDataReader to use the associated SqlConnection for any other purpose.

The Close method fills in the values for output parameters, return values and RecordsAffected, increasing the time that it takes to close a SqlDataReader that was used to process a large or complex query. When the return values and the number of records affected by a query are not significant, the time that it takes to close the SqlDataReader can be reduced by calling the Cancel method of the associated SqlCommand object before calling the Close method.

[cheenamalhotra]

@roji waiting for your confirmation.
Thanks!

[roji]

The first paragraph read much better.

The Close method fills in the values for output parameters, return values and RecordsAffected

According to what @Wraith2 wrote here, that may not be the case... And in any case, that does seem to be the part that really increases closes time (as opposed to consuming the pending resultset). Here's a proposal tweaking the 2nd paragraph:

The Close method guarantees that the values for output parameters, return values and RecordsAffected are populated on the reader.

The Close method consumes any results that are still pending on the SqlDataReader at the time it is invoked. This may be a long operation depending on the amount of data to be consumed, and may be shortened by calling the Cancel method of the associated SqlCommand object before the Close method is called.

[David-Engel]

@roji

The Close method guarantees that the values for output parameters, return values and RecordsAffected are populated on the reader.

I'm not sure about saying "guarantees" as that may be misinterpreted that you can call Cancel before Close and still get output parameters, return values, and RecordsAffected.

How about:

The Close method populates the values for output parameters, return values and RecordsAffected on the SqlDataReader by consuming any pending results. This may be a long operation depending on the amount of data to be consumed. If it is not important to populate output values, return values, and RecordsAffected, the time to close may be shortened by calling the Cancel method of the associated SqlCommand object before the Close method is called.

[roji]

I'm not sure about saying "guarantees" as that may be misinterpreted that you can call Cancel before Close and still get output parameters, return values, and RecordsAffected.

Good point. I'm a little worried our wording here encourages people to cancel commands as a way for better perf, which really shouldn't be a good pattern - maybe we should just leave the bit out about cancellation altogether. But I'm OK with it in any case.

[Wraith2]

I'm not sure about saying "guarantees" as that may be misinterpreted that you can call Cancel before Close and still get output parameters, return values, and RecordsAffected.

You can. Close has nothing to do with populating those values. Only the current progress of the rpc read does, whether or not it has got as far as returning return values.

[roji]

@Wraith2 I think @David-Engel meant that saying "guarantees" implied that the values will be populated even if the command is cancelled prior to calling Close, which I'm pretty sure isn't the case?

[Wraith2]

agreed.

[David-Engel]

I like providing the Cancel tip and caveat. We've had users file issues that closing a reader takes a long time in drivers and we've told them that the alternative is to call Cancel. Slight tweak in the last sentence. How about:

The Close method populates the values for output parameters, return values and RecordsAffected on the SqlDataReader by consuming any pending results. This may be a long operation depending on the amount of data to be consumed. If output values, return values, and RecordsAffected are not important to your application, the time to close may be shortened by calling the Cancel method of the associated SqlCommand object before the Close method is called.

[roji]

LGTM!