Fix multiple item upsert with identity columns (#247)

* fix logic

* Add test

* Add justification

* Add new function

Author: Charles-Gagnon

samples/samples-csharp/GlobalSuppressions.cs

@@ -21,4 +21,5 @@
 [assembly: SuppressMessage("Style", "IDE0060:Remove unused parameter", Justification = "Unused parameter is required by functions binding", Scope = "member", Target = "~M:Microsoft.Azure.WebJobs.Extensions.Sql.Samples.OutputBindingSamples.TimerTriggerProducts.Run(Microsoft.Azure.WebJobs.TimerInfo,Microsoft.Extensions.Logging.ILogger,Microsoft.Azure.WebJobs.ICollector{Microsoft.Azure.WebJobs.Extensions.Sql.Samples.Common.Product})")]
 [assembly: SuppressMessage("Style", "IDE0060:Remove unused parameter", Justification = "Unused parameter is required by functions binding", Scope = "member", Target = "~M:Microsoft.Azure.WebJobs.Extensions.Sql.Samples.InputBindingSamples.GetProducts.Run(Microsoft.AspNetCore.Http.HttpRequest,System.Collections.Generic.IEnumerable{Microsoft.Azure.WebJobs.Extensions.Sql.Samples.Common.Product})~Microsoft.AspNetCore.Mvc.IActionResult")]
 [assembly: SuppressMessage("Style", "IDE0060:Remove unused parameter", Justification = "Unused parameter is required by functions binding", Scope = "member", Target = "~M:Microsoft.Azure.WebJobs.Extensions.Sql.Samples.InputBindingSamples.GetProductNamesView.Run(Microsoft.AspNetCore.Http.HttpRequest,System.Collections.Generic.IEnumerable{Microsoft.Azure.WebJobs.Extensions.Sql.Samples.Common.ProductName})~Microsoft.AspNetCore.Mvc.IActionResult")]
-[assembly: SuppressMessage("Style", "IDE0060:Remove unused parameter", Justification = "Unused parameter is required by functions binding", Scope = "member", Target = "~M:Microsoft.Azure.WebJobs.Extensions.Sql.Samples.InputBindingSamples.GetProductsStoredProcedureFromAppSetting.Run(Microsoft.AspNetCore.Http.HttpRequest,System.Collections.Generic.IEnumerable{Microsoft.Azure.WebJobs.Extensions.Sql.Samples.Common.Product})~Microsoft.AspNetCore.Mvc.IActionResult")]
+[assembly: SuppressMessage("Style", "IDE0060:Remove unused parameter", Justification = "Unused parameter is required by functions binding", Scope = "member", Target = "~M:Microsoft.Azure.WebJobs.Extensions.Sql.Samples.InputBindingSamples.GetProductsStoredProcedureFromAppSetting.Run(Microsoft.AspNetCore.Http.HttpRequest,System.Collections.Generic.IEnumerable{Microsoft.Azure.WebJobs.Extensions.Sql.Samples.Common.Product})~Microsoft.AspNetCore.Mvc.IActionResult")]
+[assembly: SuppressMessage("Style", "IDE0060:Remove unused parameter", Justification = "Unused parameter is required by functions binding", Scope = "member", Target = "~M:Microsoft.Azure.WebJobs.Extensions.Sql.Samples.OutputBindingSamples.AddProductsWithIdentityColumnArray.Run(Microsoft.AspNetCore.Http.HttpRequest,Microsoft.Azure.WebJobs.Extensions.Sql.Samples.OutputBindingSamples.ProductWithoutId[]@)~Microsoft.AspNetCore.Mvc.IActionResult")]

samples/samples-csharp/OutputBindingSamples/AddProductsWithIdentityColumnArray.cs

@@ -0,0 +1,43 @@
+// Copyright (c) Microsoft Corporation. All rights reserved.
+// Licensed under the MIT License. See License.txt in the project root for license information.
+
+using Microsoft.AspNetCore.Http;
+using Microsoft.AspNetCore.Mvc;
+using Microsoft.Azure.WebJobs.Extensions.Http;
+
+namespace Microsoft.Azure.WebJobs.Extensions.Sql.Samples.OutputBindingSamples
+{
+    public static class AddProductsWithIdentityColumnArray
+    {
+        /// <summary>
+        /// This shows an example of a SQL Output binding where the target table has a primary key
+        /// which is an identity column. In such a case the primary key is not required to be in
+        /// the object used by the binding - it will insert a row with the other values and the
+        /// ID will be generated upon insert.
+        /// </summary>
+        /// <param name="req">The original request that triggered the function</param>
+        /// <param name="product">The created Product object</param>
+        /// <returns>The CreatedResult containing the new object that was inserted</returns>
+        [FunctionName(nameof(AddProductsWithIdentityColumnArray))]
+        public static IActionResult Run(
+            [HttpTrigger(AuthorizationLevel.Anonymous, "get", "post")]
+            HttpRequest req,
+            [Sql("dbo.ProductsWithIdentity", ConnectionStringSetting = "SqlConnectionString")] out ProductWithoutId[] products)
+        {
+            products = new[]
+            {
+                new ProductWithoutId
+                {
+                    Name = "Cup",
+                    Cost = 2
+                },
+                new ProductWithoutId
+                {
+                    Name = "Glasses",
+                    Cost = 12
+                }
+            };
+            return new CreatedResult($"/api/addproductswithidentitycolumnarray", products);
+        }
+    }
+}

src/SqlAsyncCollector.cs

@@ -290,26 +290,36 @@ private static void GenerateDataQueryForMerge(TableInformation table, IEnumerabl
             {
                 if (typeof(T) != typeof(JObject))
                 {
-                    // SQL Server allows 900 bytes per primary key, so use that as a baseline
-                    var combinedPrimaryKey = new StringBuilder(900 * table.PrimaryKeys.Count());
-
-                    // Look up primary key of T. Because we're going in the same order of fields every time,
-                    // we can assume that if two rows with the same primary key are in the list, they will collide
-                    foreach (PropertyInfo primaryKey in table.PrimaryKeys)
+                    if (table.HasIdentityColumnPrimaryKeys)
                     {
-                        object value = primaryKey.GetValue(row);
-                        // Identity columns are allowed to be optional, so just skip the key if it doesn't exist
-                        if (value == null)
-                        {
-                            continue;
-                        }
-                        combinedPrimaryKey.Append(value.ToString());
+                        // If the table has an identity column as a primary key then
+                        // all rows are guaranteed to be unique so we can insert them all
+                        rowsToUpsert.Add(row);
                     }
-                    // If we have already seen this unique primary key, skip this update
-                    if (uniqueUpdatedPrimaryKeys.Add(combinedPrimaryKey.ToString()))
+                    else
                     {
-                        // This is the first time we've seen this particular PK. Add this row to the upsert query.
-                        rowsToUpsert.Add(row);
+                        // SQL Server allows 900 bytes per primary key, so use that as a baseline
+                        var combinedPrimaryKey = new StringBuilder(900 * table.PrimaryKeys.Count());
+                        // Look up primary key of T. Because we're going in the same order of fields every time,
+                        // we can assume that if two rows with the same primary key are in the list, they will collide
+                        foreach (PropertyInfo primaryKey in table.PrimaryKeys)
+                        {
+                            object value = primaryKey.GetValue(row);
+                            // Identity columns are allowed to be optional, so just skip the key if it doesn't exist
+                            if (value == null)
+                            {
+                                continue;
+                            }
+                            combinedPrimaryKey.Append(value.ToString());
+                        }
+                        string combinedPrimaryKeyStr = combinedPrimaryKey.ToString();
+                        // If we have already seen this unique primary key, skip this update
+                        // If the combined key is empty that means
+                        if (uniqueUpdatedPrimaryKeys.Add(combinedPrimaryKeyStr))
+                        {
+                            // This is the first time we've seen this particular PK. Add this row to the upsert query.
+                            rowsToUpsert.Add(row);
+                        }
                     }
                 }
                 else
@@ -358,18 +368,23 @@ public class TableInformation
             /// </summary>
             public string Query { get; }
 
+            /// <summary>
+            /// Whether at least one of the primary keys on this table is an identity column
+            /// </summary>
+            public bool HasIdentityColumnPrimaryKeys { get; }
             /// <summary>
             /// Settings to use when serializing the POCO into SQL.
             /// Only serialize properties and fields that correspond to SQL columns.
             /// </summary>
             public JsonSerializerSettings JsonSerializerSettings { get; }
 
-            public TableInformation(IEnumerable<MemberInfo> primaryKeys, IDictionary<string, string> columns, StringComparer comparer, string query)
+            public TableInformation(IEnumerable<MemberInfo> primaryKeys, IDictionary<string, string> columns, StringComparer comparer, string query, bool hasIdentityColumnPrimaryKeys)
             {
                 this.PrimaryKeys = primaryKeys;
                 this.Columns = columns;
                 this.Comparer = comparer;
                 this.Query = query;
+                this.HasIdentityColumnPrimaryKeys = hasIdentityColumnPrimaryKeys;
 
                 this.JsonSerializerSettings = new JsonSerializerSettings
                 {
@@ -618,7 +633,7 @@ public static async Task<TableInformation> RetrieveTableInformationAsync(SqlConn
                 sqlConnProps.Add(TelemetryPropertyName.QueryType.ToString(), usingInsertQuery ? "insert" : "merge");
                 sqlConnProps.Add(TelemetryPropertyName.HasIdentityColumn.ToString(), hasIdentityColumnPrimaryKeys.ToString());
                 TelemetryInstance.TrackDuration(TelemetryEventName.GetTableInfoEnd, tableInfoSw.ElapsedMilliseconds, sqlConnProps, durations);
-                return new TableInformation(primaryKeyFields, columnDefinitionsFromSQL, comparer, query);
+                return new TableInformation(primaryKeyFields, columnDefinitionsFromSQL, comparer, query, hasIdentityColumnPrimaryKeys);
             }
         }
 

test/Integration/SqlOutputBindingIntegrationTests.cs

@@ -173,6 +173,19 @@ public void AddProductWithIdentity()
             Assert.Equal(1, this.ExecuteScalar("SELECT COUNT(*) FROM dbo.ProductsWithIdentity"));
         }
 
+        /// <summary>
+        /// Tests that for tables with an identity column we are able to insert multiple items at once
+        /// </summary>
+        [Fact]
+        public void AddProductsWithIdentityColumnArray()
+        {
+            this.StartFunctionHost(nameof(AddProductsWithIdentityColumnArray));
+            Assert.Equal(0, this.ExecuteScalar("SELECT COUNT(*) FROM dbo.ProductsWithIdentity"));
+            this.SendOutputRequest(nameof(AddProductsWithIdentityColumnArray)).Wait();
+            // Multiple items should have been inserted
+            Assert.Equal(2, this.ExecuteScalar("SELECT COUNT(*) FROM dbo.ProductsWithIdentity"));
+        }
+
         /// <summary>
         /// Tests that for tables with multiple primary columns (including an itemtity column) we are able to
         /// insert items.