-
Notifications
You must be signed in to change notification settings - Fork 3.2k
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
Microsoft.Data.SqlClient 3.0.0 breaks async enumeration of results of SQL Server query including null rowversion value #25074
Comments
Note that EF is reverting back to SqlClient 2.1 (see #25057), since SqlClient 3.0 isn't an LTS release. |
Does that imply EF does not intend to maintain compatibility with the latest stable SqlClient? We don't tend to worry about LTS versions as we deploy continuously. For .NET projects, we usually take dependencies on the latest stable releases on the assumption we stay up to date with latest bug fixes and enhancements. |
It means that LTS versions of EF should only depend on LTS versions of SqlClient, otherwise you'll have LTS EF relying on an unsupported version at some point. Of course, assuming SqlClient itself is backwards-compatible, you can still decide to run with 3.0.0 in your particular program. I also think we should investigate this issue (i.e. is it actually an issue an EF) regardless of whether we actually reference 3.0.0. |
@frankbuckley are you using EF Core 5 in both cases, I see both 5 and 6 mentioned in your .csproj file? So, can you repro with EF Core 5 with the switch enabled? |
@ErikEJ - yes, repro with 5.0.7 and daily build - running with .NET 5 |
@roji Thanks for clarification |
Also seeing this in another test - yet, query results do not contain any bigints. This is not IAsyncEnumerable. Oddly (compared to above), exception only arises if I do not call options.EnableRetryOnFailure().
|
we get the same issue but we are not using EnableRetryOnFailure
|
@frankbuckley - For issue with long to int, that seems to be separate issue than the other issues reported, perhaps we should start a separate issue thread for it. Stacktrace is trying to read Int32 value but server returned Int64. You can get SQL for EF Core query and try investigating which column returns Int64 and why. Generally that happens when there is some mismatch in configuration somewhere. Also if you don't have enabled Sensitive data logging, it can also give more details sometimes in terms of which column/property caused error. See https://docs.microsoft.com/en-us/ef/core/logging-events-diagnostics/simple-logging#getting-detailed-messages |
For original issue with byte[], efcore/src/EFCore.SqlServer/Storage/Internal/SqlServerTypeMappingSource.cs Lines 48 to 56 in 7105f84
We specify a custom comparer for byte[] in SqlServer, perhaps that comparer was working before as we had empty array and now it fails because we need the usual DbNull check. I would expect this to cause issue in all the cases and not just the case with EnableRetryOnFailure. EnableRetryOnFailure in above cases is just red herring. Hence @SimonCropp is seeing error without it. The other possibility of error (though less likely) is in BufferedDataReader code. Which buffers a DbDataReader for re-trying strategies. EF Core may buffer queries in certain scenarios even when EnableRetryOnFailure is not enabled. Specifically when doing split query and MARS being off (what I am seeing in stacktrace of @SimonCropp's report). cc: @ajcvickers for value comparer in type mapping source |
@smitpatel is that documented anywhere? |
This is the PR dotnet/SqlClient#998 for the change. Breaking change noted here https://github.com/dotnet/SqlClient/blob/main/release-notes/3.0/3.0.0-preview2.md |
shouldnt the final release notes for SqlClient include all the changes? i would not expect people would review all the preview release notes when going from one stable to another stable |
|
I also started seeing this issue after upgrading to
|
@jonsagara can you provide a complete repro? |
Unfortunately, I haven't been able to reproduce it in a test app that I can share, but I can reproduce it at will in my application. :( The application is a .NET 5 console app using EF Core 5.0.7 with a SQL Server database. The code that generates the failing query looks like this: var query = _context.UploadedDocuments
.Include(ud => ud.RequestedBy)
.Include(ud => ud.LastDownloadedByUser)
.Where(ud => ud.DocumentType == UploadedDocumentType.PDF && ud.UploadedUtc != null);
if (noTracking)
{
query = query.AsNoTracking();
}
return await query
.OrderByDescending(ud => ud.UploadedUtc)
.FirstOrDefaultAsync(); An I hope that helps. |
I note this has been assigned to v6, which i assume is in November. IMO this is a significant enough issue that perhaps it justifies a patch? |
@SimonCropp A complete repro is needed first. 😀 |
@ErikEJ does the repro provided in the description not work? ie
|
so does this justify a patch? |
just wondering if there is any answer for my question from 19 days ago? |
@SimonCropp - This issue is put in 6.0 milestone so we plan to fix it in 6.0 release. We will look into the repro to investigate and fix the issue when we get around to it. We are currently working on other things. We will circle back if we are not able to repro the issue when we investigate. We will discuss about patching this once we have identified the fix (the complexity and risk associated with the fix determine if it can be patched or not). Since this behavior only happens when user manually upgrade to a higher major version of SqlClient being used rather than the one shipped with EF Core in past, it lowers priority. Though we intend to make sure that future releases of EF Core works well with SqlClient 3.0 |
@smitpatel thanks so much for the clarification |
I have the same issue. Enabling the I recreated the issue here: dotnet/SqlClient#1175 |
Ran into the same thing. All of a sudden, a query like this
The code was unchanged for a while, but I recently enabled Things work if I go sync again. it also works if I just run the query with two includes, and then follow the table links (so access As my byte arrays are version stamps, they're not empty, and there's no other byte arrays in my model. |
A fix to make the AppContext switch work properly is in progress dotnet/SqlClient#1182 |
I've tracked this down to what looks like a bug in SqlClient 3.0.0, opened dotnet/SqlClient#1228 to track. In a nutshell, SqlDataReader.IsDbNull returns wrong results for null timestamp when used after ReadAsync. Note also that The fix for LegacyRowVersionNullBehaviour has been merged for 4.0.0-preview1 (hopefully also to be backported to 3.0.1), though if dotnet/SqlClient#1228 is fixed that AppContext switch shouldn't be necessary for EF Core to work properly. |
According to dotnet/SqlClient#1228 (comment), the SqlClient bug was also already fixed as part of dotnet/SqlClient#1228. |
Checked with the just-released 4.0.0-preview1.21237.2, and the bug no longer repro's there. |
Upgrading to Microsoft.Data.SqlClient 3.0.0 results in InvalidCastException ("Unable to cast object of type 'System.DBNull' to type 'System.Byte[]'") - that does not occur with Microsoft.Data.SqlClient 2.1.3 - when async enumerating over the results of a query that includes null rowversion values and when
sqlOptions.EnableRetryOnFailure()
.I thought this might be something to do with dotnet/SqlClient#998. However, enabling the
LegacyRowVersionNullBehaviour
switch does not fix the problem.In trying to narrow down a repro, it became clear the error only occurs if
sqlOptions.EnableRetryOnFailure()
is called when configuring the context. This, plus the fact that non-async enumeration of the same query works seems to suggest problem in EfCore.Versions
Observed with:
Repro:
Repro project at: https://github.com/frankbuckley/efcore-sqldata3
Database:
Program:
Stacktrace:
Environment
Originally discovered in integration tests running on Ubuntu 20.04 with SDK 5.0.301 and Azure SQL Database.
Repro tested on Windows with local SQL Server 15.0.2080.9:
The text was updated successfully, but these errors were encountered: