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

Anonymous record fields names are sorted #6422

Closed
enricosada opened this issue Apr 3, 2019 · 10 comments
Closed

Anonymous record fields names are sorted #6422

enricosada opened this issue Apr 3, 2019 · 10 comments

Comments

@enricosada
Copy link
Contributor

enricosada commented Apr 3, 2019

When i create an anonymous record, the fields get sorted by name.

This cause issues when libraries expect the same order of declaration.
An example in dapper

// dotnet add package Dapper

open Dapper

// docker pull mcr.microsoft.com/mssql/server:2017-latest-ubuntu
// docker run -e "ACCEPT_EULA=Y" -e "SA_PASSWORD=<YourStrong!Passw0rd>" -p 1433:1433 --name sql1 -d mcr.microsoft.com/mssql/server:2017-latest-ubuntu
let connection = @"Data Source=localhost,1433;Database=master;User=sa;Password=<YourStrong!Passw0rd>;";

use db = new SqlConnection(connection)
db.Open()

// OK, same order
let res = db.QuerySingle<{|Name: string; Tid: int; X:string; Y: int|}>("select Name='fdsf', Tid=20, X='hfds', Y=15") 

// fails but understandable, different order
let res = db.QuerySingle<{|Name: string; Tid: int; X:string; Y: int|}>("select Tid=20, Name='fdsf', X='hfds', Y=15")

// fails and unexpected, same order
let res = db.QuerySingle<{|Tid: int; Name: string; X:string; Y: int|}>("select Tid=20, Name='fdsf', X='hfds', Y=15")  

the error is cryptic too, but explain it doesnt find the constructor with right shape

Unhandled Exception: System.InvalidOperationException: A parameterless default constructor or one matching signature (System.Int32 Tid, System.String Name, System.String X, System.Int32 Y) is required for <>f__AnonymousType1611086028`4'[[System.String, System.Private.CoreLib, Version=4.0.0.0, Culture=neutral, PublicKeyToken=7cec85d7bea7798e],[System.Int32, System.Private.CoreLib, Version=4.0.0.0, Culture=neutral, PublicKeyToken=7cec85d7bea7798e],[System.String, System.Private.CoreLib, Version=4.0.0.0, Culture=neutral, PublicKeyToken=7cec85d7bea7798e],[System.Int32, System.Private.CoreLib, Version=4.0.0.0, Culture=neutral, PublicKeyToken=7cec85d7bea7798e]] materialization

Repro steps

Use the snippet above.

We can also compare C# and F#

// F#
let x = {| Tid = 20; Name = "fdsf"; X = "hfds"; Y = 15 |}

// IL
IL_00ea: ldstr        "fdsf"
IL_00ef: ldc.i4.s     20 // 0x14
IL_00f1: ldstr        "hfds"
IL_00f6: ldc.i4.s     15 // 0x0f
IL_00f8: newobj       instance void class '<>f__AnonymousType1611086028`4\''<string, int32, string, int32>::.ctor(!0/*string*/, !1/*int32*/, !2/*string*/, !3/*int32*/)

while C#

// C#
var x = new { Tid = 20, Name = "fdsf", X = "hfds", Y = 15 };

