DapperLib / Dapper

Dapper - a simple object mapper for .Net
https://www.learndapper.com/
Other
17.52k stars 3.68k forks source link

QueryMultiple on sproc with 4 result sets reading too much data on second Read command #272

Open lusid opened 9 years ago

lusid commented 9 years ago

I am using Mono v3.12.1 and Dapper v1.40.

I have a stored proc that returns 4 result sets. My code is using a QueryMultiple followed by 4 different foreach loops using unbuffered Read commands. It seems to detect when the first result set ends and moves on to the second one, but then continues to retrieve all of the remaining data from result sets 2-4 via the second foreach loop, and when it gets to the third loop, an ObjectDisposedException is thrown with message: The reader has been disposed; this can happen after all data has been consumed

The code is basically the same as what you have in the documentation at the link below. The only difference is that I'm streaming the data unbuffered using foreach loops (so no .Single or .ToList, although it breaks when I do that as well) and I'm using a SQL Server stored proc instead of inline SQL. https://github.com/StackExchange/dapper-dot-net#multiple-results

mgravell commented 9 years ago

k; just so we're on the same page - do you have a repeatable repro that demonstrates the problem? I can try to construct one, but it usually works better if we start from "here {code} is some code that I expect to work, and doesn't"

lusid commented 9 years ago

I just tried with a similar, but simpler, setup, and it actually worked... so I might have to try a few more things that bring it closer to the code I have that is failing before I can give you an exact repro that will trigger it every time. I know it is an actual bug though because I am able to use the SqlMapper.cs file from a month or two ago and everything runs ok. I'll just have to find a way to reproduce it for you.

lusid commented 9 years ago

Actually, my test project is failing. I was just using the old SqlMapper.cs file. When I replace it with the new one or use the latest Nuget project, it fails.

Gist: https://gist.github.com/lusid/421a29e924e4bf2c4c80 Updated again as Github removed my generic types due to HTML tag filtering.

Output: Language: C Language: C# Language: C++ Language: Javascript Fruit: Fruit: Fruit: Fruit:

Unhandled Exception: System.ObjectDisposedException: The reader has been disposed; this can happen after all data has been consumed at Dapper.SqlMapper+GridReader.ReadImpl[Vegetables](System.Type type, Boolean buffered) [0x00000] in :0 at Dapper.SqlMapper+GridReader.Read[Vegetables](Boolean buffered) [0x00000] in :0 [ERROR] FATAL UNHANDLED EXCEPTION: System.ObjectDisposedException: The reader has been disposed; this can happen after all data has been consumed at Dapper.SqlMapper+GridReader.ReadImpl[Vegetables](System.Type type, Boolean buffered) [0x00000] in :0 at Dapper.SqlMapper+GridReader.Read[Vegetables](Boolean buffered) [0x00000] in :0

mgravell commented 9 years ago

Thanks; that's helpful; will look On 17 Apr 2015 20:12, "Marc Melvin" notifications@github.com wrote:

Actually, my test project is failing. I was just using the old SqlMapper.cs file. When I replace it with the new one or use the latest Nuget project, it fails.

SQL Stored Proc:

CREATE PROC TestStuff AS SET NOCOUNT ON SET TRANSACTION ISOLATION LEVEL READ UNCOMMITTED

CREATE TABLE #test1 (ID BIGINT PRIMARY KEY, [Language] VARCHAR(30)) INSERT #test1 (ID, [Language]) VALUES (1, 'C'), (2, 'C#'), (3, 'C++'), (4, 'Javascript')

CREATE TABLE #test2 (ID BIGINT PRIMARY KEY, [Fruit] VARCHAR(30)) INSERT #test2 (ID, Fruit) VALUES (1, 'Apple'), (2, 'Banana'), (3, 'Orange'), (4, 'Lemon')

CREATE TABLE #test3 (ID BIGINT PRIMARY KEY, [Vegetable] VARCHAR(30)) INSERT #test3 (ID, Vegetable) VALUES (1, 'Carrot'), (2, 'Pea'), (3, 'Celery'), (4, 'Broccoli')

SELECT * FROM #test1 SELECT * FROM #test2 SELECT * FROM #test3

DROP TABLE #test1 DROP TABLE #test2 DROP TABLE #test3

C#:

