netstandard2.0: DataTable isn't usable as a parameter (SqlClient/DataTable issue)

dotnet / runtime

.NET is a cross-platform runtime for cloud, mobile, desktop, and IoT apps.

https://docs.microsoft.com/dotnet/core/

MIT License

14.98k stars 4.66k forks source link

netstandard2.0: DataTable isn't usable as a parameter (SqlClient/DataTable issue) #21671

Closed NickCraver closed 4 years ago

NickCraver commented 7 years ago

I'm not sure if there are other issues beyond this, but hitting the first boom with SQL TVPs (table valued parameters) when attempting to restore support in Dapper with netstandard2.0.

In the reference source for SqlParameter.CoerceValue(), there's an explicit check to not attempt conversion on a DataTable:

else if (TdsEnums.SQLTABLE == destinationType.TDSType &&
            (value is DataTable ||
            value is DbDataReader ||
            value is System.Collections.Generic.IEnumerable<SqlDataRecord>)) {
    // no conversion for TVPs.
    typeChanged = false;
}

Compared to the code in CoreFX:

else if (TdsEnums.SQLTABLE == destinationType.TDSType && (
            value is DbDataReader ||
            value is System.Collections.Generic.IEnumerable<SqlDataRecord>))
{
    // no conversion for TVPs.
    typeChanged = false;
}

Without this match, it falls down below resulting in:

System.InvalidCastException : Failed to convert parameter value from a DataTable to a IEnumerable`1.
---- System.InvalidCastException : Object must implement IConvertible.
Stack Trace:
    at System.Data.SqlClient.SqlParameter.CoerceValue(Object value, MetaType destinationType, Boolean& coercedToDataFeed, Boolean& typeChanged, Boolean allowStreaming)
    at System.Data.SqlClient.SqlParameter.GetCoercedValue()
    at System.Data.SqlClient.SqlParameter.Validate(Int32 index, Boolean isCommandProc)
    at System.Data.SqlClient.SqlCommand.SetUpRPCParameters(_SqlRPC rpc, Int32 startCount, Boolean inSchema, SqlParameterCollection parameters)
    at System.Data.SqlClient.SqlCommand.BuildRPC(Boolean inSchema, SqlParameterCollection parameters, _SqlRPC& rpc)
    at System.Data.SqlClient.SqlCommand.RunExecuteReaderTds(CommandBehavior cmdBehavior, RunBehavior runBehavior, Boolean returnStream, Boolean async, Int32 timeout, Task& task, Boolean asyncWrite, SqlDataReader ds)
    at System.Data.SqlClient.SqlCommand.RunExecuteReader(CommandBehavior cmdBehavior, RunBehavior runBehavior, Boolean returnStream, TaskCompletionSource`1 completion, Int32 timeout, Task& task, Boolean asyncWrite, String method)
    at System.Data.SqlClient.SqlCommand.ExecuteReader(CommandBehavior behavior)
    at System.Data.SqlClient.SqlCommand.ExecuteDbDataReader(CommandBehavior behavior)
    at System.Data.Common.DbCommand.System.Data.IDbCommand.ExecuteReader(CommandBehavior behavior)
C:\git\Dapper\Dapper\SqlMapper.cs(1052,0): at Dapper.SqlMapper.ExecuteReaderWithFlagsFallback(IDbCommand cmd, Boolean wasClosed, CommandBehavior behavior)
C:\git\Dapper\Dapper\SqlMapper.cs(1080,0): at Dapper.SqlMapper.<QueryImpl>d__130`1.MoveNext()
    at System.Collections.Generic.List`1.AddEnumerable(IEnumerable`1 enumerable)
    at System.Linq.Enumerable.ToList[TSource](IEnumerable`1 source)
C:\git\Dapper\Dapper\SqlMapper.cs(722,0): at Dapper.SqlMapper.Query[T](IDbConnection cnn, String sql, Object param, IDbTransaction transaction, Boolean buffered, Nullable`1 commandTimeout, Nullable`1 commandType)
C:\git\Dapper\Dapper.Tests\ParameterTests.cs(435,0): at Dapper.Tests.ParameterTests.DataTableParametersWithExtendedProperty()
----- Inner Stack Trace -----
    at System.Convert.ChangeType(Object value, Type conversionType, IFormatProvider provider)
    at System.Data.SqlClient.SqlParameter.CoerceValue(Object value, MetaType destinationType, Boolean& coercedToDataFeed, Boolean& typeChanged, Boolean allowStreaming)

This may be a single backporting oversight, or there may be deeper issues with DataTable as a parameter beyond this. Filing because this is the wall I'm hitting at the moment.

karelz commented 7 years ago

@saurabh500 @corivera can you please take a look? This looks like 2.0 blocker. Thanks!

NickCraver commented 7 years ago

I've pushed up the netstandard2.0 branch of Dapper to GitHub, you can grab it here to run the tests: https://github.com/StackExchange/Dapper/tree/7e5bd308f99b790212e6990bc4ec8b0c72579406

After pulling, you can do a build.cmd -BuildNumber 12345 to run all tests, etc. and see it go boom.

saurabh500 commented 7 years ago

@NickCraver While running the Dapper tests I get the following error:

Run Parameters:
Version:
BuildNumber: 12345
CreatePackages: False
RunTests: True
Base Version: 1.50.3-alpha1-12345
error: /ConsoleLoggerParameters:Verbosity=Quiet
Running tests: Dapper.Tests (all frameworks)
No executable found matching command "dotnet-xunit"
Error with tests, aborting build.

Do you have anything special setup before running build.cmd to setup dotnet-xunit?

NickCraver commented 7 years ago

@saurabh500 Nothing special, and I just tried it on a fresh VM with only .NET Core 2 installed. Perhaps you have a newer/internal dotnet in play and need to lower the SDK down to 2 with a global.json? It seems like it's not loading compatible build-time resources there :-/

geleems commented 7 years ago

@NickCraver After running build.cmd -BuildNumber 12345, I got all unit test failed with the same error message Dapper.Tests.QueryMultipleTests.SO35554284_QueryMultipleUntilConsumed [FAIL] Do you have any idea what config I should setup additionally?

D:\Projects\Test\Dapper>build.cmd -BuildNumber 12345
Run Parameters:
Version:
BuildNumber: 12345
CreatePackages: False
RunTests: True
Base Version: 1.50.3-alpha1-12345

Welcome to .NET Core!
---------------------
Learn more about .NET Core @ https://aka.ms/dotnet-docs. Use dotnet --help to see available commands or go to https://aka.ms/dotnet-cli-docs.

Telemetry
--------------
The .NET Core tools collect usage data in order to improve your experience. The data is anonymous and does not include command-line arguments. The data is collected by Microsoft and shared with the community.
You can opt out of telemetry by setting a DOTNET_CLI_TELEMETRY_OPTOUT environment variable to 1 using your favorite shell.
You can read more about .NET Core tools telemetry @ https://aka.ms/dotnet-cli-telemetry.

Configuring...
-------------------
A command is running to initially populate your local package cache, to improve restore speed and enable offline access. This command will take up to a minute to complete and will only happen once.
Decompressing 100% 4727 ms
Expanding 100% 20714 ms
Running tests: Dapper.Tests (all frameworks)
Detecting target frameworks in Dapper.Tests.csproj...
Building for framework net452...
  Dapper -> D:\Projects\Test\Dapper\Dapper\bin\Debug\net451\Dapper.dll
  Dapper -> D:\Projects\Test\Dapper\Dapper\bin\Debug\net45\Dapper.dll
  Dapper.EntityFramework -> D:\Projects\Test\Dapper\Dapper.EntityFramework\bin\Debug\net45\Dapper.EntityFramework.dll
  Dapper.Contrib -> D:\Projects\Test\Dapper\Dapper.Contrib\bin\Debug\net45\Dapper.Contrib.dll
  Dapper.Tests -> D:\Projects\Test\Dapper\Dapper.Tests\bin\Debug\net452\Dapper.Tests.dll
  2 File(s) copied
  2 File(s) copied
Running desktop CLR tests for framework net452...
xUnit.net Console Runner (64-bit Desktop .NET 4.0.30319.42000)
  Discovering: Dapper.Tests
