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

Add a driver to support Microsoft.Data.SqlClient provider #2216

Merged
merged 6 commits into from
Dec 23, 2019

Conversation

hazzik
Copy link
Member

@hazzik hazzik commented Sep 16, 2019

This is just a cleaned-up copy of SqlClientDriver.

@fredericDelaporte
Copy link
Member

So for adding some context, this is about following Microsoft move on their data provider, as explained here.
This data provider targets netcoreapp2.1, netfx46, netstandard2.0, while NHibernate targets netcoreapp2.0.
So there is a mismatch. It is not blocking due to using reflection. And well, since consumers are supposed to explicitly add the dependency on the data provider anyway, they should discover by themselves they have to target netcoreapp2.1 at least.
(I have not looked into this in details, so I am just commenting, not reviewing yet.)

@hazzik
Copy link
Member Author

hazzik commented Sep 28, 2019

This is pity that they claim to support netstandard 2.0 while this is not true.

@bahusoid
Copy link
Member

they should discover by themselves they have to target netcoreapp2.1 at least.

Or use netstandard2.0 version of the lib.

This is pity that they claim to support netstandard2.0 while this is not true.

Why it's not true? netcoreapp2.1 might have some target specific optimizations missing in netstandard2.0

@hazzik
Copy link
Member Author

hazzik commented Sep 29, 2019

@bahusoid everything in netstandard2.0 throwing PlatformNotSupportedException.

@bahusoid
Copy link
Member

bahusoid commented Sep 29, 2019

It seems going to be fixed according to dotnet/SqlClient#204 (comment)

@fredericDelaporte

This comment has been minimized.

@hazzik

This comment has been minimized.

@hazzik

This comment has been minimized.

@fredericDelaporte
Copy link
Member

fredericDelaporte commented Oct 6, 2019

Checking the test suite, there is a number of troubles with this new data provider.

  • ExceptionsTest.SQLExceptionConversionTest are no more receiving the expected exception.
  • Be it when using SQL 2012 dialect or 2005, Time types tests are failing due to expecting DateTime instead of TimeSpan (that may be due the changes I suggested, but seems strange.)
  • Min date test fails too, complaining about the date being out of the old date range... (TypesTest.DateTypeFixture.ReadWriteMin).

Copy link
Member

@fredericDelaporte fredericDelaporte left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All tests now pass excepted GH1594 which is just a bad test that I am going to fix in another PR.

else
connection = new SqlConnection(connectionString);
connection = Sfi.ConnectionProvider.Driver.CreateConnection();
connection.ConnectionString = connectionString;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Driver.CreateConnection() should have been used from the start. It is meant for this. It just creates the connection, not doing anything else with it.

@hazzik hazzik merged commit c8c52aa into nhibernate:master Dec 23, 2019
@hazzik hazzik deleted the Microsoft.Data.SqlClient branch December 23, 2019 00:20
@fredericDelaporte fredericDelaporte added this to the 5.3 milestone Dec 23, 2019
@andreyciocan
Copy link

Hello everyone. I just tried to publish the master project and include it in my project. The problem is that MicrosoftDataSqlClient is not available under Nhibernate.Driver. Also, if I try to set the connection.driver_class to MicrosoftDataSqlClient it throws me an error that the driver cannot be created. Any hints?

@hazzik
Copy link
Member Author

hazzik commented Feb 3, 2020

Any hints?

Yes: it is not yet released.

@fredericDelaporte
Copy link
Member

You may use binaries from the CI builds if you wish. See the commits, click the green check then follow the Details link from "NHibernate (Release Package)" build. (Latest link) as of this writing.) Then login with any TeamCity account or as Guest, and go to the Artifact tabs. It will contain .Net Framework binaries and NuGet packages build from master.

@andreyciocan
Copy link

Any hints?

Yes: it is not yet released.

Thank's. Any idea when it will be released?

@bahusoid
Copy link
Member

bahusoid commented Feb 3, 2020

The problem is that MicrosoftDataSqlClient is not available under Nhibernate.Driver

There is no such class even on master. It's MicrosoftDataSqlClientDriver and not MicrosoftDataSqlClient

@andreyciocan
Copy link

As @fredericDelaporte suggested, I downloaded the Nuget package and installed Nhibernate from Local. I am using the following code to configure Nhibernate:
var cfg = new Configuration(); cfg.SetProperty("connection.connection_string", configuration["ConnectionStrings:MyConnection"]); cfg.SetProperty("dialect", "Project.DB.DataAccess.Dynamic.CustomDialect, Project.DB.DataAccess"); cfg.SetProperty("connection.driver_class", "Nhibernate.Driver.MicrosoftDataSqlClientDriver, Nhibernate"); return cfg;

