Dapper.Contrib cannot handle GUID primary keys & SqlMapperExtensions.Insert<T> has wrong usage int & long

[SwissMaWi]

SqlMapperExtensions.Insert will fail when numeric db keys reach the Int32 limit (yes, this happens on high traffic dbs) and also fails completely since it assumes primary keys are integers. If primary keys are not integers (i.e. GUIDs or something else) the library fails completely.
Is there any interest for an improvement? I have some proposition in mind.

`        /// <summary>
        /// Inserts an entity into table "Ts" and returns identity id or number of inserted rows if inserting a list.
        /// </summary>
        /// <typeparam name="T">The type to insert.</typeparam>
        /// <param name="connection">Open SqlConnection</param>
        /// <param name="entityToInsert">Entity to insert, can be list of entities</param>
        /// <param name="transaction">The transaction to run under, null (the default) if none</param>
        /// <param name="commandTimeout">Number of seconds before command execution timeout</param>
        /// **<returns>Identity of inserted entity, or number of inserted rows if inserting a list</returns>**
        public static **long** Insert<T>(this IDbConnection connection, T entityToInsert, IDbTransaction transaction = null, int? commandTimeout = null) where T : class
`

[maulik-modi]

@SwissMaWi , You are right it indeed has a limitation. Could you please format the code?

@mgravell or @SamSaffron , your thoughts?

[SwissMaWi]

My proposition is to separate insertion of single rows and lists.
Then we will be able to remove the limitation if primary key types to int and make it generic or dynamic.
Primary keys can be anything of int, (n)varchar of any kind and of course GUIDs.
The latter is important for security to remove the possibility of key guessing.
The list insertion could then return a list of inserted primary keys.

[maulik-modi]

@SwissMaWi , Do you mean something like
https://github.com/tmsmith/Dapper-Extensions/blob/master/DapperExtensions/DapperImplementor.cs

I had to use DapperExtensions because it allows Fluent mapping in separate class as opposed to Attributes decorated on top of entities.

[dallasbeek]

This was my reason for not using this lib and creating my own. Check out dapper.database https://github.com/dallasbeek/Dapper.Database

[SwissMaWi]

@dallasbeek let's improve dapper... this is sustainable.
@NickCraver should I work out an improvement here?

[SwissMaWi]

Just tried to implement something with minimal changes, however it is not possible since if you do not use DB autoincrement INT keys, the pattern of primary key creation is entirely different. I think we have to separate the concern of auto incrementing PK from the concern of PK type first, before this can be implemented. No matter how, it will cause major changes in the INSERT workflow I believe. Regarding GUID keys: should the application create them or should Dapper offer to "autocreate" them?`I would need some opinions here...

[maulik-modi]

@SwissMaWi , EF Core has sequential GUID generator https://github.com/dotnet/efcore/blob/main/src/EFCore/ValueGeneration/SequentialGuidValueGenerator.cs

Also EF Core has a method or attribute for ValueGeneratedNever
https://www.learnentityframeworkcore.com/configuration/fluent-api/valuegeneratednever-method

[SwissMaWi]

@maulik-modi so in your opinion generating GUID keys should be the concern of Dapper.Contrib?
I am aware that .Net GUIDS are affine to SQL Server GUIDS but what about other DB systems?

[maulik-modi]

@SwissMaWi ,

1. Ability to opt-out from auto incremented Id/PK using explicit option ValueGeneratedNever or DatabaseGenerated:False
2. Allow to declare TId type, find record by TId
3. For inserting multiple, I doubt there is native support in ADO.NET to return back list of generated Id e.g. New System.Data.Common batching API dotnet/runtime#28633 is still open
4. GUID generation should not be kept in Dapper.contrib