Dapper: Dapper.SqlMapper, Dapper, Version=1.50.2.0, Culture=neutral, PublicKeyToken=null
Using Connectionstring: Data Source=.;Initial Catalog=tempdb;Integrated Security=True
.NET: 4.0.30319.42000
Loading native assemblies for SQL types...(from: D:\Projects\Test\Dapper\Dapper.Tests\bin\Debug\net452\x64\)...done.
  Discovered:  Dapper.Tests
  Starting:    Dapper.Tests
    Dapper.Tests.DataReaderTests.DiscriminatedUnionWithMultiMapping [FAIL]
      System.Data.SqlClient.SqlException : A network-related or instance-specific error occurred while establishing a connection to SQL Server. The server was not found or was not accessible. Verify that the instance name is correct and that SQL Server is configured to allow remote connections. (provider: Named Pipes Provider, error: 40 - Could not open a connection to SQL Server)
      ---- System.ComponentModel.Win32Exception : The system cannot find the file specified
      Stack Trace:
           at System.Data.ProviderBase.DbConnectionPool.TryGetConnection(DbConnection owningObject, UInt32 waitForMultipleObjectsTimeout, Boolean allowCreate, Boolean onlyOneCheckConnection, DbConnectionOptions userOptions, DbConnectionInternal& connection)
           at System.Data.ProviderBase.DbConnectionPool.TryGetConnection(DbConnection owningObject, TaskCompletionSource`1 retry, DbConnectionOptions userOptions, DbConnectionInternal& connection)
           at System.Data.ProviderBase.DbConnectionFactory.TryGetConnection(DbConnection owningConnection, TaskCompletionSource`1 retry, DbConnectionOptions userOptions, DbConnectionInternal oldConnection, DbConnectionInternal& connection)
           at System.Data.ProviderBase.DbConnectionInternal.TryOpenConnectionInternal(DbConnection outerConnection, DbConnectionFactory connectionFactory, TaskCompletionSource`1 retry, DbConnectionOptions userOptions)
           at System.Data.SqlClient.SqlConnection.TryOpenInner(TaskCompletionSource`1 retry)
           at System.Data.SqlClient.SqlConnection.TryOpen(TaskCompletionSource`1 retry)
           at System.Data.SqlClient.SqlConnection.Open()
        D:\Projects\Test\Dapper\Dapper.Tests\TestBase.cs(35,0): at Dapper.Tests.TestBase.GetOpenConnection(Boolean mars)
        D:\Projects\Test\Dapper\Dapper.Tests\TestBase.cs(21,0): at Dapper.Tests.TestBase.get_connection()
        D:\Projects\Test\Dapper\Dapper.Tests\DataReaderTests.cs(84,0): at Dapper.Tests.DataReaderTests.DiscriminatedUnionWithMultiMapping()
        ----- Inner Stack Trace -----

    Dapper.Tests.QueryMultipleTests.SO35554284_QueryMultipleUntilConsumed [FAIL]
      System.Data.SqlClient.SqlException : A network-related or instance-specific error occurred while establishing a connection to SQL Server. The server was not found or was not accessible. Verify that the instance name is correct and that SQL Server is configured to allow remote connections. (provider: Named Pipes Provider, error: 40 - Could not open a connection to SQL Server)
...

NickCraver commented 7 years ago

@geleems Dapper assumes a local SQL Server running (since we have to have a database to hit). If you fire one up, it uses your login and runs in tempdb.

geleems commented 7 years ago

@NickCraver Thanks. I ran localdb, and was able to kick off the tests. But it seems it does not hit the Dapper.Tests.ParameterTests.DataTableParametersWithExtendedProperty() test. I added Console.WriteLine("*********") line to see if the test was executed, but it does appear to the console. Do you have any idea why the test does not run?

D:\Projects\Test\Dapper>build.cmd -BuildNumber 12345
Run Parameters:
Version:
BuildNumber: 12345
CreatePackages: False
RunTests: True
Base Version: 1.50.3-alpha1-12345
Running tests: Dapper.Tests (all frameworks)
Detecting target frameworks in Dapper.Tests.csproj...
Building for framework net452...
  Dapper -> D:\Projects\Test\Dapper\Dapper\bin\Debug\net451\Dapper.dll
  Dapper -> D:\Projects\Test\Dapper\Dapper\bin\Debug\net45\Dapper.dll
  Dapper.Contrib -> D:\Projects\Test\Dapper\Dapper.Contrib\bin\Debug\net45\Dapper.Contrib.dll
  Dapper.EntityFramework -> D:\Projects\Test\Dapper\Dapper.EntityFramework\bin\Debug\net45\Dapper.EntityFramework.dll
  Dapper.Tests -> D:\Projects\Test\Dapper\Dapper.Tests\bin\Debug\net452\Dapper.Tests.dll
  2 File(s) copied
  2 File(s) copied
Running desktop CLR tests for framework net452...
xUnit.net Console Runner (64-bit Desktop .NET 4.0.30319.42000)
  Discovering: Dapper.Tests
Dapper: Dapper.SqlMapper, Dapper, Version=1.50.2.0, Culture=neutral, PublicKeyToken=null
Using Connectionstring: Data Source=.;Initial Catalog=tempdb;Integrated Security=True
.NET: 4.0.30319.42000
Loading native assemblies for SQL types...(from: D:\Projects\Test\Dapper\Dapper.Tests\bin\Debug\net452\x64\)...done.
  Discovered:  Dapper.Tests
  Starting:    Dapper.Tests
    Dapper.Tests.Providers.FirebirdTests.Issue178_Firebird [SKIP]
      Bug in Firebird; a PR to fix it has been submitted
    Dapper.Tests.PostcresqlTests.TestPostgresqlArrayParameters [SKIP]
      Postgresql is unavailable: No connection could be made because the target machine actively refused it
    Dapper.Tests.SQLCETests.MultiRSSqlCE [FAIL]
      System.Data.SqlServerCe.SqlCeException : Unable to load the native components of SQL Server Compact corresponding to the ADO.NET provider of version 8876. Install the correct version of SQL Server Compact. Refer to KB article 974247 for more details.
      Stack Trace:
           at System.Data.SqlServerCe.NativeMethods.LoadNativeBinaries()
           at System.Data.SqlServerCe.SqlCeEngine..ctor()
           at System.Data.SqlServerCe.SqlCeEngine..ctor(String connectionString)
        D:\Projects\Test\Dapper\Dapper.Tests\Providers\SQLCETests.cs(18,0): at Dapper.Tests.SQLCETests.MultiRSSqlCE()
System.Runtime.Serialization.SerializationException: Unable to find assembly 'System.Data.SqlServerCe, Version=4.0.0.0, Culture=neutral, PublicKeyToken=89845dcd8080cc91'.
   at System.Runtime.Serialization.Formatters.Binary.BinaryAssemblyInfo.GetAssembly()
   at System.Runtime.Serialization.Formatters.Binary.ObjectReader.GetType(BinaryAssemblyInfo assemblyInfo, String name)
   at System.Runtime.Serialization.Formatters.Binary.ObjectMap..ctor(String objectName, String[] memberNames, BinaryTypeEnum[] binaryTypeEnumA, Object[] typeInformationA, Int32[] memberAssemIds, ObjectReader objectReader, Int32 objectId, BinaryAssemblyInfo assemblyInfo, SizedArray assemIdToAssemblyTable)
   at System.Runtime.Serialization.Formatters.Binary.__BinaryParser.ReadObjectWithMapTyped(BinaryObjectWithMapTyped record)
   at System.Runtime.Serialization.Formatters.Binary.__BinaryParser.Run()
   at System.Runtime.Serialization.Formatters.Binary.ObjectReader.Deserialize(HeaderHandler handler, __BinaryParser serParser, Boolean fCheck, Boolean isCrossAppDomain, IMethodCallMessage methodCallMessage)
   at System.Runtime.Serialization.Formatters.Binary.BinaryFormatter.Deserialize(Stream serializationStream, HeaderHandler handler, Boolean fCheck, Boolean isCrossAppDomain, IMethodCallMessage methodCallMessage)
   at System.Runtime.Remoting.Channels.CrossAppDomainSerializer.DeserializeObject(MemoryStream stm)
   at System.AppDomain.Deserialize(Byte[] blob)
   at System.AppDomain.UnmarshalObject(Byte[] blob)
Building for framework netcoreapp1.0...
  Dapper -> D:\Projects\Test\Dapper\Dapper\bin\Debug\netstandard1.3\Dapper.dll
  Dapper.Contrib -> D:\Projects\Test\Dapper\Dapper.Contrib\bin\Debug\netstandard1.3\Dapper.Contrib.dll
  Dapper.Tests -> D:\Projects\Test\Dapper\Dapper.Tests\bin\Debug\netcoreapp1.0\Dapper.Tests.dll
  2 File(s) copied
  2 File(s) copied
Running .NET Core tests for framework netcoreapp1.0...
xUnit.net Console Runner (64-bit .NET Core 4.6.25009.01)
  Discovering: Dapper.Tests
Dapper: Dapper.SqlMapper, Dapper, Version=1.50.2.0, Culture=neutral, PublicKeyToken=null
Using Connectionstring: Data Source=.;Initial Catalog=tempdb;Integrated Security=True
CoreCLR
  Discovered:  Dapper.Tests
  Starting:    Dapper.Tests
    Dapper.Tests.MiscTests.Issue263_Timeout [SKIP]
      Long running
No pipeline: 2265ms
Pipeline: 2759ms
No pipeline: 1627ms
Pipeline: 2327ms
  Finished:    Dapper.Tests
=== TEST EXECUTION SUMMARY ===
   Dapper.Tests  Total: 259, Errors: 0, Failed: 0, Skipped: 1, Time: 32.245s
Error with tests, aborting build.

NickCraver commented 7 years ago

@geleems I only see the netstandard1.3 run - are you sure you're on the netstandard2 branch? It looks like you're testing current master.

saurabh500 commented 7 years ago

@geleems I could get the tests running

I cloned Dapper and then checked out Nick's commit git checkout 7e5bd308f99b790212e6990bc4ec8b0c72579406

I uninstalled all the versions of .Net Core. I have .Net Core 2.0 preview 1 installed from https://www.microsoft.com/net/core/preview#windowscmd

I ran the tests and I got an error

Run Parameters:
Version:
BuildNumber: 12345
CreatePackages: False
RunTests: True
Base Version: 1.50.3-alpha1-12345
Running tests: Dapper.Tests (all frameworks)
It was not possible to find any compatible framework version
The specified framework 'Microsoft.NETCore.App', version '1.0.4' was not found.
  - Check application dependencies and target a framework version installed at:
      \
  - Alternatively, install the framework version '1.0.4'.
Error with tests, aborting build.

After this I installed .Net Core Runtime 1.0.5 from https://www.microsoft.com/net/download/core#/runtime.

After this I can execute the tests successfully and I can repro the failures that @NickCraver has reported. The beginning of my Console Output looks like this.

D:\work\Dapper>build.cmd -BuildNumber 12345 -RunTests 1
Run Parameters:
Version:
BuildNumber: 12345
CreatePackages: False
RunTests: True
Base Version: 1.50.3-alpha1-12345
Running tests: Dapper.Tests (all frameworks)
Detecting target frameworks in Dapper.Tests.csproj...
Building for framework net452...
  Dapper -> D:\work\Dapper\Dapper\bin\Debug\net451\Dapper.dll
  Dapper -> D:\work\Dapper\Dapper\bin\Debug\net45\Dapper.dll
  Dapper.EntityFramework -> D:\work\Dapper\Dapper.EntityFramework\bin\Debug\net45\Dapper.EntityFramework.dll
  Dapper.Contrib -> D:\work\Dapper\Dapper.Contrib\bin\Debug\net45\Dapper.Contrib.dll
  Dapper.Tests -> D:\work\Dapper\Dapper.Tests\bin\Debug\net452\Dapper.Tests.dll
  2 File(s) copied
  2 File(s) copied
Running desktop CLR tests for framework net452...
xUnit.net Console Runner (64-bit Desktop .NET 4.0.30319.42000)
  Discovering: Dapper.Tests
Dapper: Dapper.SqlMapper, Dapper, Version=1.50.2.0, Culture=neutral, PublicKeyToken=null
Using Connectionstring: Data Source=.;Initial Catalog=tempdb;Integrated Security=True
.NET: 4.0.30319.42000
Loading native assemblies for SQL types...(from: D:\work\Dapper\Dapper.Tests\bin\Debug\net452\x64\)...done.
  Discovered:  Dapper.Tests
  Starting:    Dapper.Tests
    Dapper.Tests.SqliteTests.DapperEnumValue_Sqlite [SKIP]
      Sqlite is unavailable: Unable to load DLL 'sqlite3': The specified module could not be found. (Exception from HRESULT: 0x8007007E)
    Dapper.Tests.SqliteTests.Issue466_SqliteHatesOptimizations_Async [SKIP]
      Sqlite is unavailable: Unable to load DLL 'sqlite3': The specified module could not be found. (Exception from HRESULT: 0x8007007E)
    Dapper.Tests.SqliteTests.Isse467_SqliteLikesParametersWithPrefix [SKIP]
      Sqlite is unavailable: Unable to load DLL 'sqlite3': The specified module could not be found. (Exception from HRESULT: 0x8007007E)
    Dapper.Tests.SqliteTests.Issue466_SqliteHatesOptimizations [SKIP]
      Sqlite is unavailable: Unable to load DLL 'sqlite3': The specified module could not be found. (Exception from HRESULT: 0x8007007E)
    Dapper.Tests.SqliteTests.Isse467_SqliteLikesParametersWithoutPrefix [SKIP]
      Sqlite is unavailable: Unable to load DLL 'sqlite3': The specified module could not be found. (Exception from HRESULT: 0x8007007E)
    Dapper.Tests.PostcresqlTests.TestPostgresqlArrayParameters [SKIP]
      Postgresql is unavailable: No connection could be made because the target machine actively refused it
    Dapper.Tests.MySQLTests.Issue295_NullableDateTime_MySql_Default [SKIP]
      MySql is unavailable: Unable to connect to any of the specified MySQL hosts.
    Dapper.Tests.MySQLTests.Issue426_SO34439033_DateTimeGainsTicks [SKIP]
      MySql is unavailable: Unable to connect to any of the specified MySQL hosts.
    Dapper.Tests.MySQLTests.SO36303462_Tinyint_Bools [SKIP]
      MySql is unavailable: Unable to connect to any of the specified MySQL hosts.
    Dapper.Tests.MySQLTests.Issue295_NullableDateTime_MySql_ConvertZeroDatetime [SKIP]
      MySql is unavailable: Unable to connect to any of the specified MySQL hosts.
    Dapper.Tests.MySQLTests.Issue552_SignedUnsignedBooleans [SKIP]
      MySql is unavailable: Unable to connect to any of the specified MySQL hosts.
    Dapper.Tests.MySQLTests.Issue295_NullableDateTime_MySql_ConvertAllowZeroDatetime [SKIP]
      MySql is unavailable: Unable to connect to any of the specified MySQL hosts.
    Dapper.Tests.MySQLTests.Issue295_NullableDateTime_MySql_AllowZeroDatetime [SKIP]
      MySql is unavailable: Unable to connect to any of the specified MySQL hosts.

saurabh500 commented 7 years ago

I have a repro with SqlClient code, I extracted this from Dapper

public static void TestParametersWithTVP()
        {
            SqlConnectionStringBuilder builder = new SqlConnectionStringBuilder();
            builder.DataSource = "localhost";
            builder.IntegratedSecurity = true;
            builder.InitialCatalog = "tempdb";
            var table = new DataTable { Columns = { { "id", typeof(int) } }, Rows = { { 1 }, { 2 }, { 3 } } };

            table.ExtendedProperties["dapper:TypeName"] = "MyTVPType";

            using (SqlConnection connection = new SqlConnection(builder.ConnectionString))
            {
                connection.Open();

                using (SqlCommand cmd = connection.CreateCommand())
                {
                    try { cmd.CommandText = "drop proc #DataTableParameters"; cmd.ExecuteNonQuery(); }
                    catch { /* Dont Care about exception */ }
                }
                using (SqlCommand cmd = connection.CreateCommand())
                {
                    try { cmd.CommandText = "drop table #DataTableParameters"; cmd.ExecuteNonQuery(); }
                    catch { /* Dont Care about exception */ }
                }
                using (SqlCommand cmd = connection.CreateCommand())
                {
                    try { cmd.CommandText = "drop type MyTVPType"; cmd.ExecuteNonQuery(); }
                    catch { /* Dont Care about exception */ }
                }
                using (SqlCommand cmd = connection.CreateCommand())
                {
                    try { cmd.CommandText = "create type MyTVPType as table (id int)"; cmd.ExecuteNonQuery(); }
                    catch { /* Dont Care about exception */ }
                }

                using (SqlCommand cmd = connection.CreateCommand())
                {
                    try { cmd.CommandText = "create proc #DataTableParameters @ids MyTVPType readonly as select count(1) from @ids"; cmd.ExecuteNonQuery(); }
                    catch { /* Dont Care about exception */ }
                }

                using (SqlCommand cmd = connection.CreateCommand())
                {

                    cmd.CommandText = "#DataTableParameters";
                    SqlParameter parameter = cmd.Parameters.AddWithValue("@ids", table);
                    parameter.TypeName = "MyTVPType";
                    using (SqlDataReader reader = cmd.ExecuteReader())
                    {
                         while(reader.HasRows) { reader.NextResult(); }                    
                    }
                }
            }
        }

What this repro highlights is that SqlClient Parameters don't support TVPs. Looking into this.

saurabh500 commented 7 years ago

I was able to port the code for TVPs using DataTables in SqlParameters and after running the Dapper tests using my changes I could see 100% pass rate in Dapper.Tests


  Finished:    Dapper.Tests
=== TEST EXECUTION SUMMARY ===
   Dapper.Tests  Total: 274, Errors: 0, Failed: 0, Skipped: 13, Time: 37.300s
Building for framework netcoreapp2.0...
  Dapper -> D:\work\Dapper\Dapper\bin\Debug\netstandard2.0\Dapper.dll
  Dapper -> D:\work\Dapper\Dapper\bin\Debug\netstandard1.3\Dapper.dll
  Dapper.Contrib -> D:\work\Dapper\Dapper.Contrib\bin\Debug\netstandard1.3\Dapper.Contrib.dll
  Dapper.Tests -> D:\work\Dapper\Dapper.Tests\bin\Debug\netcoreapp2.0\Dapper.Tests.dll
  2 File(s) copied
  2 File(s) copied

...

Running .NET Core tests for framework netcoreapp2.0...
xUnit.net Console Runner (64-bit .NET Core 4.6.25302.01)
  Discovering: Dapper.Tests.Contrib
  Discovered:  Dapper.Tests.Contrib
  Starting:    Dapper.Tests.Contrib
    Dapper.Tests.Contrib.SqlServerTestSuite.TransactionScope [FAIL]
      Assert.Null() Failure
      Expected: (null)
      Actual:   Car { Computed = null, Id = 5, Name = "one car" }
      Stack Trace:
        D:\work\Dapper\Dapper.Tests\Helpers\Assert.cs(44,0): at Dapper.Tests.Assert.IsNull(Object obj)
        D:\work\Dapper\Dapper.Tests.Contrib\TestSuite.cs(542,0): at Dapper.Tests.Contrib.TestSuite.TransactionScope()

Dapper.Tests.Contrib.SQLiteTestSuite.TransactionScope [FAIL]
  Assert.Null() Failure
  Expected: (null)
  Actual:   Car { Computed = null, Id = 5, Name = "one car" }
  Stack Trace:
    D:\work\Dapper\Dapper.Tests\Helpers\Assert.cs(44,0): at Dapper.Tests.Assert.IsNull(Object obj)
    D:\work\Dapper\Dapper.Tests.Contrib\TestSuite.cs(542,0): at Dapper.Tests.Contrib.TestSuite.TransactionScope()

@NickCraver I do see a couple failures in Dapper.Tests.Contrib listed above.

One of them is Dapper.Tests.Contrib.SqlServerTestSuite.TransactionScope It looks like the test is expecting to use TransactionScope with SqlClient. SqlClient doesn't support TransactionScope. We had to move the feature out of .Net 2.0 due to lack time and this is quite a bit of an involved feature https://github.com/dotnet/corefx/issues/12534

karelz commented 7 years ago

@saurabh500 how can customers detect the missing feature? Is it runtime failure? Or compile time / missing API failure? If it is runtime failure, I'd be curious how blocking it is for which scenarios? (@NickCraver? @FransBouma? @cincuranet?) At minimum, we need to document the difference in ApPiCompat, so that Immo's tooling flags it to people ...

saurabh500 commented 7 years ago

@karelz I think you are referring to my comment It looks like the test is expecting to use TransactionScope with SqlClient. SqlClient doesn't support TransactionScope. We had to move the feature out of .Net 2.0 due to lack time and this is quite a bit of an involved feature dotnet/runtime#18918

This will lead to runtime anomaly in behavior. If one expects SqlClient command executions to be wrapped in a Single TransactionScope and a transaction executed in the scope, then the customers would find out that the TransactionScope transaction enlistment is not supported. This will be profound in failure cases, where the operations in TransactionScope are not really considered a part of a single transaction.

With TransactionScope support in SqlClient, we will not be able to get to the complete Desktop behavior, because only Local transactions are supported in Sys.Transactions and Distributed transactions across different Sql Servers will not be supported due to the current limitations of Sys.Transactions.

karelz commented 7 years ago

What will the customer see? Exception? Or just "wrong non-transactional behavior"?

saurabh500 commented 7 years ago

No exceptions. They will see non-transactional behavior

karelz commented 7 years ago

How bad and surprising is it? Can we flag specific method, field, enum value or something in the ApiCompat tool?

FransBouma commented 7 years ago

IMHO, if the user uses TransactionScope in the code, they'll assume it will work if there are no compile errors. If the user gets a runtime exception, it will be frustrating perhaps for the user, but at least they won't assume the work is performed using a single transaction. If things are silently done without transactional behavior it can be seriously bad if they have to rollback due to an error and the code won't roll back, resulting in an inconsistent database.

saurabh500 commented 7 years ago

I have opened an issue https://github.com/dotnet/corefx/issues/19894 to discuss this behavior.

I agree with @FransBouma . In case all the operations in a TransactionScope go well, then there would be no customer complains, but if one of the operations fail, the the application would expect all the operations in TransactionScope to be rolled back. This is bad for data integrity. The failure case is where Sys.Transactions are most important for. This can be bad for customers who are migrating existing applications.

karelz commented 7 years ago

Reopening to track port to rel/2.0.0 branch

moleculomjd commented 7 years ago

If I can chime in, I think the ongoing lack of support for TransactionScope in SqlClient is going to make it difficult to embrace .NET Core. The usual suggestion for creating connections is that you create and open them as late as possible, then dispose of them as soon as possible, meaning that for a given business process, the work of writing to the database may very well be split across multiple connections. As I understand it, if TransactionScope is not available, the only option is to create a transaction from an existing connection, which leaves no simple way to guarantee transactional behavior when a process requires multiple writes to a database -- which in my case, and I would imagine for a lot of others, happens quite frequently. For my application, the only clear workaround -- using the same connection for multiple commands -- would force connection and transaction management up into the domain layer, increasing complexity and adding extra responsibilities. In other words, the lack of support for TransactionScope seems to make less-than-desirable design choices inevitable. I'm certainly not on expert on this subject, so I would definitely welcome any insight on this problem. There's more about my particular situation here: https://stackoverflow.com/questions/44163915/net-core-transaction-decorator-without-transactionscope

karelz commented 7 years ago

@moleculomjd thanks for you information. Definitely valuable view point. @saurabh500 do we have TransactionScope issue tracking the future work? I would love to move the discussion there. If it is important, we should find a way to fund it across teams rather sooner than later. Thanks!

karelz commented 7 years ago

cc @divega

FransBouma commented 7 years ago

@moleculomjd ambient transactions aren't always a plus, as they create some overhead, and not every database supports them on the server side (afaik, only sqlserver does, and Oracle over MTS/COM+ with a separate service). An alternative can be to use a unit of work which collects the activity you have to perform for the business process, and then performs the actions quickly in a single ado.net transaction. Some ORMs supply a Unit of work class which is separated from persistence logic (EF does not btw). It isn't always a good alternative (e.g. when the business process is long with a lot of back/forth activity), but for multiple save actions in a pipeline of calls it might be.

divega commented 7 years ago

@karelz @moleculomjd @FransBouma TransactionScope support in SqlClient is covered by an issue @saurabh500 mentioned before in this thread: https://github.com/dotnet/corefx/issues/12534. Makes sense to start any discussion about the details of what it means for SqlClient there.

Any discussion about inherent limitations of System.Transactions in .NET Core (e.g. lack of distributed transaction support) ideally should go in a separate issue.

Mrpankaj666 commented 4 years ago

@NickCraver I am also facing the same issue object must implement iconvertible after upgrading .net core 2.2 to 3.1 but with Efcore and ado.net