Merge pull request #11369 from umbraco/v8/bugfix/maxparametercount

Paul Johnson · web-flow · commit c61eaa225789 · 2021-10-22T21:08:55.000+01:00
Add Constants.Sql.MaxParameterCount and update query logic
diff --git a/src/Umbraco.Core/Constants-Sql.cs b/src/Umbraco.Core/Constants-Sql.cs
@@ -0,0 +1,17 @@
+﻿namespace Umbraco.Core
+{
+    public static partial class Constants
+    {
+        public static class Sql
+        {
+            /// <summary>
+            /// The maximum amount of parameters that can be used in a query.
+            /// </summary>
+            /// <remarks>
+            /// The actual limit is 2100 (https://docs.microsoft.com/en-us/sql/sql-server/maximum-capacity-specifications-for-sql-server),
+            /// but we want to ensure there's room for additional parameters if this value is used to create groups/batches.
+            /// </remarks>
+            public const int MaxParameterCount = 2000;
+        }
+    }
+}
diff --git a/src/Umbraco.Core/Persistence/NPocoDatabaseExtensions-Bulk.cs b/src/Umbraco.Core/Persistence/NPocoDatabaseExtensions-Bulk.cs
@@ -137,7 +137,7 @@ internal static IDbCommand[] GenerateBulkInsertCommands<T>(this IUmbracoDatabase
             // Math.Floor(2100 / 8) = 262 record per command
             // 4168 / 262 = 15.908... = there will be 16 command in total
             // (if we have disabled db parameters, then all records will be included, in only one command)
-            var recordsPerCommand = paramsPerRecord == 0 ? int.MaxValue : Convert.ToInt32(Math.Floor(2000.00 / paramsPerRecord));
+            var recordsPerCommand = paramsPerRecord == 0 ? int.MaxValue : Convert.ToInt32(Math.Floor((double)Constants.Sql.MaxParameterCount / paramsPerRecord));
             var commandsCount = Convert.ToInt32(Math.Ceiling((double)records.Length / recordsPerCommand));
 
             var commands = new IDbCommand[commandsCount];
diff --git a/src/Umbraco.Core/Persistence/Repositories/Implement/AuditEntryRepository.cs b/src/Umbraco.Core/Persistence/Repositories/Implement/AuditEntryRepository.cs
@@ -54,7 +54,7 @@ protected override IEnumerable<IAuditEntry> PerformGetAll(params int[] ids)
 
             var entries = new List<IAuditEntry>();
 
-            foreach (var group in ids.InGroupsOf(2000))
+            foreach (var group in ids.InGroupsOf(Constants.Sql.MaxParameterCount))
             {
                 var sql = Sql()
                     .Select<AuditEntryDto>()
diff --git a/src/Umbraco.Core/Persistence/Repositories/Implement/ContentRepositoryBase.cs b/src/Umbraco.Core/Persistence/Repositories/Implement/ContentRepositoryBase.cs
@@ -638,7 +638,7 @@ protected IDictionary<int, PropertyCollection> GetPropertyCollections<T>(List<Te
             // in the table?
 
             // get all PropertyDataDto for all definitions / versions
-            var allPropertyDataDtos = Database.FetchByGroups<PropertyDataDto, int>(versions, 2000, batch =>
+            var allPropertyDataDtos = Database.FetchByGroups<PropertyDataDto, int>(versions, Constants.Sql.MaxParameterCount, batch =>
                 SqlContext.Sql()
                     .Select<PropertyDataDto>()
                     .From<PropertyDataDto>()
@@ -647,7 +647,7 @@ protected IDictionary<int, PropertyCollection> GetPropertyCollections<T>(List<Te
 
             // get PropertyDataDto distinct PropertyTypeDto
             var allPropertyTypeIds = allPropertyDataDtos.Select(x => x.PropertyTypeId).Distinct().ToList();
-            var allPropertyTypeDtos = Database.FetchByGroups<PropertyTypeDto, int>(allPropertyTypeIds, 2000, batch =>
+            var allPropertyTypeDtos = Database.FetchByGroups<PropertyTypeDto, int>(allPropertyTypeIds, Constants.Sql.MaxParameterCount, batch =>
                 SqlContext.Sql()
                     .Select<PropertyTypeDto>(r => r.Select(x => x.DataTypeDto))
                     .From<PropertyTypeDto>()
diff --git a/src/Umbraco.Core/Persistence/Repositories/Implement/ContentTypeRepositoryBase.cs b/src/Umbraco.Core/Persistence/Repositories/Implement/ContentTypeRepositoryBase.cs
@@ -768,7 +768,7 @@ private void CopyTagData(int? sourceLanguageId, int? targetLanguageId, IReadOnly
             // note: important to use SqlNullableEquals for nullable types, cannot directly compare language identifiers
 
             var whereInArgsCount = propertyTypeIds.Count + (contentTypeIds?.Count ?? 0);
-            if (whereInArgsCount > 2000)
+            if (whereInArgsCount > Constants.Sql.MaxParameterCount)
                 throw new NotSupportedException("Too many property/content types.");
 
             // delete existing relations (for target language)
@@ -906,7 +906,7 @@ private void CopyPropertyData(int? sourceLanguageId, int? targetLanguageId, IRea
             // note: important to use SqlNullableEquals for nullable types, cannot directly compare language identifiers
             //
             var whereInArgsCount = propertyTypeIds.Count + (contentTypeIds?.Count ?? 0);
-            if (whereInArgsCount > 2000)
+            if (whereInArgsCount > Constants.Sql.MaxParameterCount)
                 throw new NotSupportedException("Too many property/content types.");
 
             //first clear out any existing property data that might already exists under the target language
@@ -1005,7 +1005,7 @@ private void RenormalizeDocumentEditedFlags(IReadOnlyCollection<int> propertyTyp
             //based on the current variance of each item to see if it's 'edited' value should be true/false.
 
             var whereInArgsCount = propertyTypeIds.Count + (contentTypeIds?.Count ?? 0);
-            if (whereInArgsCount > 2000)
+            if (whereInArgsCount > Constants.Sql.MaxParameterCount)
                 throw new NotSupportedException("Too many property/content types.");
 
             var propertySql = Sql()
@@ -1094,14 +1094,20 @@ private void RenormalizeDocumentEditedFlags(IReadOnlyCollection<int> propertyTyp
                 }
             }
 
-            //lookup all matching rows in umbracoDocumentCultureVariation
-            var docCultureVariationsToUpdate = editedLanguageVersions.InGroupsOf(2000)
-                .SelectMany(_ => Database.Fetch<DocumentCultureVariationDto>(
-                    Sql().Select<DocumentCultureVariationDto>().From<DocumentCultureVariationDto>()
-                            .WhereIn<DocumentCultureVariationDto>(x => x.LanguageId, editedLanguageVersions.Keys.Select(x => x.langId).ToList())
-                            .WhereIn<DocumentCultureVariationDto>(x => x.NodeId, editedLanguageVersions.Keys.Select(x => x.nodeId))))
-                //convert to dictionary with the same key type
-                .ToDictionary(x => (x.NodeId, (int?)x.LanguageId), x => x);
+            // lookup all matching rows in umbracoDocumentCultureVariation
+            // fetch in batches to account for maximum parameter count (distinct languages can't exceed 2000)
+            var languageIds = editedLanguageVersions.Keys.Select(x => x.langId).Distinct().ToArray();
+            var nodeIds = editedLanguageVersions.Keys.Select(x => x.nodeId).Distinct();
+            var docCultureVariationsToUpdate = nodeIds.InGroupsOf(Constants.Sql.MaxParameterCount - languageIds.Length)
+                .SelectMany(group =>
+                {
+                    var sql = Sql().Select<DocumentCultureVariationDto>().From<DocumentCultureVariationDto>()
+                        .WhereIn<DocumentCultureVariationDto>(x => x.LanguageId, languageIds)
+                        .WhereIn<DocumentCultureVariationDto>(x => x.NodeId, group);
+
+                    return Database.Fetch<DocumentCultureVariationDto>(sql);
+                })
+                .ToDictionary(x => (x.NodeId, (int?)x.LanguageId), x => x); //convert to dictionary with the same key type
 
             var toUpdate = new List<DocumentCultureVariationDto>();
             foreach (var ev in editedLanguageVersions)
diff --git a/src/Umbraco.Core/Persistence/Repositories/Implement/DictionaryRepository.cs b/src/Umbraco.Core/Persistence/Repositories/Implement/DictionaryRepository.cs
@@ -260,13 +260,12 @@ public IEnumerable<IDictionaryItem> GetDictionaryItemDescendants(Guid? parentId)
 
             Func<Guid[], IEnumerable<IEnumerable<IDictionaryItem>>> getItemsFromParents = guids =>
             {
-                //needs to be in groups of 2000 because we are doing an IN clause and there's a max parameter count that can be used.
-                return guids.InGroupsOf(2000)
-                    .Select(@group =>
+                return guids.InGroupsOf(Constants.Sql.MaxParameterCount)
+                    .Select(group =>
                     {
                         var sqlClause = GetBaseQuery(false)
                             .Where<DictionaryDto>(x => x.Parent != null)
-                            .Where($"{SqlSyntax.GetQuotedColumnName("parent")} IN (@parentIds)", new { parentIds = @group });
+                            .WhereIn<DictionaryDto>(x => x.Parent, group);
 
                         var translator = new SqlTranslator<IDictionaryItem>(sqlClause, Query<IDictionaryItem>());
                         var sql = translator.Translate();
diff --git a/src/Umbraco.Core/Persistence/Repositories/Implement/DocumentRepository.cs b/src/Umbraco.Core/Persistence/Repositories/Implement/DocumentRepository.cs
@@ -1342,7 +1342,7 @@ private IDictionary<int, ContentScheduleCollection> GetContentSchedule(params in
         {
             var result = new Dictionary<int, ContentScheduleCollection>();
 
-            var scheduleDtos = Database.FetchByGroups<ContentScheduleDto, int>(contentIds, 2000, batch => Sql()
+            var scheduleDtos = Database.FetchByGroups<ContentScheduleDto, int>(contentIds, Constants.Sql.MaxParameterCount, batch => Sql()
                 .Select<ContentScheduleDto>()
                 .From<ContentScheduleDto>()
                 .WhereIn<ContentScheduleDto>(x => x.NodeId, batch));
@@ -1391,7 +1391,7 @@ private IDictionary<int, List<ContentVariation>> GetContentVariations<T>(List<Te
             }
             if (versions.Count == 0) return new Dictionary<int, List<ContentVariation>>();
 
-            var dtos = Database.FetchByGroups<ContentVersionCultureVariationDto, int>(versions, 2000, batch
+            var dtos = Database.FetchByGroups<ContentVersionCultureVariationDto, int>(versions, Constants.Sql.MaxParameterCount, batch
                 => Sql()
                     .Select<ContentVersionCultureVariationDto>()
                     .From<ContentVersionCultureVariationDto>()
@@ -1420,7 +1420,7 @@ private IDictionary<int, List<DocumentVariation>> GetDocumentVariations<T>(List<
         {
             var ids = temps.Select(x => x.Id);
 
-            var dtos = Database.FetchByGroups<DocumentCultureVariationDto, int>(ids, 2000, batch =>
+            var dtos = Database.FetchByGroups<DocumentCultureVariationDto, int>(ids, Constants.Sql.MaxParameterCount, batch =>
                 Sql()
                     .Select<DocumentCultureVariationDto>()
                     .From<DocumentCultureVariationDto>()
diff --git a/src/Umbraco.Core/Persistence/Repositories/Implement/EntityContainerRepository.cs b/src/Umbraco.Core/Persistence/Repositories/Implement/EntityContainerRepository.cs
@@ -60,7 +60,7 @@ protected override IEnumerable<EntityContainer> PerformGetAll(params int[] ids)
         {
             if (ids.Any())
             {
-                return Database.FetchByGroups<NodeDto, int>(ids, 2000, batch =>
+                return Database.FetchByGroups<NodeDto, int>(ids, Constants.Sql.MaxParameterCount, batch =>
                     GetBaseQuery(false)
                         .Where<NodeDto>(x => x.NodeObjectType == NodeObjectTypeId)
                         .WhereIn<NodeDto>(x => x.NodeId, batch))
diff --git a/src/Umbraco.Core/Persistence/Repositories/Implement/EntityRepository.cs b/src/Umbraco.Core/Persistence/Repositories/Implement/EntityRepository.cs
@@ -281,7 +281,7 @@ private IEnumerable<DocumentEntitySlim> BuildVariants(IEnumerable<DocumentEntity
             if (v == null) return entitiesList;
 
             // fetch all variant info dtos
-            var dtos = Database.FetchByGroups<VariantInfoDto, int>(v.Select(x => x.Id), 2000, GetVariantInfos);
+            var dtos = Database.FetchByGroups<VariantInfoDto, int>(v.Select(x => x.Id), Constants.Sql.MaxParameterCount, GetVariantInfos);
 
             // group by node id (each group contains all languages)
             var xdtos = dtos.GroupBy(x => x.NodeId).ToDictionary(x => x.Key, x => x);
diff --git a/src/Umbraco.Core/Persistence/Repositories/Implement/MemberRepository.cs b/src/Umbraco.Core/Persistence/Repositories/Implement/MemberRepository.cs
@@ -431,16 +431,13 @@ public IEnumerable<IMember> FindMembersInRole(string roleName, string usernameTo
             var matchedMembers = Get(query).ToArray();
 
             var membersInGroup = new List<IMember>();
+
             //then we need to filter the matched members that are in the role
-            //since the max sql params are 2100 on sql server, we'll reduce that to be safe for potentially other servers and run the queries in batches
-            var inGroups = matchedMembers.InGroupsOf(1000);
-            foreach (var batch in inGroups)
+            foreach (var group in matchedMembers.Select(x => x.Id).InGroupsOf(Constants.Sql.MaxParameterCount))
             {
-                var memberIdBatch = batch.Select(x => x.Id);
-
                 var sql = Sql().SelectAll().From<Member2MemberGroupDto>()
                     .Where<Member2MemberGroupDto>(dto => dto.MemberGroup == memberGroup.Id)
-                    .WhereIn<Member2MemberGroupDto>(dto => dto.Member, memberIdBatch);
+                    .WhereIn<Member2MemberGroupDto>(dto => dto.Member, group);
 
                 var memberIdsInGroup = Database.Fetch<Member2MemberGroupDto>(sql)
                     .Select(x => x.Member).ToArray();
diff --git a/src/Umbraco.Core/Persistence/Repositories/Implement/PermissionRepository.cs b/src/Umbraco.Core/Persistence/Repositories/Implement/PermissionRepository.cs
@@ -38,44 +38,41 @@ public PermissionRepository(IScopeAccessor scopeAccessor, AppCaches cache, ILogg
         /// <param name="entityIds"></param>
         /// <returns></returns>
         /// <remarks>
-        /// This method will not support passing in more than 2000 group Ids
+        /// This method will not support passing in more than 2000 group IDs when also passing in entity IDs.
         /// </remarks>
         public EntityPermissionCollection GetPermissionsForEntities(int[] groupIds, params int[] entityIds)
         {
             var result = new EntityPermissionCollection();
 
-            foreach (var groupOfGroupIds in groupIds.InGroupsOf(2000))
+            if (entityIds.Length == 0)
             {
-                //copy local
-                var localIds = groupOfGroupIds.ToArray();
-
-                if (entityIds.Length == 0)
+                foreach (var group in groupIds.InGroupsOf(Constants.Sql.MaxParameterCount))
                 {
                     var sql = Sql()
                         .SelectAll()
                         .From<UserGroup2NodePermissionDto>()
-                        .Where<UserGroup2NodePermissionDto>(dto => localIds.Contains(dto.UserGroupId));
+                        .Where<UserGroup2NodePermissionDto>(dto => group.Contains(dto.UserGroupId));
+
                     var permissions = AmbientScope.Database.Fetch<UserGroup2NodePermissionDto>(sql);
                     foreach (var permission in ConvertToPermissionList(permissions))
                     {
                         result.Add(permission);
                     }
                 }
-                else
+            }
+            else
+            {
+                foreach (var group in entityIds.InGroupsOf(Constants.Sql.MaxParameterCount - groupIds.Length))
                 {
-                    //iterate in groups of 2000 since we don't want to exceed the max SQL param count
-                    foreach (var groupOfEntityIds in entityIds.InGroupsOf(2000))
+                    var sql = Sql()
+                        .SelectAll()
+                        .From<UserGroup2NodePermissionDto>()
+                        .Where<UserGroup2NodePermissionDto>(dto => groupIds.Contains(dto.UserGroupId) && group.Contains(dto.NodeId));
+
+                    var permissions = AmbientScope.Database.Fetch<UserGroup2NodePermissionDto>(sql);
+                    foreach (var permission in ConvertToPermissionList(permissions))
                     {
-                        var ids = groupOfEntityIds;
-                        var sql = Sql()
-                            .SelectAll()
-                            .From<UserGroup2NodePermissionDto>()
-                            .Where<UserGroup2NodePermissionDto>(dto => localIds.Contains(dto.UserGroupId) && ids.Contains(dto.NodeId));
-                        var permissions = AmbientScope.Database.Fetch<UserGroup2NodePermissionDto>(sql);
-                        foreach (var permission in ConvertToPermissionList(permissions))
-                        {
-                            result.Add(permission);
-                        }
+                        result.Add(permission);
                     }
                 }
             }
@@ -133,11 +130,10 @@ public void ReplacePermissions(int groupId, IEnumerable<char> permissions, param
 
             var db = AmbientScope.Database;
 
-            //we need to batch these in groups of 2000 so we don't exceed the max 2100 limit
             var sql = "DELETE FROM umbracoUserGroup2NodePermission WHERE userGroupId = @groupId AND nodeId in (@nodeIds)";
-            foreach (var idGroup in entityIds.InGroupsOf(2000))
+            foreach (var group in entityIds.InGroupsOf(Constants.Sql.MaxParameterCount))
             {
-                db.Execute(sql, new { groupId, nodeIds = idGroup });
+                db.Execute(sql, new { groupId, nodeIds = group });
             }
 
             var toInsert = new List<UserGroup2NodePermissionDto>();
diff --git a/src/Umbraco.Core/Persistence/Repositories/Implement/RedirectUrlRepository.cs b/src/Umbraco.Core/Persistence/Repositories/Implement/RedirectUrlRepository.cs
@@ -37,8 +37,9 @@ protected override IRedirectUrl PerformGet(Guid id)
 
         protected override IEnumerable<IRedirectUrl> PerformGetAll(params Guid[] ids)
         {
-            if (ids.Length > 2000)
-                throw new NotSupportedException("This repository does not support more than 2000 ids.");
+            if (ids.Length > Constants.Sql.MaxParameterCount)
+                throw new NotSupportedException($"This repository does not support more than {Constants.Sql.MaxParameterCount} ids.");
+
             var sql = GetBaseQuery(false).WhereIn<RedirectUrlDto>(x => x.Id, ids);
             var dtos = Database.Fetch<RedirectUrlDto>(sql);
             return dtos.WhereNotNull().Select(Map);
diff --git a/src/Umbraco.Core/Persistence/Repositories/Implement/RepositoryBaseOfTIdTEntity.cs b/src/Umbraco.Core/Persistence/Repositories/Implement/RepositoryBaseOfTIdTEntity.cs
@@ -186,17 +186,17 @@ public IEnumerable<TEntity> GetMany(params TId[] ids)
 
             // can't query more than 2000 ids at a time... but if someone is really querying 2000+ entities,
             // the additional overhead of fetching them in groups is minimal compared to the lookup time of each group
-            const int maxParams = 2000;
-            if (ids.Length <= maxParams)
+            if (ids.Length <= Constants.Sql.MaxParameterCount)
             {
                 return CachePolicy.GetAll(ids, PerformGetAll);
             }
 
             var entities = new List<TEntity>();
-            foreach (var groupOfIds in ids.InGroupsOf(maxParams))
+            foreach (var group in ids.InGroupsOf(Constants.Sql.MaxParameterCount))
             {
-                entities.AddRange(CachePolicy.GetAll(groupOfIds.ToArray(), PerformGetAll));
+                entities.AddRange(CachePolicy.GetAll(group.ToArray(), PerformGetAll));
             }
+
             return entities;
         }
 
diff --git a/src/Umbraco.Core/Persistence/Repositories/Implement/TagRepository.cs b/src/Umbraco.Core/Persistence/Repositories/Implement/TagRepository.cs
@@ -39,7 +39,7 @@ protected override IEnumerable<ITag> PerformGetAll(params int[] ids)
         {
             var dtos = ids.Length == 0
                 ? Database.Fetch<TagDto>(Sql().Select<TagDto>().From<TagDto>())
-                : Database.FetchByGroups<TagDto, int>(ids, 2000, batch => Sql().Select<TagDto>().From<TagDto>().WhereIn<TagDto>(x => x.Id, batch));
+                : Database.FetchByGroups<TagDto, int>(ids, Constants.Sql.MaxParameterCount, batch => Sql().Select<TagDto>().From<TagDto>().WhereIn<TagDto>(x => x.Id, batch));
 
             return dtos.Select(TagFactory.BuildEntity).ToList();
         }
diff --git a/src/Umbraco.Core/Umbraco.Core.csproj b/src/Umbraco.Core/Umbraco.Core.csproj
@@ -132,6 +132,7 @@
     <Compile Include="Configuration\ICoreDebug.cs" />
     <Compile Include="Constants-CharArrays.cs" />
     <Compile Include="Collections\EventClearingObservableCollection.cs" />
+    <Compile Include="Constants-Sql.cs" />
     <Compile Include="Constants-SqlTemplates.cs" />
     <Compile Include="Events\UnattendedInstallEventArgs.cs" />
     <Compile Include="Logging\ILogger2.cs" />
@@ -1665,4 +1666,4 @@
     <Folder Include="Auditing\" />
   </ItemGroup>
   <Import Project="$(MSBuildToolsPath)\Microsoft.CSharp.targets" />
-</Project>
+</Project>
diff --git a/src/Umbraco.Web/Models/Mapping/UserMapDefinition.cs b/src/Umbraco.Web/Models/Mapping/UserMapDefinition.cs
@@ -240,7 +240,7 @@ private void Map(IUserGroup source, UserGroupDisplay target, MapperContext conte
                 // the entity service due to too many Sql parameters.
 
                 var list = new List<IEntitySlim>();
-                foreach (var idGroup in allContentPermissions.Keys.InGroupsOf(2000))
+                foreach (var idGroup in allContentPermissions.Keys.InGroupsOf(Constants.Sql.MaxParameterCount))
                     list.AddRange(_entityService.GetAll(UmbracoObjectTypes.Document, idGroup.ToArray()));
                 contentEntities = list.ToArray();
             }

Original file line number	Diff line number	Diff line change
`@@ -54,7 +54,7 @@ protected override IEnumerable<IAuditEntry> PerformGetAll(params int[] ids)`
`54`	`54`
`55`	`55`	`var entries = new List<IAuditEntry>();`
`56`	`56`
`57`		`- foreach (var group in ids.InGroupsOf(2000))`
	`57`	`+ foreach (var group in ids.InGroupsOf(Constants.Sql.MaxParameterCount))`
`58`	`58`	`{`
`59`	`59`	`var sql = Sql()`
`60`	`60`	`.Select<AuditEntryDto>()`
Original file line number	Diff line number	Diff line change
`@@ -60,7 +60,7 @@ protected override IEnumerable<EntityContainer> PerformGetAll(params int[] ids)`
`60`	`60`	`{`
`61`	`61`	`if (ids.Any())`
`62`	`62`	`{`
`63`		`- return Database.FetchByGroups<NodeDto, int>(ids, 2000, batch =>`
	`63`	`+ return Database.FetchByGroups<NodeDto, int>(ids, Constants.Sql.MaxParameterCount, batch =>`
`64`	`64`	`GetBaseQuery(false)`
`65`	`65`	`.Where<NodeDto>(x => x.NodeObjectType == NodeObjectTypeId)`
`66`	`66`	`.WhereIn<NodeDto>(x => x.NodeId, batch))`