Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Test DbDataReader for non-query commands #28

Closed
bricelam opened this issue Jun 13, 2019 · 2 comments
Closed

Test DbDataReader for non-query commands #28

bricelam opened this issue Jun 13, 2019 · 2 comments

Comments

@bricelam
Copy link
Contributor

I uncovered a new nest of scenarios that would be good to add coverage for in dotnet/efcore#16053

var nonQueryCommand = connection.CreateCommand();
nonQueryCommand .CommandText = "DELETE FROM X WHERE 0 = 1";

var reader = nonQueryCommand.ExecuteReader();
Assert.False(reader.HasRows);
Assert.Equal(0, reader.FieldCount);
Assert.Throws(() => reader.GetSchemaTable());
Assert.False(reader.Read());
// Other members should throw "no data" (the same as after Read() returns false)
@bgrainger
Copy link
Member

Most providers currently just return null from DbDataReader.GetSchemaTable(). However, this seems incorrect, so I've chosen to test for throwing InvalidOperationException instead of accepting the de facto behaviour.

@roji
Copy link
Collaborator

roji commented Jun 17, 2019

@bgrainger I think the logic for returning null in this situation is that (dynamic) API consumers may not be aware of whether the command was supposed to return a resultset or not. Telling them to call the API and catch the exception isn't good (i.e. use of exceptions for regular flow control).

On the other hand I do agree that using null to indicate no rows isn't ideal (am thinking ahead to when we null-annotate the ADO.NET surface API). We could tell consumers to check FieldCount first, and that calling GetSchemaTable() when FieldCount is 0 throws an exception.

/cc @divega @ajcvickers @Wraith2 @FransBouma

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

3 participants