Repositories: Batch WHERE IN queries to avoid SQL Server 2100-parameter limit - #22987
Conversation
The memory files should describe the current rule and safe patterns; specific historical bugs belong in commit history, not CLAUDE.md. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Claude finished @AndyButland's task in 6m 6s —— View job PR ReviewTarget: Adds batching to
Suggestions
Approved with Suggestions for improvementGood defensive fix that closes a real production footgun across several high-traffic rebuild/seeding paths. The CLAUDE.md documentation and the |
There was a problem hiding this comment.
Pull request overview
This PR hardens repository queries against SQL Server’s 2100-parameter limit by batching WHERE IN clauses using InGroupsOf(Constants.Sql.MaxParameterCount), preventing runtime SqlException 8003 when batch-size settings or datasets grow beyond safe defaults.
Changes:
- Batch
WHERE INqueries inDatabaseCacheRepository(content/media source reads + rebuild “ForNodes” helpers) to stay underConstants.Sql.MaxParameterCount. - Update
RedirectUrlRepository.PerformGetAllto (a) treat null/empty ids as “fetch all” and (b) batch id lookups instead of throwing above the parameter ceiling. - Add/extend integration tests covering the common (“small N”) paths for both repositories, and document safe batching patterns in the repo CLAUDE guides.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/Umbraco.Tests.Integration/Umbraco.PublishedCache.HybridCache/DatabaseCacheRepositoryTests.cs | Adds integration coverage for hybrid-cache source lookups (keys + empty keys) and rebuild pipeline path. |
| tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Persistence/Repositories/RedirectUrlRepositoryTests.cs | Adds tests for GetMany with ids and for the “no ids => fetch all” convention. |
| src/Umbraco.PublishedCache.HybridCache/Persistence/DatabaseCacheRepository.cs | Implements batching for GUID/id WHERE IN queries to avoid exceeding SQL Server parameter limits. |
| src/Umbraco.Infrastructure/Persistence/Repositories/Implement/RedirectUrlRepository.cs | Implements batched PerformGetAll and aligns empty/null ids behavior with “fetch all” expectations. |
| src/Umbraco.Infrastructure/CLAUDE.md | Documents decision rules and safe batching patterns for the 2100-parameter limit. |
| src/Umbraco.Core/CLAUDE.md | Adds cross-reference to Constants.Sql.MaxParameterCount and batching helpers. |
| CLAUDE.md | Adds top-level guidance pointing contributors to the detailed batching section. |
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Zeegaan
left a comment
There was a problem hiding this comment.
Probably a good preventative measure 😁
Description
We've had a few issues over the years with
WHERE INqueries exceeding the SQL Server parameter limit.This PR is a preventive update of a few other cases I found where we haven't had issues reported, but depending on configuration and customer data, could in theory trip up on this in future.
Also included are
CLAUDE.mdupdates in an attempt that this oversight in future is caught in development or review.The updates are:
DatabaseCacheRepository.GetContentSourcesAsync/GetMediaSourcesAsyncand the five private rebuildForNodeshelpers now batch theirWHERE INqueries withInGroupsOf(Constants.Sql.MaxParameterCount). These were driven byCacheSettings.DocumentSeedBatchSize,CacheSettings.MediaSeedBatchSizeandNuCacheSettings.SqlPageSize— defaults are safe but the settings are documented as tunable, and raising any of them past 2000 would break seeding/rebuilds with this exception.RedirectUrlRepository.PerformGetAllno longer throws when given more thanMaxParameterCountids. It now batches the fetch, and treats null/empty ids as "fetch all" (matching the convention used by other repositories likeDomainRepository.PerformGetAll).Testing
Additional integration tests have been added to ensure there's no regression of the expected and happy path of a small number of IDs for which entities are retrieved.