Fixes QueryMultiple corrupting a later Query<T>'s DynamicParameters -- fixes #243, #2091#2214
Open
NadeemAfana wants to merge 1 commit into
Open
Fixes QueryMultiple corrupting a later Query<T>'s DynamicParameters -- fixes #243, #2091#2214NadeemAfana wants to merge 1 commit into
NadeemAfana wants to merge 1 commit into
Conversation
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.
Summary
Calling a query (stored proc or text) via
QueryMultiple(...).Read<T>()and then viaQuery<T>(...)/QueryAsync<T>(...)withDynamicParameterson the same SQL can cause the second call to send the wrong parameters, typically dropping them entirely. Depending on the target's signature this surfaces as:Must declare the scalar variable "@x".(text)Procedure or function 'X' expects parameter '@y', which was not supplied.Procedure or function 'X' has too many arguments specified.@ParameterNames1 is not a parameter for procedure X.(the literal symptom in Issue with Dapper: Intermittent '@ParameterNames1 is not a parameter for procedure...' Error #2091)It's intermittent in real apps (depends on which call populates the shared cache entry first) and clears on app restart because the corruption lives in the process-wide query cache.
Fixes #243. Fixes #2091.
Root cause
Query<T>and the first grid of aQueryMultipleover the same SQL share a single cache entry. A grid read only needs the deserializer and has no parameter instance, so it populates that shared entry with a null parameterobject. For
DynamicParameters(whose real parameters live in an internal collection, not in the type's properties), Dapper then builds a parameter-reader from the type's properties (ParameterNames,RemoveUnused) that never emits the actual parameters. Because the grid identity still carries the parameter type, it collides with theQuery<T>identity, so a laterQuery<T>with
DynamicParametersreuses that wrong reader and sends the wrong parameters (or none). Whichever call populates the entry first wins, which is why it's intermittent and clears on restart.Anonymous/POCO parameters are unaffected since their parameters are their properties.
Fix
Identity.ForGrid(...)now passesnullforparametersType(grid reads never bind parameters).This:
Query<T>identities, so aQuery<T>always builds/uses its own correct ParamReader; andGetCacheInfoskip building a ParamReader for grid reads.Impact / trade-offs
parametersType.(sql, T)is no longer shared with aQuery<T>for the same(sql, T)so at most one extra deserializer compile (one-time, cached). No per-call overhead.Conversely, deserializers are now shared across different parameter types, which the old keying prevented (a small net win).
Query<T>(sql)(both haveparametersType == null); that's harmless as neither builds a ParamReader.