HHH-19383 improvements #10198
HHH-19383 improvements #10198
Conversation
NativeQueryConstructorTransformer does not prevent the result type from having multiple constructors, it just rejects a result type with multiple constructors with matching arity
| paramTypes[i]) ) { | ||
| return false; | ||
| // TODO: Only one constructor with the right number of parameters is allowed | ||
| // (see NativeQueryConstructorTransformer) so we should validate that |
There was a problem hiding this comment.
Wasn't I doing just that in my original code?
I don't quite understand why you would instead iterate all constructors, if clearly only exactly one with the right amt. of params is allowed ... ?
I removed the constructor iteration I originally also had when I saw there should only be one ...
There was a problem hiding this comment.
Well we have to skip over all the constructors with the wrong number of parameters until we get to one with the right number of parameters.
There was a problem hiding this comment.
IINM, that is not true; NativeQueryConstructorTransformershould allow only a constructor with the same number of params, else an InstantiationExceptionwould be thrown, so no need to iterate?
There was a problem hiding this comment.
That's just not correct. Look at the code in NativeQueryConstructorTransformer. It iterates over the constructors, looking for one with the right number of parameters, and ignores all the rest:
for ( final Constructor<?> candidate : resultClass.getDeclaredConstructors() ) {
final Class<?>[] parameterTypes = candidate.getParameterTypes();
if ( parameterTypes.length == elements.length ) {
// found a candidate with the right number
// of parametersThere was a problem hiding this comment.
Ok, I was mislead by the exception message ...
Maybe it's somewhat confusing since it's possible for a resultclass to have several constructors with the same number of params as needed, but with different param types, and then you'd still get that message?
I'm just annoyed by the fact I initially had a perfectly good implementation including constructor iteration, and then I took that out thinking it was not needed, only for the lead Hibernate guy to come in after me and slap me on the wrist by rewriting my code. Fucks up my self-esteem in so many ways ...
There was a problem hiding this comment.
it's possible for a resultclass to have several constructors with the same number of params as needed, but with different param types, and then you'd still get that message?
Correct. The potentially-ambiguous case is where you have multiple constructors with the same number of parameters but different types.
Fucks up my self-esteem in so many ways
I make mistakes all the time. I don't let it bother me.
2 participants
[Please describe here what your change is about]
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license
and can be relicensed under the terms of the LGPL v2.1 license in the future at the maintainers' discretion.
For more information on licensing, please check here.
https://hibernate.atlassian.net/browse/HHH-19383