When I try to run a db select command, the following exception is thrown:
NHibernate.HibernateException : Could not create the driver from Nhibernate.Driver.MicrosoftDataSqlClientDriver, Nhibernate. ----> System.TypeLoadException : Could not load type 'Nhibernate.Driver.MicrosoftDataSqlClientDriver' from assembly 'NHibernate, Version=5.2.0.0, Culture=neutral, PublicKeyToken=aa95f207798dfdb4'.

@bahusoid
Copy link
Member

bahusoid commented Feb 3, 2020

Common... Just type it right... How come namespace become "Nhibernate.Driver"??

And avoid using strings when you can use typed values. Something like:

cfg.SetProperty("connection.driver_class", typeof(MicrosoftDataSqlClientDriver).AssemblyQualifiedName);

@andreyciocan
Copy link

@bahusoid Thank you! It was a silly mistake...

@luisabreu
Copy link

Hello guys.

quick question: when will NH depend on Microsoft.Data.SqlClient instead of the System.Data.SqlClient? It's suposed to be merged, but it seems like NH's nuget package still references System.Data.SqlClient package...

@fredericDelaporte
Copy link
Member

This merge here does not change anything to the existing SqlClient drivers. It adds a new one, named MicrosoftDataSqlClientDriver. So instead of configuring NHibernate to use SqlClientDriver or Sql2008ClientDriver, configure it to use MicrosoftDataSqlClientDriver.

As this PR is in the 5.3 milestone, this also requires you to use at least a v5.3 NHibernate, currently available only as preview in the NHibernate MyGet feed.

@EngSayed
Copy link

EngSayed commented Jul 23, 2023

@fredericDelaporte, @bahusoid
I just switched from System.Data.SqlClient to Microsoft.Data.SqlClient and I added this configuration connection.driver_class to use MicrosoftDataSqlClientDriver but I am getting the following exception inside SqlClientBatchingBatcher.AddToBatch(..)

Unable to cast object of type 'Microsoft.Data.SqlClient.SqlCommand' to type 'System.Data.SqlClient.SqlCommand'.
Description: An unhandled exception occurred during the execution of the current web request. Please review the stack trace for more information about the error and where it originated in the code.

Exception Details: System.InvalidCastException: Unable to cast object of type 'Microsoft.Data.SqlClient.SqlCommand' to type 'System.Data.SqlClient.SqlCommand'.

I have the following config:

[Environment.BatchStrategy] = typeof(CustomDeleteSqlBatcherFactory).AssemblyQualifiedName,
[Environment.TransactionStrategy] = typeof(AdoNetWithSystemTransactionFactory).AssemblyQualifiedName,
[Environment.Dialect] = typeof(MsSql2008Dialect).AssemblyQualifiedName,
[Environment.Isolation] = "ReadCommitted",
[Environment.ReleaseConnections] = "on_close",
[Environment.ConnectionProvider] = typeof(DriverConnectionProvider).AssemblyQualifiedName,
[Environment.ConnectionDriver] = typeof(MicrosoftDataSqlClientDriver).AssemblyQualifiedName,
[Environment.ProxyFactoryFactoryClass] = typeof(StaticProxyFactoryFactory).AssemblyQualifiedName,
[Environment.BatchSize] = "1",
[Environment.DefaultBatchFetchSize] = "25",
[Environment.BatchFetchStyle] = "Dynamic"

@EngSayed
Copy link

Our CustomDeleteSqlBatcher inherits from SqlClientBatchingBatcher which is causing that exception to be thrown. Is there another class to inherit from which supports Microsoft.Data.SqlClient?

@fredericDelaporte
Copy link
Member

fredericDelaporte commented Jul 23, 2023

See #3258. (TLDR: use GenericBatchingBatcher.)

@EngSayed
Copy link

See #3258. (TLDR: use GenericBatchingBatcher.)

Thanks @fredericDelaporte, I found about the generic batcher. I tried it and exception was gone. Could you please let me know if there any performance difference While using the generic batcher? - just wondering

@fredericDelaporte
Copy link
Member

That is discussed in #3258 too. We are not aware of any significant performance difference, but we have not done extensive benchmarks to check this.

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

Successfully merging this pull request may close these issues.

None yet

6 participants