Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

HHH-18556 Expressions.nullExpresion() in querydsl result in NPE in SqmExpressible #8936

Merged
merged 2 commits into from
Sep 12, 2024
Merged

HHH-18556 Expressions.nullExpresion() in querydsl result in NPE in SqmExpressible #8936

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8b1fafa
HHH-18556 Add test for issue
dreab8 Sep 11, 2024
46f8691
HHH-18556 Expressions.nullExpresion() in querydsl result in NPE in Sq…
dreab8 Sep 11, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 18 additions & 10 deletions hibernate-core/src/main/java/org/hibernate/query/internal/QueryParameterBindingImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import org.hibernate.query.spi.QueryParameterBinding;
import org.hibernate.query.spi.QueryParameterBindingValidator;
import org.hibernate.query.sqm.SqmExpressible;
import org.hibernate.query.sqm.tree.expression.NullSqmExpressible;
import org.hibernate.type.descriptor.java.JavaType;
import org.hibernate.type.descriptor.java.JavaTypeHelper;
import org.hibernate.type.spi.TypeConfiguration;
Expand Down Expand Up @@ -113,10 +114,10 @@ public void setBindValue(T value, boolean resolveJdbcTypeIfNecessary) {
final Object coerced;
if ( ! sessionFactory.getSessionFactoryOptions().getJpaCompliance().isLoadByIdComplianceEnabled() ) {
try {
if ( bindType != null ) {
if ( canValueBeCoerced( bindType) ) {
coerced = coerce( value, bindType );
}
else if ( queryParameter.getHibernateType() != null ) {
else if ( canValueBeCoerced( queryParameter.getHibernateType() ) ) {
coerced = coerce( value, queryParameter.getHibernateType() );
}
else {
Expand Down Expand Up @@ -190,7 +191,7 @@ private boolean isRegisteredAsBasicType(Class<?> valueClass) {
private void bindValue(Object value) {
isBound = true;
bindValue = value;
if ( bindType == null && value != null ) {
if ( canBindValueBeSet( value, bindType ) ) {
bindType = sessionFactory.getMappingMetamodel().resolveParameterBindType( value );
}
}
Expand All @@ -206,10 +207,10 @@ public void setBindValue(T value, BindableType<T> clarifiedType) {
}

final Object coerced;
if ( bindType != null ) {
if ( canValueBeCoerced( bindType ) ) {
coerced = coerce( value, bindType );
}
else if ( queryParameter.getHibernateType() != null ) {
else if ( canValueBeCoerced( queryParameter.getHibernateType() ) ) {
coerced = coerce( value, queryParameter.getHibernateType() );
}
else {
Expand All @@ -233,7 +234,7 @@ public void setBindValue(T value, TemporalType temporalTypePrecision) {

final Object coerced;
if ( ! sessionFactory.getSessionFactoryOptions().getJpaCompliance().isLoadByIdComplianceEnabled() ) {
if ( bindType != null ) {
if (canValueBeCoerced( bindType ) ) {
try {
coerced = coerce( value, bindType );
}
Expand All @@ -249,7 +250,7 @@ public void setBindValue(T value, TemporalType temporalTypePrecision) {
);
}
}
else if ( queryParameter.getHibernateType() != null ) {
else if ( canValueBeCoerced( queryParameter.getHibernateType() ) ) {
coerced = coerce( value, queryParameter.getHibernateType() );
}
else {
Expand Down Expand Up @@ -299,10 +300,9 @@ public void setBindValues(Collection<? extends T> values) {
value = iterator.next();
}

if ( bindType == null && value != null ) {
this.bindType = sessionFactory.getMappingMetamodel().resolveParameterBindType( value );
if ( canBindValueBeSet( value, bindType ) ) {
bindType = sessionFactory.getMappingMetamodel().resolveParameterBindType( value );
}

}

@Override
Expand Down Expand Up @@ -378,4 +378,12 @@ private void validate(Object value, TemporalType clarifiedTemporalType) {
public TypeConfiguration getTypeConfiguration() {
return sessionFactory.getTypeConfiguration();
}

private static boolean canValueBeCoerced(BindableType<?> bindType) {
return bindType != null && !( bindType instanceof NullSqmExpressible );
}

private static boolean canBindValueBeSet(Object value, BindableType<?> bindType) {
return value != null && ( bindType == null || bindType instanceof NullSqmExpressible );
}
}
3 changes: 2 additions & 1 deletion hibernate-core/src/main/java/org/hibernate/query/spi/AbstractCommonQueryContract.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@
import org.hibernate.query.criteria.JpaExpression;
import org.hibernate.query.internal.QueryOptionsImpl;
import org.hibernate.query.sqm.SqmExpressible;
import org.hibernate.query.sqm.tree.expression.NullSqmExpressible;
import org.hibernate.query.sqm.tree.expression.SqmLiteral;
import org.hibernate.query.sqm.tree.expression.SqmParameter;
import org.hibernate.query.sqm.tree.select.SqmSelectStatement;
Expand Down Expand Up @@ -906,7 +907,7 @@ public CommonQueryContract setParameter(int position, Object value) {
final QueryParameter<Object> param = binding.getQueryParameter();
if ( param.allowsMultiValuedBinding() ) {
final BindableType<?> hibernateType = param.getHibernateType();
if ( hibernateType == null || isInstance( hibernateType, value ) ) {
if ( hibernateType == null || hibernateType instanceof NullSqmExpressible || isInstance( hibernateType, value ) ) {
Copy link
Member

Choose a reason for hiding this comment

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

@dreab8 Why did you make this change for positional parameters, but not for named parameters?

Copy link
Member Author

Choose a reason for hiding this comment

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

Hi @gavinking,

a big oversight :( I'm going to open a Jira and fix it.

Copy link
Member Author

Choose a reason for hiding this comment

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

thanks for spotting it

Copy link
Member

Choose a reason for hiding this comment

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

Excellent, cheers mate 👍

if ( value instanceof Collection && !isRegisteredAsBasicType( value.getClass() ) ) {
//noinspection rawtypes
return setParameterList( position, (Collection) value );
Expand Down
25 changes: 25 additions & 0 deletions hibernate-core/src/test/java/org/hibernate/orm/test/query/SelectCaseWhenNullLiteralTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

import org.hibernate.testing.TestForIssue;
import org.hibernate.testing.orm.junit.DomainModel;
import org.hibernate.testing.orm.junit.JiraKey;
import org.hibernate.testing.orm.junit.SessionFactory;
import org.hibernate.testing.orm.junit.SessionFactoryScope;
import org.junit.jupiter.api.AfterEach;
Expand Down Expand Up @@ -55,6 +56,30 @@ public void testSelectCaseWhenNullLiteral(SessionFactoryScope scope) {
);
}

@Test
@JiraKey( "HHH-18556" )
public void testSelectCaseWhenNullLiteralWithParameters(SessionFactoryScope scope) {
scope.inTransaction(
session -> {
List result = session.createQuery( "select case when 1=1 then ?1 else null end from Person p" )
.setParameter( 1, 2 )
.list();
assertThat( result.size(), is( 1 ) );
assertThat( result.get( 0 ), is( 2 ) );
}
);

scope.inTransaction(
session -> {
List result = session.createQuery( "select count(case when 1=1 then ?1 else null end) from Person p" )
.setParameter( 1, 2 )
.list();
assertThat( result.size(), is( 1 ) );
assertThat( result.get( 0 ), is( 1L ) );
}
);
}

@Entity(name = "Person")
@Table(name = "PERSON_TABLE")
public static class Person {
Expand Down