Sproc fixes #28942
Sproc fixes #28942
Conversation
* Don't attempt to propagate rows affected result columns. * Warn if an unexpected trailing result set is found. * Throw an informative message if a result set is missing. Fixes dotnet#28803
| @@ -75,12 +75,15 @@ protected override void Consume(RelationalDataReader reader) | |||
| } | |||
| } | |||
|
|
|||
| Debug.Assert(onResultSet != true, "Unexpected result set found at end"); | |||
| if (onResultSet == true) | |||
There was a problem hiding this comment.
Note that this is best-effort. Specifically, if there were no (expected) result sets, then a trailing one wouldn't be detected (since onResultSet is null here). We could add logic to check for that (calling reader.Read and NextResult), but that would potentially slow things down only for the purpose of this check.
| @@ -53,7 +53,7 @@ protected override void Consume(RelationalDataReader reader) | |||
| { | |||
| if (onResultSet == false) | |||
| { | |||
| Check.DebugFail("Missing a result set"); | |||
| throw new InvalidOperationException(RelationalStrings.MissingResultSetWhenSaving); | |||
There was a problem hiding this comment.
See https://github.com/dotnet/efcore/pull/28942/files#r960524847, the same holds here.
| Dependencies.UpdateLogger.UnexpectedTrailingResultSetWhenSaving(); | ||
| } | ||
|
|
||
| reader.Close(); |
There was a problem hiding this comment.
Wouldn't you need to consume all result sets before closing the reader to populate output parameters?
There was a problem hiding this comment.
Wouldn't you need to consume all result sets before closing the reader to populate output parameters?
Not AFAIK - all output parameters should be made available after Close is called, regardless of whether NextResult was called before that (until it returned false). From the SqlClient docs:
The Close method populates the values for output parameters, return values and RecordsAffected on the SqlDataReader by consuming any pending results.
In any case, we've definitely consumed all known result sets - the only unconsumed ones here would be for resultsets the user hasn't configured in metadata, so I'm not sure what we'd do with output parameters from those commands...
There was a problem hiding this comment.
In any case, we've definitely consumed all known result sets - the only unconsumed ones here would be for resultsets the user hasn't configured in metadata, so I'm not sure what we'd do with output parameters from those commands...
The extra result sets might be unrelated and safe to ignore
Fixes to stored procedure update mapping
Fixes #28803