Skip to content

Conversation

dreab8
Copy link
Member

@dreab8 dreab8 commented Oct 1, 2025

https://hibernate.atlassian.net/browse/HHH-19085


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.


Comment on lines 2185 to 2189
if ( value == null ) {
return new ValueBindJpaCriteriaParameter<>( null, value, this );
}
final var bindableType = resolveInferredParameterType( value, typeInferenceSource, getTypeConfiguration() );
if ( bindableType == null || isInstance( bindableType, value) ) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wouldn't it be better to try retaining the type inference source?

Suggested change
if ( value == null ) {
return new ValueBindJpaCriteriaParameter<>( null, value, this );
}
final var bindableType = resolveInferredParameterType( value, typeInferenceSource, getTypeConfiguration() );
if ( bindableType == null || isInstance( bindableType, value) ) {
final var bindableType = resolveInferredParameterType( value, typeInferenceSource, getTypeConfiguration() );
if ( bindableType == null || value == null || isInstance( bindableType, value ) ) {

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see

if ( value != null || criteriaParameter.getNodeType() == null ) {

if we set the type than later on the value is not binded.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not sure why we are not binding the null value when criteriaParameter.getNodeType() is not null

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like this was added as part of d9446e7, but I doubt that this is still useful. I'd suggest removing the if and always bind the value.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AbstractSqmSelectionQuery should probably be using this instead: if ( criteriaParameter instanceof ValueBindJpaCriteriaParameter<?> ) {

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants