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

Mapping to a ValueTuple with 8 or more elements only retrieves some columns #982

Closed
R2D221 opened this issue Mar 27, 2018 · 4 comments · Fixed by #1242
Closed

Mapping to a ValueTuple with 8 or more elements only retrieves some columns #982

R2D221 opened this issue Mar 27, 2018 · 4 comments · Fixed by #1242
Labels

Comments

@R2D221
Copy link

R2D221 commented Mar 27, 2018

I'm not sure if mapping to ValueTuples is supported 100%, but I noticed that if the tuple has 8 or more elements, only the first 7 are mapped and the rest are filled with default values. My guess is that tuples with more items are defined as an 8-tuple with a TRest parameter that is another tuple.

@mgravell
Copy link
Member

mgravell commented Mar 27, 2018 via email

@jnm2
Copy link
Contributor

jnm2 commented Apr 20, 2019

I started working on this. There appear to be two options: add a new API which is more powerful than IMemberMap, capable of setting valueTuple.Rest.Rest.Item1, or special-case ValueTuple in SqlMapper's IL generation.

The API route is more appealing to me because I've wanted it for user code in the past, so I asked about it in the v2 planning thread:

Would you be open to breaking the IMemberMap interface in v2 or introducing a new more powerful abstract MemberMap class? I'm looking at fixing #982, the issue of Dapper only setting the first seven fields of C# tuples. C# tuples encode the eighth item of a tuple as valueTuple.Rest.Item1 and the 20th item as valueTuple.Rest.Rest.Item6. Ideally, you'd put all the values on the stack and then call a series of ValueTuple constructors. The other option would be a series of ldflda for each .Rest field followed by stfld to set the final ItemX field.

There is no way to support this via IMemberMap. If a more powerful API is not introduced, ValueTuple will have to be special-cased in SqlMapper's IL generation logic.

The limitations of the IMemberMap API have been frustrating in my own code in the past as well, but I don't have any use cases on hand. We just decided not to use Dapper in those scenarios. It could be nice for Dapper users in general if we were able to map to actions other than passing a constructor parameter or setting a property or field.

@mgravell Please let me know if I missed anything. In the meantime, I'll assume the worst and work on a special-casing implementation that doesn't use the mapping API.

Unit tests for TupleTests.cs
        [Fact]
        public void TupleReturnValue_Works_With8Elements()
        {
            // C# encodes an 8-tuple as ValueTuple<T1, T2, T3, T4, T5, T6, T7, ValueTuple<T8>>

            var val = connection.QuerySingle<(int e1, int e2, int e3, int e4, int e5, int e6, int e7, int e8)>(
                "select 1, 2, 3, 4, 5, 6, 7, 8");

            Assert.Equal(1, val.e1);
            Assert.Equal(2, val.e2);
            Assert.Equal(3, val.e3);
            Assert.Equal(4, val.e4);
            Assert.Equal(5, val.e5);
            Assert.Equal(6, val.e6);
            Assert.Equal(7, val.e7);
            Assert.Equal(8, val.e8);
        }

        [Fact]
        public void TupleReturnValue_Works_With15Elements()
        {
            // C# encodes a 15-tuple as ValueTuple<T1, T2, T3, T4, T5, T6, T7, ValueTuple<T8, T9, T10, T11, T12, T13, T14, ValueTuple<T15>>>

            var val = connection.QuerySingle<(int e1, int e2, int e3, int e4, int e5, int e6, int e7, int e8, int e9, int e10, int e11, int e12, int e13, int e14, int e15)>(
                "select 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15");

            Assert.Equal(1, val.e1);
            Assert.Equal(2, val.e2);
            Assert.Equal(3, val.e3);
            Assert.Equal(4, val.e4);
            Assert.Equal(5, val.e5);
            Assert.Equal(6, val.e6);
            Assert.Equal(7, val.e7);
            Assert.Equal(8, val.e8);
            Assert.Equal(9, val.e9);
            Assert.Equal(10, val.e10);
            Assert.Equal(11, val.e11);
            Assert.Equal(12, val.e12);
            Assert.Equal(13, val.e13);
            Assert.Equal(14, val.e14);
            Assert.Equal(15, val.e15);
        }

@jnm2
Copy link
Contributor

jnm2 commented Apr 20, 2019

Pull request is ready for review! #1242

@mgravell
Copy link
Member

Awesome! 4-day weekend here and most of it has been pre-booked for family stuff, but I'll see if I can review this in the week. Much appreciated.

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

Successfully merging a pull request may close this issue.

4 participants