// IL
IL_0001: ldc.i4.s     20 // 0x14
IL_0003: ldstr        "fdsf"
IL_0008: ldstr        "hfds"
IL_000d: ldc.i4.s     15 // 0x0f
IL_000f: newobj       instance void class '<>f__AnonymousType0`4'<int32, string, string, int32>::.ctor(!0/*int32*/, !1/*string*/, !2/*string*/, !3/*int32*/)```

Expected behavior

The generated IL respect order of declaration

Actual behavior

The generated IL sort the field names

Known workarounds

None

Related information

> dotnet --info
.NET Core SDK (reflecting any global.json):
 Version:   3.0.100-preview3-010431
 Commit:    d72abce213

Runtime Environment:
 OS Name:     Windows
 OS Version:  10.0.17134
 OS Platform: Windows
 RID:         win10-x64

Host (useful for support):
  Version: 3.0.0-preview3-27503-5
  Commit:  3844df9537
@dsyme
Copy link
Contributor

dsyme commented Apr 3, 2019

This cause issues when libraries expect the same order of declaration. An example in dapper

Ouch, I didn't expect there would be C# libraries requiring a sorted field ordering. But of course there are. Bzzzz.

The RFC says sorting is used and it is relevant to the type checking process, see https://github.com/fsharp/fslang-design/blob/master/FSharp-4.6/FS-1030-anonymous-records.md#alternative-do-not-sort-fields-by-name

I'm not quite sure what to do. This has to be a feature extension I think. One option would be something like this if we can find a syntax that we want. The other option is to take a breaking change under a language version switch.

{| unsorted Name: string; Tid: int; X:string; Y: int|}

@cartermp
Copy link
Contributor

cartermp commented Apr 3, 2019

@dsyme I think it's worth expounding upon this in the RFC:

Sorting by field name is the natural thing for the programmer from a type-system usability perspective.

What is more usable?

@dsyme
Copy link
Contributor

dsyme commented Apr 3, 2019

What is more usable?

With F# records, order doesn't matter. You just do { Y=3; X=3 } and the compiler evaluates in order then permutes to create the object, and at another creation site you can just do { X=4; Y=4 }. Placing the assignments in order is, I think, less usable (as it would be for named arguments).

Regardless of usability, if differently ordered anonymous types are regarded as different incompatible types then that increases the divergence of the feature from F# records.

@forki
Copy link
Contributor

forki commented Apr 3, 2019 via email

@cartermp
Copy link
Contributor

cartermp commented Apr 3, 2019

I think we could file an issue and see what they think

@dsyme
Copy link
Contributor

dsyme commented Apr 3, 2019

Column order is important in SQL, I get why they do this.

I really do suspect there is a use case for order-sensitive anonymous records. It's just one of those things that plays both ways

@smoothdeveloper
Copy link
Contributor

smoothdeveloper commented Apr 3, 2019

what about considering a fix to dapper?

Most .net and sql stuff is generally reliably on resolving column indices / names in case resultset doesn't come with same order, it really looks like a Dapper shortcoming to me.

As the feature work as specified in the RFC, and I believe this is impacting type unification with current implementation details, we should postpone changes for current release and keep what's working now.

The anonymous records are really convenient for singleton values and replacing tuples already.

In future, maybe this can be relaxed in non breaking way, I like the unsorted idea, should we try gathering more input on this issue from community in fsharp/fslang-design#170 or here?

The first point:

var foos = new[]
{
    new {a = 1, b = 'c'},
    new {b = 'd', a = 2},
};

error CS0826: No best type found for implicitly-typed array

Do you want F# to behave same as C# on this regard?

I actually like that the order doesn't matter, otherwise at construction it is inconsistant with F# record type and would (could?) produce different records.

What is more usable?

I'm not sure there is a definitive or non subjective answer, F# anonymous records are more usable than C# anonymous objects, they can get exposed without reflection outside local scope and work with type inference the F# way (+ they allow slight typo in order of fields :D )

@enricosada
Copy link
Contributor Author

enricosada commented Apr 3, 2019

My bad, it's a new feature and i missed in the RFC part about ordering.

It was just strange, the following code seems valid and i didnt understood the why about the F#/C# difference.

let res = db.QuerySingle< {| Tid:int; Name:string; X:string; Y:int |} >("select Tid=20, Name='fdsf', X='hfds', Y=15")  

Now make sense and i prefer the F# behaviour.

Yes is dapper who should have checked the parameters names too instead of just signature maybe (i'll open an issue and send a PR).
With anonymous type is strange, because the constructor generated will accept the parameters in definition order.

I expected to be something who may show in other libs (or create friction), that's why i opened the issue.

Having an unsorted way may be nice to work with column oriented structures without names, but is another RFC, not this one

I'll close, make sense by design

@mdpedersen
Copy link

mdpedersen commented Apr 1, 2021

For anybody else coming across this in the context of Dapper: Dapper.FSharp happens to work around this issue of ordering by using reflection on the result type when constructing projection clauses. The resulting ordering of columns in projection clauses then matches the ordering which Dapper uses for decoding results (also based on reflection on the result type).

@callmekohei
Copy link

workaround

fix query ( put in alphabetical order )

select t.aaa, t.bbb, t.ccc from foo t 

anonymous record label's order is free (。・ω・。)ノ

con.Query<{|ccc:string;bbb:string;aaa:string|}>(sql=sql)

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

7 participants