Add Linq parameter type detection #2365
Conversation
|
|
||
| // When MappedAs is used we have to put all sql types information in the key in order to | ||
| // distinct when different precisions/sizes are used. | ||
| if (_sessionFactory != null && value is IType type) |
There was a problem hiding this comment.
With this PR ExpressionKeyVisitor generates different keys when MappedAs type is changed in order to avoid using the same query plan as now parameter types are always retrieved from the ParameterMetadata.
| if (expression.Value == null) | ||
| type = NHibernateUtil.GuessType(expression.Type); | ||
|
|
||
| // Constant characters should be sent as strings | ||
| if (expression.Type == typeof(char)) | ||
| // TODO 6.0: Remove | ||
| if (_queryVariables == null && expression.Type == typeof(char)) | ||
| { | ||
| value = value.ToString(); |
There was a problem hiding this comment.
This to me looks as a workaroud to fix CharIndexFunction and CharIndexOffsetNegativeFunction tests on Sql Server and Sql CE databases. Instead of converting it to string, this PR set the correct parameter size for drivers that fail without it.
|
|
||
| namespace NHibernate.Util | ||
| { | ||
| internal static class ParameterHelper |
There was a problem hiding this comment.
The methods were extracted from AbstractQueryImpl in order to be used by ConstantTypeLocator.
maca88
changed the title
maca88
force-pushed
the
LinqParamTypeDetection
branch
from
6b441a9
to
7fd6dc7
Compare
maca88
changed the title
|
There is a failing new test. |
|
Interesting. While developing the NodaTime integration I came upon a case where a type (DatePeriod) implements IEnumerable. It works just fine in terms of persistence, but when querying, a DatePeriod parameter will be added using SetParameterList, which falls apart completely. It's not a list. It's a composite type. The "implements IEnumerable but is not a string" check is just a bit too naïve, but given its context, it's the best that can be done. Will this PR or its siblings open up a way for this to be remedied? |
Originally not, but with the last commit it will no longer call |
maca88
force-pushed
the
LinqParamTypeDetection
branch
from
5c36581
to
1c313f9
Compare
|
Rebased. |
|
Please hold off merging till weekend. I want to review this. |
|
By pure coincidence, I'm creating something very similar in my NodaTime project (required to prevent ambiguous Linq-to-Hql processes). I wouldn't mind using this instead, but it's internal and I may need to avoid taking the shortcut of just returning the first candidate type when there's a coalesce or a conditional. |
I am not sure if we should make |
maca88
force-pushed
the
LinqParamTypeDetection
branch
from
6059b58
to
67d2640
Compare
|
Rebased due to conflict with #2387. |
|
I couldn't agree more. Something that carries mapping metadata within the expression tree (which doesn't have to be Linq based. It could cause confusion). Hibernate is working on the Semantic Query Model approach. I'm not sure how that will turn out, but the idea seems right. Preferably, the SQL would also be expressed as a visitable tree. |
Co-authored-by: Alexander Zaytsev <alexzaytsev2019@gmail.com>
fredericDelaporte
force-pushed
the
LinqParamTypeDetection
branch
from
9f67be2
to
98137f1
Compare
|
Rebased for resolving a conflict. |
This is a continuation of #2335 where the code was extracted from #2079. The main purpose of this PR is to gain the ability to have parameter types calculated before rewriting the query model, which we can use when building the HQL tree. Today parameter types are calculated by HQL logic which is unable to correctly calculate types is certain cases (Example: mod function discussed here).