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

C# 9 records #1571

Open
silkfire opened this issue Nov 16, 2020 · 26 comments
Open

C# 9 records #1571

silkfire opened this issue Nov 16, 2020 · 26 comments

Comments

@silkfire
Copy link

Are there any plans to add support for records (added in C# 9/.NET 5), more specifically positional records?

@mgravell
Copy link
Member

well, we'd need to think about what that means, since positional records still have names; do we bind by constructor parameter name (which would be my expectation)? or by ordinal position? what would you expect to happen here?

@silkfire
Copy link
Author

By name sounds most logical to me. See here on how it can be done.

@mgravell
Copy link
Member

I'm very familiar; right now, for nominal matching, it needs a parameterless constructor, which means that (if we assume the columns aren't an exact match) this doesn't work:

private record PositionalCarRecord(int Age, Car.TrapEnum Trap, string Name)

but this does:

private record PositionalCarRecord(int Age, Car.TrapEnum Trap, string Name)
{
    public PositionalCarRecord() : this(default, default, default) { }
}

I'll have to look to see how much work is involved in handling the first case.

mgravell added a commit that referenced this issue Nov 17, 2020
* initial net5.0 tweaks; also fix current IDE warnings

* lib updates and fixes

* more cleanup

* fix snafu

* skip locals init

* disable xunit shadow copy in tests

* test records

* explore scenario that currently works re #1571
@Tornhoof
Copy link
Contributor

I did it by name in spanjson, fwiw it was more work to get mixed records (with extra normal properties) to work than the positional stuff.
Additionally https://stackoverflow.com/questions/63097273/c-sharp-9-0-records-reflection-and-generic-constraints might help, but Marc probably already knows about it.

@PureKrome
Copy link
Contributor

👋🏻 G'Day @mgravell (and others). Is there any news on this? Boy this would be hella-sweet if this was implemented!

I just tried this (and assploded) ..

private record MyData(int Id, string Name, string Address, string Blah);

...
using (var db = new SqlConnection(connectionString))
{
    var results = await db.QueryAsync<MyData>(query, new { Name = request.Name });
}

so can't wait to see if this can come into Dapper.

(Side note: I just don't like using dynamic, which the above code works 💯 fine. I just prefer strongly typed stuff)

@mgravell
Copy link
Member

In what way exactly did it "assplode". Also, what was query, and how did that match to the type members? The reason I ask is: we have tests for both nominal and positional records in the current test suite, and they work fine. Also, what version are you testing against?

@mgravell
Copy link
Member

It may also we worth testing against the myget feed, in case the problem is simply that we haven't pushed it to NuGet: https://www.myget.org/F/dapper/api/v3/index.json

@PureKrome
Copy link
Contributor

I'm an silly sausage. I tried (myget 2.0.82) + a bugfix (on my side) it worked. I dropped back to 2.0.78 and it also worked.

So it could have always worked.

This was the error message I received:

A parameterless default constructor or one matching signature
(System.Int32 ,
System.String ,
System.String ,
System.String ,
System.String ,
System.Int32 ,
System.Int32 ,
System.Int32 ,
System.String ,
System.String ,
System.Int32 ) is required for materialization

So it seemed to be a strong-type mismatch between my result record set the ctor.

I'm usually a bit more careful than this (im guessing i didn't read the 2nd half of the error message + late night programming + 💤 ) so I'm sorry for potentially wasting your time. 😞

TL;DR; for the rest of the internet: Dapper 2.0.78 is working nicely with records :)

@phillip-haydon
Copy link
Contributor

@PureKrome doesn't appear to work with .NET 3.1, complains about parameterless constructor. 😭

@silkfire
Copy link
Author

@PureKrome doesn't appear to work with .NET 3.1, complains about parameterless constructor. 😭

AFAIK, records are a C# 9 feature which only works with .NET 5 and above.

@phillip-haydon
Copy link
Contributor

You can use records in 3.1, they just don't work 100% like they do in 5.0 it appears.

https://btburnett.com/csharp/2020/12/11/csharp-9-records-and-init-only-setters-without-dotnet5.html

@AnsonWooBizCloud
Copy link

I have found an interesting case that using Class works but Record doesn't:

public record Foo(string EffectiveDate);
public record Foo() {
public string EffectiveDate { get; set; }
}

public class Foo {
public string EffectiveDate { get; set; }
}

When the table is using Date / Datetime / Datetime2 data type, dapper will throw an exception said it can't match the Record which works fine when I switched back to Class.

@ModernRonin
Copy link

ModernRonin commented Dec 2, 2021

I noticed you need to take care about extraneous fields in the record coming from the DB. While with classes or even default-constructable records, this works fine, for positionally-constructed records, you get an error message about a parameterless constructor matching certain arguments and if you look at the message closely, you will notice what's the issue.

Example:
Say the type is:

public record Event
{
	public Guid Id { get; init; } 
	public EventType Type { get; init; } 
	public Guid MapId { get; init; } 
}

