Insert underscore in combined parameter name #1428
Conversation
|
@gliljas I've changed this to point to the older issue. Also, could you please add tests for "future" queries? |
|
Do you mean multiqueries (and criteria)? |
|
Ah, sorry, it's fine. I wanted for Linq/Criteria Futures. But you've already added them. |
|
Still need tests for MultiCriteria |
gliljas
force-pushed
the
GH-1427
branch
2 times, most recently
from
2845a28 to
4d1abee
Compare
| multi.Add(criteria); | ||
| } | ||
| //Paramater combining is only used for cacheable queries | ||
| multi.SetCacheable(true); |
There was a problem hiding this comment.
Small typo (Paramater instead of Parameter) and yet another somewhat undue implementation discrepancy between criteria and query. I think the criteria implementation is the right one, the query one creates those parameters uselessly when the query is not cacheable. They are only used in the query cache key.
So if we realign/mutualize implementation later, the multi-query test may no more test anything. It would be better to enable caching on it too (by enabling caching on all its queries, with same region).
There was a problem hiding this comment.
Good idea. I'll fix the typo, set cacheable and rebase.
Simplest possible fix for #1357, which bit us in production today.
Ideally, this should be extracted to a testable class.