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

Suggested Improvement to handling Insert with Db-generated primary keys #136

Open
EdJohnstonG9 opened this issue Apr 30, 2022 · 0 comments

Comments

@EdJohnstonG9
Copy link

EdJohnstonG9 commented Apr 30, 2022

This addresses 3 issues for Insert:

  1. Use of GUID as a Db generated PK
  2. Handling a Pk field that is not named Id
  3. Returning the Pk values with the original insert object for Multi and Single Insert (GUID or Int and any name)

The suggested handling below is working examples but only for SQL Server Sync methods, though this can be extended to Async SQL Server for sure. Other's expertise would be needed for other supported databases.

The following is for single entity Insert statements that will include the updated PK in the return record, whether it is of type Int or Guid or if it is named other than Id (i.e. UserId rather than simply Id).

Ive left the return value = 1 if there is a GUID as the Pk but ideally it would change to dynamic and return the GUID if one is present. Returning OUTPUT null is there just to minimise other code changes.

public int Insert(IDbConnection connection, IDbTransaction transaction, int? commandTimeout, string tableName, string columnList, string parameterList, IEnumerable<PropertyInfo> keyProperties, object entityToInsert)
    {
        string outClause = "";
        var propertyInfos = keyProperties as PropertyInfo[] ?? keyProperties.ToArray();
        if (propertyInfos.Length > 0)
        {
            outClause = $"OUTPUT INSERTED.{propertyInfos[0].Name} as autoId";
        }
        else
        {
            outClause = $"OUTPUT null as autoId";
        }
        var cmd = $"insert into {tableName} ({columnList}) {outClause} values ({parameterList})";
        var multi = connection.QueryMultiple(cmd, entityToInsert, transaction, commandTimeout);

        var first = multi.Read().FirstOrDefault();
        if (first == null || first.autoId == null) return 0;

        if (propertyInfos.Length > 0)
        {
            propertyInfos[0].SetValue(entityToInsert, Convert.ChangeType(first.autoId, propertyInfos[0].PropertyType), null);
            if (propertyInfos[0].PropertyType == typeof(int))
                return (int)first.autoId;
        }
        return 1;
    }

The suggestion below for multiple objects needs improvement, but I ran out of knowledge/understanding of the excellent Dapper codebase!

Also I am not entirely happy with the approach, but I think the principle is sound, at least for SQL Server:

  1. Instead of parameterised query, create an insert query with literal values and execute as Query
  2. Use the OUTPUT clause to return the Pk values Inserted
  3. Push the values back into the original Pk fields of the returned object

The things that make me unhappy:

  • Not using Parameters as you do throughout the rest of the codebase.
  • Converting parameter values to literal values is done crudely here for proof of concept
  • Using Select (work round) rather than ExecuteScalar (logically the correct call)
  • This also un-caches the result as return values must be enumerated within the Insert command.
        public static long Insert<T>(this IDbConnection connection, T entityToInsert, IDbTransaction transaction = null, int? commandTimeout = null) where T : class
        {
            var isList = false;

            var type = typeof(T);

            if (type.IsArray)
            {
                isList = true;
                type = type.GetElementType();
            }
            else if (type.IsGenericType)
            {
                var typeInfo = type.GetTypeInfo();
                bool implementsGenericIEnumerableOrIsGenericIEnumerable =
                    typeInfo.ImplementedInterfaces.Any(ti => ti.IsGenericType && ti.GetGenericTypeDefinition() == typeof(IEnumerable<>)) ||
                    typeInfo.GetGenericTypeDefinition() == typeof(IEnumerable<>);

                if (implementsGenericIEnumerableOrIsGenericIEnumerable)
                {
                    isList = true;
                    type = type.GetGenericArguments()[0];
                }
            }

            var name = GetTableName(type);
            var sbColumnList = new StringBuilder(null);
            var sbKeyList = new StringBuilder(null);
            var allProperties = TypePropertiesCache(type);
            var keyProperties = KeyPropertiesCache(type);
            var computedProperties = ComputedPropertiesCache(type);
            var allPropertiesExceptKeyAndComputed = allProperties.Except(keyProperties.Union(computedProperties)).ToList();

            var adapter = GetFormatter(connection);

            for (var i = 0; i < allPropertiesExceptKeyAndComputed.Count; i++)
            {
                var property = allPropertiesExceptKeyAndComputed[i];
                adapter.AppendColumnName(sbColumnList, property.Name);  //fix for issue #336
                if (i < allPropertiesExceptKeyAndComputed.Count - 1)
                    sbColumnList.Append(", ");
            }

            var sbParameterList = new StringBuilder(null);
            for (var i = 0; i < allPropertiesExceptKeyAndComputed.Count; i++)
            {
                var property = allPropertiesExceptKeyAndComputed[i];
                sbParameterList.AppendFormat("@{0}", property.Name);
                if (i < allPropertiesExceptKeyAndComputed.Count - 1)
                    sbParameterList.Append(", ");
            }

            if (keyProperties.Count > 0) //why would you have 2??
            {
                sbKeyList.Append("OUTPUT INSERTED.");
                adapter.AppendColumnName(sbKeyList, keyProperties[0].Name);
                sbKeyList.Append(" as autoId");
            }

            int returnVal;
            var wasClosed = connection.State == ConnectionState.Closed;
            if (wasClosed) connection.Open();

            if (!isList)    //single entity
            {
                returnVal = adapter.Insert(connection, transaction, commandTimeout, name, sbColumnList.ToString(),
                    sbParameterList.ToString(), keyProperties, entityToInsert);
            }
            else
            {
                //Generate a list of literal Values rather than Parameters
                string values = "VALUES ";
                var toInsertArr = (entityToInsert as IEnumerable<object>).ToArray();
                foreach (var item in toInsertArr)
                {
                    string toAdd = "";
                    for (var i = 0; i < allPropertiesExceptKeyAndComputed.Count; i++)
                    {
                        var property = allPropertiesExceptKeyAndComputed[i];
                        var val = property.GetValue(item);
                        //For sure there will be a better way to do this somewhere in Dapper!
                        toAdd += $"{quotedVal(val)}, ";
                    }
                    toAdd = $"\n({toAdd.TrimEnd(", ".ToCharArray())}),";
                    values += toAdd;
                }
                values = values.TrimEnd(',');

                var cmd = $"insert into {name} ({sbColumnList}) {sbKeyList} {values}";


                //insert list of entities and return Enumerable of key values
                var output = connection.Query(cmd, null, transaction, false, commandTimeout, CommandType.Text);

                var outputArr = output.ToArray();
                returnVal = outputArr.Count();

                if (returnVal == toInsertArr.Length)
                {
                    for (int i = 0; i < outputArr.Count(); i++)
                    {
                        foreach (var key in keyProperties)
                        {
                            var keyVal = outputArr[i].autoId;
                            key.SetValue(toInsertArr[i], keyVal);
                        }
                    }
                }
            }
            if (wasClosed) connection.Close();
            return returnVal;
        }

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

1 participant