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

[BUG] Query failed when field mapping used on FSharp records. #662

Closed
DmitryBatalov opened this issue Dec 4, 2020 · 9 comments
Closed
Assignees
Labels
bug Something isn't working deployed Feature or bug is deployed at the current release fixed The bug, issue, incident has been fixed.

Comments

@DmitryBatalov
Copy link

DmitryBatalov commented Dec 4, 2020

The model in the code doesn't match the database. I use the mapping feature in the following way:

type User = { 
  Id: int
  [<Map("first_name")>]
  Name: string 
}

and then try to read data from database

MySqlBootstrap.Initialize()
use connection = new MySqlConnection("Server=...")
connection.QueryAll<User>()

As a result, I've got the exception:

Unhandled exception. System.InvalidOperationException: Compiler: Failed to initialize the member properties or the constructor arguments.
 ---> System.ArgumentException: Incorrect number of arguments for constructor
   at System.Dynamic.Utils.ExpressionUtils.ValidateArgumentCount(MethodBase method, ExpressionType nodeKind, Int32 count, ParameterInfo[] pis)
   at System.Dynamic.Utils.ExpressionUtils.ValidateArgumentTypes(MethodBase method, ExpressionType nodeKind, ReadOnlyCollection`1& arguments, String methodParamName)
   at System.Linq.Expressions.Expression.New(ConstructorInfo constructor, IEnumerable`1 arguments)
   at RepoDb.Reflection.Compiler.CompileDataReaderToDataEntity[TResult](DbDataReader reader, IEnumerable`1 dbFields, IDbSetting dbSetting)
   --- End of inner exception stack trace ---
   at RepoDb.Reflection.Compiler.CompileDataReaderToDataEntity[TResult](DbDataReader reader, IEnumerable`1 dbFields, IDbSetting dbSetting)
   at RepoDb.Reflection.Compiler.CompileDataReaderToType[TResult](DbDataReader reader, IEnumerable`1 dbFields, IDbSetting dbSetting)
   at RepoDb.Reflection.FunctionFactory.CompileDataReaderToType[TResult](DbDataReader reader, IEnumerable`1 dbFields, IDbSetting dbSetting)
   at RepoDb.FunctionCache.DataReaderToTypeCache`1.Get(DbDataReader reader, IEnumerable`1 dbFields, IDbSetting dbSetting)
   at RepoDb.FunctionCache.GetDataReaderToTypeCompiledFunction[TResult](DbDataReader reader, IEnumerable`1 dbFields, IDbSetting dbSetting)
   at RepoDb.Reflection.DataReader.ToEnumerable[TResult](DbDataReader reader, IEnumerable`1 dbFields, IDbSetting dbSetting)+MoveNext()
   at System.Collections.Generic.List`1..ctor(IEnumerable`1 collection)
   at System.Linq.Enumerable.ToList[TSource](IEnumerable`1 source)
   at RepoDb.Extensions.EnumerableExtension.AsList[T](IEnumerable`1 value)
   at RepoDb.DbConnectionExtension.ExecuteQueryInternalForType[TResult](IDbConnection connection, String commandText, Object param, Nullable`1 commandType, String cacheKey, Nullable`1 cacheItemExpiration, Nullable`1 commandTimeout, IDbTransaction transaction, ICache cache, String tableName, Boolean skipCommandArrayParametersCheck)
   at RepoDb.DbConnectionExtension.ExecuteQueryInternal[TResult](IDbConnection connection, String commandText, Object param, Nullable`1 commandType, String cacheKey, Nullable`1 cacheItemExpiration, Nullable`1 commandTimeout, IDbTransaction transaction, ICache cache, String tableName, Boolean skipCommandArrayParametersCheck)
   at RepoDb.DbConnectionExtension.QueryAllInternalBase[TEntity](IDbConnection connection, String tableName, IEnumerable`1 fields, IEnumerable`1 orderBy, String hints, String cacheKey, Nullable`1 cacheItemExpiration, Nullable`1 commandTimeout, IDbTransaction transaction, ICache cache, ITrace trace, IStatementBuilder statementBuilder)
   at RepoDb.DbConnectionExtension.QueryAllInternal[TEntity](IDbConnection connection, String tableName, IEnumerable`1 fields, IEnumerable`1 orderBy, String hints, String cacheKey, Nullable`1 cacheItemExpiration, Nullable`1 commandTimeout, IDbTransaction transaction, ICache cache, ITrace trace, IStatementBuilder statementBuilder)
   at RepoDb.DbConnectionExtension.QueryAll[TEntity](IDbConnection connection, IEnumerable`1 fields, IEnumerable`1 orderBy, String hints, String cacheKey, Nullable`1 cacheItemExpiration, Nullable`1 commandTimeout, IDbTransaction transaction, ICache cache, ITrace trace, IStatementBuilder statementBuilder)

When the model match DB, then I can read without any issues.
Also, insert operation work with mapping without any issue.

Environment: ubuntu 20.04, .net core 3.1
Version: RepoDb.MySql 1.1.2
Database: MySql 8.0
Example: consoleapp.zip