public class Languages { public long ID { get; set; } public string Language { get; set; } }

public class Fruits { public long ID { get; set; } public string Fruit { get; set; } }

public class Vegetables { public long ID { get; set; } public string Vegetable { get; set; } }

public class Program { using (var sql = new SqlConnection()) { var result = sql.QueryMultiple ("TestStuff", null, null, 300, CommandType.StoredProcedure); foreach (var obj in result.Read(false)) Console.WriteLine ("Language: {0}", obj.Language); foreach (var obj in result.Read(false)) Console.WriteLine ("Fruit: {0}", obj.Fruit); foreach (var obj in result.Read(false)) Console.WriteLine ("Vegetable: {0}", obj.Vegetable); return 0; } }

Output: Language: C Language: C# Language: C++ Language: Javascript Fruit: Fruit: Fruit: Fruit:

Unhandled Exception: System.ObjectDisposedException: The reader has been disposed; this can happen after all data has been consumed at Dapper.SqlMapper+GridReader.ReadImplVegetables http://System.Type%20type,%20Boolean%20buffered [0x00000] in :0 at Dapper.SqlMapper+GridReader.ReadVegetables http://Boolean%20buffered [0x00000] in :0 [ERROR] FATAL UNHANDLED EXCEPTION: System.ObjectDisposedException: The reader has been disposed; this can happen after all data has been consumed at Dapper.SqlMapper+GridReader.ReadImplVegetables http://System.Type%20type,%20Boolean%20buffered [0x00000] in :0 at Dapper.SqlMapper+GridReader.ReadVegetables http://Boolean%20buffered [0x00000] in :0

— Reply to this email directly or view it on GitHub https://github.com/StackExchange/dapper-dot-net/issues/272#issuecomment-94055501 .

DarrenARBell commented 9 years ago

@lusid I am experiencing the same issue on mono, actually when trying to use Hangfire.io. Can you indicate the version that actually worked? Cheers

lusid commented 9 years ago

@DarrenARBell I'm not 100% sure of the commit I'm using of SqlMapper.cs, but I know I grabbed it from the repo shortly after this commit as it fixed another unrelated Mono issue that I reported: https://github.com/StackExchange/dapper-dot-net/commit/ac1c2c5289aa5c7c3e9784f1dd6901fdc65d2498

Hopefully, that helps. At the moment, it seems to me that QueryMultiple example in the docs with more than 2 resultsets could return invalid data to an application in Mono without throwing an exception of any sort, and I was unable to find the underlying cause or a workaround short of using the previous version.

lusid commented 9 years ago

I'm unable to get the unit tests to run so I'm not 100% sure this is correct, but here's a unit test I started writing in the hopes I could get a failure that was testable.

https://gist.github.com/lusid/c557c781d348c298cb8b

DarrenARBell commented 9 years ago

Hi I have added some further test to express the issue. See https://gist.github.com/DarrenARBell/3396dab132503516a379

It looks like there is could be an issue with the "yield finally" statement in the IEnumerable ReadDeferred method. It appears when buffering is enabled, the data reader is skipping every other result set. So for example when deserializing the 2nd result set, the data values from 3rd result are returned. The best (simpliest) test to see this behaviour is Issue272_ReturnQueryMultipleSingleBuffered. I hope that helps

dfal commented 8 years ago

Looks like on mono SqlMapper.GridReader.Read method can read only odd results, even are skipped.

I've tried to use 1.50.0-beta5 with mono 4.2.1, but it fails with

System.TypeInitializationException: The type initializer for 'Dapper.SqlMapper' threw an exception. ---> System.TypeLoadException: Failed for unknown reasons.
mgravell commented 8 years ago

Hmmm, that's definitely one I need to cycle back to. Odd that it works so differently, but : not unique for mono to have such ... curiosities. I've been focusing on core-clr: will need to install some up to date mono tools. Is this on windows? Linux? Both? Other? On 10 Dec 2015 8:13 p.m., "Dmitry Fal" notifications@github.com wrote:

Looks like on mono SqlMapper.GridReader.Read method can read only odd results, even are skipped.

I've tried to use 1.50.0-beta5 with mono 4.2.1, but it fails with

System.TypeInitializationException: The type initializer for 'Dapper.SqlMapper' threw an exception. ---> System.TypeLoadException: Failed for unknown reasons.