and you do a query like

select
  Id, Type, Timestamp, MapId
from
  MyTable

everything will work just fine. But if you change Event to

public record Event(Guid Id, EventType Type, Guid MapId);

the very same query will lead to an error telling you it needs a constructor matching all four fields.

Now this may seem like a stupid mistake to make, but it's much less obvious if you do something like select * or, like me, include a field in the query just to sort by (but are not interested in retrieving that field, yet the number of records is small enough you can afford to opt for simplicity over performance), like:

	(select 
		MapCommandId as Id, 
		InsertDate as Timestamp,
		MapId,
		0 as Type
	 from MapCommand
	 where MapId in @mapIds)
union
	(select
		Id,
		UpdateDate as Timestamp,
		RelatedMapId as MapId,
		2 as Type
	from Task
	where RelatedMapId in @mapIds)
union
	(select
		Id,
		UpdateDate as Timestamp,
		ChannelId as MapId,
		1 as Type
	from ChatEntry
	where ChannelId in @mapIds)
order by Timestamp asc

A work-around in my case is to wrap the query into another select that just selects the three fields in the record.

So whoever comes here to look at this issue, maybe this will help them. Also, it might not hurt to mention this somewhere prominently in the docs.

@hmih
Copy link

hmih commented Apr 6, 2023

I hit theDateTime parsing issue when database rows are mapped to positional record models. Same as @AnsonWooBizCloud mentions.

@SaahilClaypool
Copy link

SaahilClaypool commented May 31, 2023

I created a source generator for one of my projects that adds a private parameterless constructor to get around the warning

A parameterless default constructor [...] is required

namespace X;
[Srcgen.DapperConstructor]
partial record TestResult(string A, string B)
{
    public int C { get; set; }
}

which generates

namespace X
{
   public partial record TestResult
    {
        private TestResult() : this(default!, default!) { }
    }
}

Gist with the full code is here: https://gist.github.com/SaahilClaypool/b1aca690f154ec0fe9ed49751988e701

@Kurren123
Copy link

This would be extremely useful

@IdoAmar
Copy link

IdoAmar commented Aug 6, 2024

any plan to support records?

@aradalvand
Copy link

4 years later, records are now pretty mainstream in C# yet Dapper still doesn't support them.
Disappointing.

@phillip-haydon
Copy link
Contributor

4 years later, records are now pretty mainstream in C# yet Dapper still doesn't support them. Disappointing.

I’m pretty sure it’s a dead project. Between this and enums, and not accepting PRs. It seems like a project left to die. Which is a shame cos it was a great library for a long time.

@mgravell
Copy link
Member

Dead? No. Believe it or not: not every feature fits cleanly, and there is an awkward law of unexpected consequences around changing things like enum behaviour. Believe it or not, it isn't as simple as "just change it to work the way you want, and everything will be fine". Pretty much every change breaks someone in unexpected ways.

A lot of work (as discussed here) continues in the AOT direction, which gives us much more flexibility to understand the context of your code, making it possible/safe to make behaviour changes in a controlled opt-in / contextual way, which doesn't break the world. It also gives us huge abilities to influence the most complex part of Dapper (the materializer, which is currently all ref-emit and very expensive to maintain) - the materializer in the AOT output is much more capable of understanding things like records.

My suggestion: try the same with AOT mode enabled; if it doesn't work there (and I suspect it will), it is almost certainly fixable fairly easily, without any risk of us breaking the work.

And no: it hasn't been left to die. But neither is this my day job; activity comes in bursts and lulls, depending on the needs of other projects and things like "family", "personal life", etc.

@mgravell
Copy link
Member

Sorry, I should have included: https://aot.dapperlib.dev/gettingstarted

(the good news is: enabling AOT usually just means adding an attribute, more-or-less)

@x0rld
Copy link

x0rld commented Sep 10, 2024

4 years later, records are now pretty mainstream in C# yet Dapper still doesn't support them.
Disappointing.

I use records everywhere on my app with dapper as long as you put your cols and property in the same order it works perfectly

@mgravell
Copy link
Member

For reference: the AOT implementation does a much better job of handling out-of-order columns when using constructors, which includes records. Similarly (although tangential) it can also bind named tuples by column name (if even "defining a record" is too much overhead) - see https://aot.dapperlib.dev/rules/DAP012 for side notes.

@cyril265
Copy link

I tried the AOT implementation and it's working with records 👍
One minor inconvinience is that AOT doesn't work with private records. It just doesn't generate code for the method and doesn't display a warning. Not sure this is intended behavior.

@mgravell
Copy link
Member

It isn't intended; can you log that over on DapeprAOT?

@pblair-hartree
Copy link

From what I can see, the positional record signatures don't match if one introduces nullable fields. So if I have a database column storing datetime values that allows nullable fields, if my record has a DateTime? for the corresponding parameter I get a "signature does not match" error.

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

No branches or pull requests