jonwagner / Insight.Database

Fast, lightweight .NET micro-ORM
Other
861 stars 145 forks source link

Allow anonymous types for QueryAsync outputParameters #367

Closed jdmichel closed 6 years ago

jdmichel commented 6 years ago

Currently it looks like Insight requires the outputParameters param to take a named type, but it would be more convenient to pass an anonymous type. Also, the call fails unless the output parameter is passed as both input and output parameter.

Given SQL ... CREATE PROCEDURE [dbo].[GetFoos] @offset INT = 0, @count INT = 10, @total INT OUTPUT AS BEGIN

SELECT @total = COUNT(*) FROM [dbo].[Foos];

SELECT 
    [Id]
    ,[Name]
FROM [dbo].[Foos]
ORDER BY [Id] ASC
OFFSET @offset ROWS FETCH NEXT @count ROWS ONLY;

END

Then this code will compile and run, but fails to return the total. ... var outs = new { total = 0 }; await connection.QueryAsync("GetFoos", new { offset, count, outs.total }, outputParameters: outs); Debug.Assert(outs.Total > 0);

I have to define a class (not a struct) to make it work. internal class OutParamsForGetFoos { public int total; } var outs = new OutParamsForGetFoos();

Using Insight 6.2.2 on a .net core 2.0 project.

jonwagner commented 6 years ago

Interesting...that should just work.

jdmichel commented 6 years ago

I'll try to make a standalone sample sometime this weekend. Not sure what it could be if it's expected to work. I had the same problem with ExecuteAsync. Is it also supposed to require the output parameter to be duplicated as in input parameter?

jdmichel commented 6 years ago

Tried for a few hours to duplicate the problem at home with a minimal solution, but everything works fine. (although you do have to specify all OUT params as inputs. This seems like a sql server problem though since even the metadata in SSMS shows the parameters as IN/OUT rather than just OUT.)

I'll try to pare down my work solution that has the problem when I get time, but looks like it's going to be harder to track down than I hoped. (Can't access work code from home.)

jonwagner commented 6 years ago

Insight should automatically add the output parameter even if you don't supply it as an input (it will be set to null or default(T) depending on the sql type).

I'll have some time this week to try to hack out a test case, but let me know if you can reliably reproduce it.

jonwagner commented 6 years ago

Oh wait I think i remember. If you create an anonymous object with new { total = 0 } I think that the c# compiler doesn't create a set accessor, so Insight can't bind it. If I recall properly (and I probably don't) the compiler creates something like this:

class Anonymous {
    private int _some_hidden_variable;
    public int total { get { return _some_hidden_variable; } }
}

I'll have to check, but it may be that Insight was able to detect the hidden field and use that in some cases, and if so, that may have changed for .NET Core.

jonwagner commented 6 years ago

I can repro with this (works with Foo class but not anonymous) (at least in .NET Core)

        public class Foo {
            public int p;
        }

        /// <summary>
        /// Test that an anonymous type can return an output parameter.
        /// </summary>
        [Test]
        public void TestAnonymousOutputParameter()
        {
            using (var connection = Connection())
            {
                connection.Open();

//              var outs = new { p = 0 };
                var outs = new Foo();
                connection.Query("TestOutputParameters", null, outputParameters: outs);

                Assert.AreEqual(9, outs.p);
            }
        }
jdmichel commented 6 years ago

OK. I can now duplicate it at home too. (Home sick today). I had created both C# and VB versions to see if that mattered, and then accidentally ran the VB version when I thought I was running C#.
Looks like this is just a limitation of C# which only supports immutable anonymous types. (VB defaults to mutable anonymous, but can create immutable by specifying a Key property.)

Hmmm. Seems like what's needed is a different API to make it convenient to call these methods from C#. I'm not sure what that could be. The new C# 7 ValueTuple tuples are mutable which might help a bit, but besides requiring C# 7 they also don't support having a just single member. (int Total, int Dummy) outs = (0,0); // This could work maybe //but this is just a syntax error (int Total) outs = (0);

Btw, I tried that to see if it would already work, but as expected it doesn't. Not even sure you can use reflection to get the named members of a ValueTuple.

I guess for now I can just create named types for output parameters.

jonwagner commented 6 years ago

I found that too. I will experiment with tuples but it looks like the compiler doesn’t emit the name attributes.

Another thing to try is using a dynamic/FastExpando. Not sure if it will look prettier.

On May 29, 2018, at 5:29 PM, Justin Michel notifications@github.com<mailto:notifications@github.com> wrote:

OK. I can now duplicate it at home too. (Home sick today). I had created both C# and VB versions to see if that mattered, and then accidentally ran the VB version when I thought I was running C#. Looks like this is just a limitation of C# which only supports immutable anonymous types. (VB defaults to mutable anonymous, but can create immutable by specifying a Key property.)

Hmmm. Seems like what's needed is a different API to make it convenient to call these methods from C#. I'm not sure what that could be. The new C# 7 ValueTuple tuples are mutable which might help a bit, but besides requiring C# 7 they also don't support having a just single member. (int Total, int Dummy) outs = (0,0); // This could work maybe //but this is just a syntax error (int Total) outs = (0);

Btw, I tried that to see if it would already work, but as expected it doesn't. Not even sure you can use reflection to get the named members of a ValueTuple.

I guess for now I can just create named types for output parameters.

— You are receiving this because you commented. Reply to this email directly, view it on GitHubhttps://github.com/jonwagner/Insight.Database/issues/367#issuecomment-392951498, or mute the threadhttps://github.com/notifications/unsubscribe-auth/ABk3dLbzormef4o4wz0mXYzGEVUUG21lks5t3b2wgaJpZM4UOG2y.

jonwagner commented 6 years ago

And now I remember fully...

As for tuples, when you make an anonymous tuple, the compiler doesn't emit the attributes that contain the names.

Using a dynamic/FastExpando works and isn't entirely ugly:

            using (var connection = Connection())
            {
                connection.Open();

                dynamic outs = new FastExpando();
                connection.Query("TestOutputParameters", null, outputParameters: (object)outs);

                Assert.AreEqual(9, outs.p);
            }
jonwagner commented 6 years ago

So, anonymous types and tuples are off the table due to compiler restrictions. Named types and dynamic objects work.

Any other ideas?

jonwagner commented 6 years ago

it looks like we are out of ideas, and the behavior is explained, so I'll close this.