Repository navigation
Count a reused parameterized collection once when checking the SQL Server parameter limit - #39634
Open
benedict-odonovan wants to merge 1 commit into
Conversation
…rver parameter limit SqlNullabilityProcessor expands each collection parameter once and shares the expanded parameters across occurrences, but ParametersCounter added the expanded size on every occurrence, so a list used several times fell back to OPENJSON far below the 2,100-parameter limit. Fixes dotnet#39631
Comment on lines
+469
to
+483
| // Occurrences share one expansion, padded to the largest bucket any of them needs. | ||
| var parameters = parametersDecorator.GetAndDisableCaching(); | ||
| var count = ((IEnumerable?)parameters[sqlParameterExpression.Name])?.Cast<object?>().Count() ?? 0; | ||
| Count += count; | ||
|
|
||
| if (bucketization) | ||
| { | ||
| var elementTypeMapping = (RelationalTypeMapping)sqlParameterExpression.TypeMapping!.ElementTypeMapping!; | ||
| Count += bucketizationPadding(count, elementTypeMapping); | ||
| count += bucketizationPadding(count, elementTypeMapping); | ||
| } | ||
|
|
||
| var counted = _expandedParameterCounts.GetValueOrDefault(sqlParameterExpression); | ||
| if (count > counted) | ||
| { | ||
| Count += count - counted; | ||
| _expandedParameterCounts[sqlParameterExpression] = count; |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

Fixes #39631
Summary
When the same captured collection is used more than once in a query, SQL Server falls back from expanded parameters to
OPENJSONfar below the 2,100-parameter limit:OPENJSONhere costs more than it saves. In the application from the issue, a 950-item list is used in the outerWHEREand in three correlated subqueries. After the fallback, server CPU for that statement went from 24 ms to 176 ms per execution, because each subquery parses the JSON again.Cause.
SqlNullabilityProcessorexpands each collection parameter once and reuses those parameters at every occurrence, through_collectionParameterExpansionMap. That map is keyed bySqlParameterExpressionequality (name, type and type mapping). The bucket padding grows the same shared list, so it ends up at the largest size any occurrence needs.ParametersCounterto add the expanded size of aMultipleParameterscollection on every occurrence. So the list above counts as 4 × 600 = 2,400, which is overMaxParameterCount(2,098), although only 600 parameters are sent...._same_parametertests inPrimitiveCollectionsQuerySqlServerTestwere changed in Make type mapping generic so that it can create value comparers for NativeAOT #38440 to assertOPENJSON. In 10.0 they asserted expanded parameters.Change
ParametersCounternow counts each collection parameter once, keyed the same way as_collectionParameterExpansionMap. A collection that gets separate expansions, because it's compared to columns with different type mappings, is still counted once per expansion.VALUESoccurrence isn't padded and anINoccurrence is, so a list used both ways is counted at its padded size whichever comes first.Parametermode is unchanged: one parameter per collection.Tests
..._same_parameterand..._same_parameter_different_propertytests inPrimitiveCollectionsQuerySqlServerTestandPrimitiveCollectionsQueryOldSqlServerTestgo back to expecting expanded parameters. They now also check how many are sent, e.g.@extra1000but no@extra1001.IdandInthave the same type mapping, so thedifferent_propertytests share one expansion.Same_parameter_with_different_type_mappings_over_parameter_limit: 1,100 values compared to adatetimeand adatetime2column get two expansions (2,200 parameters), so the query still falls back. If the counter is keyed by name only, this test fails with SQL Server's "too many parameters" error.Parameter_collection_reused_with_and_without_padding_over_parameter_limit: 1,901 values used inCount(1,901 parameters) and inContains(padded to 2,000), plus 91 values padded to 100, is 2,100 parameters and must fall back. If only the first occurrence is counted, this test fails with the same SQL Server error.Locally, all SQL Server query tests pass (20,910 passed, 0 failed), and the primitive-collections suites pass with the two new tests. Without the change, the four reused-collection tests fail because the queries fall back to
OPENJSON.