@mikependon mikependon self-assigned this Dec 4, 2020
@mikependon mikependon added the bug Something isn't working label Dec 4, 2020
@mikependon
Copy link
Owner

This is a good finding actually. In C#, the type that was created in F# is automatically converted into Anonymous Type, that is immutable. RepoDB on the other hand only supports the mapping in both the Class and Property level, not on the ctor arguments. I would assume that the issue is on the alignment of the ctor argument, that is why push operations (i.e: Insert, etc) is working.

Can you try putting [CLIMutable] attribute only in this type if that works? Or, hopefully you have any other alternative.

Also, when do you need the fix for this?

Thank you for this report.

@DmitryBatalov
Copy link
Author

DmitryBatalov commented Dec 4, 2020

@mikependon Thanks for the quick response.

Adding CLIMutable didn't help, unfortunately.

I've created a tiny F# example generated into C#.

In case of the link above doesn't work (click to expand)

go to https://sharplab.io/ and replace F# code with code below

open System

type MapAttribute(name: string) = 
    inherit Attribute()
   
//[<CLIMutable>]
type User = {
    Id: int
    [<Map("first_name")>]
    Name: string
}

As you can see, the result will be C# class with a constructor and 2 read-only properties. The Name property tagged with Map attribute.

....
public User(int id, string name)
{
    Id@ = id;
    Name@ = name;
}

[CompilationMapping(SourceConstructFlags.Field, 0)]
public int Id
{
    get
    {
        return Id@;
    }
}

[Map("first_name")]
[CompilationMapping(SourceConstructFlags.Field, 1)]
public string Name
{
    get
    {
        return Name@;
    }
}
....

So, it looks OK.
Also, tried with Fluent configurator, it didn't help either.

As of timing, I'm not in hurry :)

@mikependon
Copy link
Owner

Oh, I did the same thing, never expect that it will generate 2 ctors at all if it is converted to C# (new knowledge).

public User(int id, string name)
{
	Id@ = id;
	Name@ = name;
}

public User()
{
}

Yes, that is what I am saying, the type User is expecting name ctor argument in which it is actually a first_name in the database. Therefore it fails as it has eliminated that argument after the equality/comparison.

This could as well complicate the things since the converted object in C# has placed the Map attribute into the property, and not on the ctor argument which I would expect.

If you are a C# developer, how would you prefer it to do?

Would you place it on the ctor argument (like below)? On this case, Map attribute would support 3 level (Class, Property and Ctor Argument).

public class User
{
	public User(int id, [Map("first_name")] string name)
	{
		Id = id;
		Name = name;
	}
	public int Id { get; set; }
	public string Name { get; set; }
}

Or, would you rather just put in on the property.

public class User
{
	public User(int id, string name)
	{
		Id = id;
		Name = name;
	}
	public int Id { get; set; }
	[Map("first_name")]
	public string Name { get; set; }
}

If you cannot provide an input, please feel free to ask an opinion from the other C# users, so this would remove my bias.

I recommend the first approach though.

@DmitryBatalov
Copy link
Author

@mikependon How does it work currently when a property tagged with Map attribute?

I would vote for the second one, but I'm not into the actual implementation and it is hard for me to predict the impact. What is the next step do you see? How I can help you with progress here?

@mikependon
Copy link
Owner

For the immutable types, the ctor argument is not auto-projecting to the property name equality. In C#, you can make a ctor argument of namewhere the property isCompleteName`. That is why I also did not project that. See below.

public class User
{
	public User(int id, string name)
	{
		Id = id;
		CompleteName = name;
	}
	public int Id { get; set; }
	public string CompleteName { get; set; }
}

If your class is mutable, then it works as expected as you mentioned. The problem to your case is that, the name ctor argument is not equals to the first_name field name from your DB, thus has been eliminated on the hydration process, throwing such exception.

The option #2 is also good, all that I have to do is to implement the ctor argument and property projection. The problem to this is, if they are not equal on the name, then the same exception will throw.

@mikependon mikependon pinned this issue Dec 4, 2020
@mikependon
Copy link
Owner

@DmitryBatalov - but since the F# is always generating the equal ctor argument and property (both name and type), then it is safe to assume the option 2. The option 1 is an additional level of mapping in which I think would add further big changes to the library for such a small case.

I decided to also go for option 2 (as of writing this).

@mikependon
Copy link
Owner

I am a bit annoyed that the PR is closing this issue even it is not linked. Anyway, I had reopened that and had Tweeted the Github for that.

I had added the Integration Tests (the model can be found here and the test can be found here).

@mikependon mikependon added the fixed The bug, issue, incident has been fixed. label Dec 4, 2020
@mikependon mikependon unpinned this issue Dec 6, 2020
@DmitryBatalov
Copy link
Author

Thanks for the quick fix. 🥇

@mikependon
Copy link
Owner

This is now available on version RepoDb v1.12.5-beta5.

@mikependon mikependon added the deployed Feature or bug is deployed at the current release label Jan 13, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working deployed Feature or bug is deployed at the current release fixed The bug, issue, incident has been fixed.
Projects
None yet
Development

No branches or pull requests

2 participants