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

netstandard2.0: SqlDataReader doesn't implement GetSchemaTable() #21696

Closed
NickCraver opened this issue May 13, 2017 · 13 comments
Closed

netstandard2.0: SqlDataReader doesn't implement GetSchemaTable() #21696

NickCraver opened this issue May 13, 2017 · 13 comments
Assignees
Labels
area-System.Data.SqlClient bug tenet-compatibility Incompatibility with previous versions or .NET Framework
Milestone

Comments

@NickCraver
Copy link
Member

There's a disconnect between the netstandard2.0 intent and actual implementations here, resulting in runtime errors on things that call .GetSchemaTable().

The way ADO.NET is setup, DbDataReader has added some methods over time (since adding to interfaces is bad), of these .GetSchemaTable() is one brought back in netstandard2.0. It's a virtual that each implementation needs to override.

Here's the Core FX DbDataReader.GetSchemaTable():

public virtual DataTable GetSchemaTable()
{
    throw new NotSupportedException();
}

The problem is SqlDataReader never overrides this.

For comparison, here's reference source for .NET 4.x where this does happen.

In the current state, the result is a NotSupportedException when actually calling the method:

System.NotSupportedException : Specified method is not supported.
Stack Trace:
    at System.Data.Common.DbDataReader.GetSchemaTable()
    at System.Data.ProviderBase.SchemaMapping..ctor(DataAdapter adapter, DataSet dataset, DataTable datatable, DataReaderContainer dataReader, Boolean keyInfo, SchemaType schemaType, String sourceTableName, Boolean gettingData, DataColumn parentChapterColumn, Object parentChapterValue)
    at System.Data.Common.DataAdapter.FillMappingInternal(DataSet dataset, DataTable datatable, String srcTable, DataReaderContainer dataReader, Int32 schemaCount, DataColumn parentChapterColumn, Object parentChapterValue)
    at System.Data.Common.DataAdapter.FillMapping(DataSet dataset, DataTable datatable, String srcTable, DataReaderContainer dataReader, Int32 schemaCount, DataColumn parentChapterColumn, Object parentChapterValue)
    at System.Data.Common.DataAdapter.FillFromReader(DataSet dataset, DataTable datatable, String srcTable, DataReaderContainer dataReader, Int32 startRecord, Int32 maxRecords, DataColumn parentChapterColumn, Object parentChapterValue)
    at System.Data.Common.DataAdapter.Fill(DataTable[] dataTables, IDataReader dataReader, Int32 startRecord, Int32 maxRecords)
    at System.Data.DataTable.Load(IDataReader reader, LoadOption loadOption, FillErrorEventHandler errorHandler)
    at System.Data.DataTable.Load(IDataReader reader)

Note that this wasn't calling it directly, it was trying to use DataTable much further away. So there's quite a few places this is used.

I think this was just an oversight after DataTable and truckloads of other things were brought back for netstandard2.0. Can we please get this in so it's usable?

Past issues for context on more usages (for prioritization):

Related concern: this isn't listed in dotnet/SqlClient#17 because it compiles and doesn't have a PlatformNotSupportedException, but it's obviously missing. I think we need a separate API call for ADO.NET for things that aren't overridden in CoreFX, that's the superset of items we need to narrow down and fill the gaps on. Not everything should be overridden, but that's likely the easiest list to generate and quickly narrow down.

@NickCraver
Copy link
Member Author

cc @karelz @saurabh500

@karelz
Copy link
Member

karelz commented May 13, 2017

@danmosemsft another example where we missed overrides in .NET Core 2.0 - I start to worry ... IMO we need to boost the tooling and scan for all missing implemented interfaces and overrides, then review them all.

[UPDATE] Created special tooling issue dotnet/corefx#19749

@karldodd
Copy link

+1. Found this problem too while migrating my lib to netstarndard2.0 .

@saurabh500
Copy link
Contributor

I have a branch with the code ported from Framework sitting at https://github.com/saurabh500/corefx/tree/datareaderSchema
This needs testing though. I can't get to it until tomorrow.
@NickCraver Do you think you can build the SqlClient from my branch and run it through Dapper tests?

@saurabh500 saurabh500 self-assigned this May 16, 2017
@NickCraver
Copy link
Member Author

@saurabh500 My time is very limited and building is usually a mess - if you can get me a .nupkg link I'm happy test it out that's pretty low time-cost and hopefully something easy to produce on your side? Let me know and I'll throw it in the mix ASAP.

@saurabh500
Copy link
Contributor

saurabh500 commented May 16, 2017

@NickCraver If you can tell me how to execute the Dapper Tests selectively, that would be great.
E.g. I want to run only Dapper.Tests.Tests.ExecuteReaderOpenAsync without executing any other tests. How do I do that?

@saurabh500
Copy link
Contributor

saurabh500 commented May 16, 2017

Running Dapper tests with my changes to SqlClient
Here is the status

Before changes
Dapper.Tests Total: 286, Errors: 0, Failed: 10, Skipped: 13, Time: 36.848s

After changes
Dapper.Tests Total: 286, Errors: 0, Failed: 5, Skipped: 13, Time: 36.291s

I don't see any more stack traces with the following.

System.NotSupportedException : Specified method is not supported.
Stack Trace:
     at System.Data.Common.DbDataReader.GetSchemaTable()

I will add tests with my changes and send out a PR.

PS: @NickCraver It would still be helpful to know the filters for Dapper tests so that I can execute them selectively.

@NickCraver
Copy link
Member Author

@saurabh500 absolutely, starting at the repo root, here's how to run a single test:

dotnet restore
cd Dapper.Tests
dotnet xunit -method "Dapper.Tests.Tests.ExecuteReaderOpenAsync"

@saurabh500
Copy link
Contributor

@NickCraver Thanks
Also is there a way to filter on the netcoreapp2.0 tests?

@NickCraver
Copy link
Member Author

@saurabh500 Yep! dotnet xunit -framework netcoreapp2.0, you can see all the filter options with dotnet xunit -h :)

@saurabh500
Copy link
Contributor

@NickCraver Thanks for the confirmation :) I was wondering if there is a different mechanism that you use.
Many different projects have their own way of doing things. So I thought I would ask

@Davilink
Copy link

Davilink commented May 1, 2018

I have i project who target the Net Framework 4.7.1 it use a project who target Net Standard 2.0. In the project Net Standard 2.0, i use an SqlDataReader and when i tried to use the GetColumnSchema() it throws an exception (Not supported).

What i'm missing, isn't supported to work ?

I tried something else and finally make work what i wanted, but it really same strange that it work.
Why this work ?

image

@msmolka
Copy link

msmolka commented May 9, 2018

@Davilink
see issue: https://github.com/dotnet/corefx/issues/26302

@msftgits msftgits transferred this issue from dotnet/corefx Jan 31, 2020
@msftgits msftgits added this to the 2.0.0 milestone Jan 31, 2020
@ghost ghost locked as resolved and limited conversation to collaborators Dec 23, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-System.Data.SqlClient bug tenet-compatibility Incompatibility with previous versions or .NET Framework
Projects
None yet
Development

No branches or pull requests

7 participants