— Reply to this email directly or view it on GitHub https://github.com/StackExchange/dapper-dot-net/issues/272#issuecomment-163737083 .

dfal commented 8 years ago

both, on mono for Windows and for Linux.

from mono trace i've extracted

 [00004C68: 0.04144 4] ENTER: System.TypeLoadException:.ctor (string,string)(this:028033E0[System.TypeLoadException dapp.exe], [STRING:028032D0:Microsoft.SqlServer.Server.SqlDataRecord], [STRING:02803330:System.Data, Version=4.0.0.0, Cu
     lture=neutral, PublicKeyToken=b77a5c561934e089], )
   9 [00004C68: 0.04155 5] ENTER: System.TypeLoadException:.ctor (string,string,string,int)(this:028033E0[System.TypeLoadException dapp.exe], [STRING:028032D0:Microsoft.SqlServer.Server.SqlDataRecord], [STRING:02803330:System.Data, Version=
     4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089], [STRING:null], 0, )
mgravell commented 8 years ago

Will try to get to the bottom of this next week. Thanks.

sklose commented 8 years ago

I can confirm this bug on mono. Works fine for me on the Windows. Not sure if it helps, but I got this code:

        private async Task<Account> LoadAccount(int? id, string email)
        {
            using (var connection = await CreateConnection())
            {
                using (var result = await connection.QueryMultipleAsync(
                        "LoadAccount", new {Id = id, Email = email},
                        commandType: CommandType.StoredProcedure))
                {
                    var dto = (await result.ReadAsync<AccountDto>()).SingleOrDefault();
                    if (dto == null)
                    {
                        return null;
                    }

                    dto.Bookings = (await result.ReadAsync<BookingDto>()).ToArray();
                    dto.Payments = (await result.ReadAsync<PaymentDto>()).ToArray();
                    dto.Postings = (await result.ReadAsync<PostingDto>()).ToArray();

                    return dto.ToAccount();
                }
            }
        }

I also noticed I can get it to work if I comment reading the last two result sets, but then something else weird happens. The objects that are returned for the 2nd result set have invalid data and the row count doesn't match up. It looks like the properties don't get set at all and just keep their default values. I tried to run a data reader manually to make sure it's not a bug in mono and this produces the correct result:

    using (var con = new SqlConnection("..."))
    {
      await con.OpenAsync();
      using (var cmd = con.CreateCommand())
      {
        cmd.CommandText = "LoadAccount";
        cmd.CommandType = CommandType.StoredProcedure;
        cmd.Parameters.Add("Email", "xx@xx.xx");
        using (var reader = await cmd.ExecuteReaderAsync())
        {
          do
          {
            while (reader.Read())
            {
              for (int i=0; i<reader.FieldCount; i++)
                 Console.WriteLine("{0}={1}", reader.GetName(i), reader.GetValue(i));
              Console.WriteLine("--------");
            }
          } while (reader.NextResult());
        }
      }
    }
betimd commented 8 years ago

Is this bug corrected? Felt today in Mono/Ubuntu.

lessismore1 commented 8 years ago

Is this bug corrected? Have this problem in my production code today.

IdentityServer3.Core.Configuration.Hosting.LogProviderExceptionLogger System.InvalidOperationException: There is already an open DataReader associated with this Command which must be closed first. at System.Data.SqlClient.SqlCommand.b24(Task1 result) at System.Threading.Tasks.ContinuationResultTaskFromResultTask2.InnerInvoke() at System.Threading.Tasks.Task.Execute() --- End of stack trace from previous location where exception was thrown --- at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task) at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task) at System.Runtime.CompilerServices.ConfiguredTaskAwaitable`1.ConfiguredTaskAwaiter.GetResult() at Dapper.SqlMapper.d231.MoveNext() --- End of stack trace from previous location where exception was thrown --- at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task) at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task) at System.Runtime.CompilerServices.TaskAwaiter1.GetResult() at IdentityServer3.Dapper.ClientStore.d__4.MoveNext() in C:\Dev\BovisionGit\Bovision\BovisionMain\src\Bovision.Web.Accounts\IdentityServer3.Dapper\Stores\ClientStore.cs:line 45 --- End of stack trace from previous location where exception was thrown ---