From 914dd25ee33d59971140fd3b465eadaf7b432513 Mon Sep 17 00:00:00 2001 From: Gavin King Date: Sun, 30 Nov 2025 22:16:27 +0100 Subject: [PATCH 1/5] first phase of strict checking of arguments to query parameters --- .../internal/ProcedureParamBindings.java | 10 +- .../query/QueryArgumentTypeException.java | 31 ++ .../internal/QueryParameterBindingsImpl.java | 11 +- .../spi/AbstractCommonQueryContract.java | 410 +++++++++++++----- .../query/spi/QueryParameterBindings.java | 12 +- .../tree/expression/SqmNamedParameter.java | 2 +- .../type/descriptor/java/EnumJavaType.java | 12 +- .../orm/test/hql/ParameterIsNullTest.java | 2 +- .../mutation/multitable/IdSelectionTests.java | 16 +- .../type/LongListTypeContributorTest.java | 2 +- 10 files changed, 360 insertions(+), 148 deletions(-) create mode 100644 hibernate-core/src/main/java/org/hibernate/query/QueryArgumentTypeException.java diff --git a/hibernate-core/src/main/java/org/hibernate/procedure/internal/ProcedureParamBindings.java b/hibernate-core/src/main/java/org/hibernate/procedure/internal/ProcedureParamBindings.java index f53fb220006a..e75a33417ea3 100644 --- a/hibernate-core/src/main/java/org/hibernate/procedure/internal/ProcedureParamBindings.java +++ b/hibernate-core/src/main/java/org/hibernate/procedure/internal/ProcedureParamBindings.java @@ -71,10 +71,9 @@ public

ProcedureParameterBinding

getQueryParamerBinding(ProcedureParamete } @Override - public

ProcedureParameterBinding

getBinding(String name) { - //noinspection unchecked + public ProcedureParameterBinding getBinding(String name) { final var parameter = - (ProcedureParameterImplementor

) + (ProcedureParameterImplementor) parameterMetadata.getQueryParameter( name ); if ( parameter == null ) { throw new IllegalArgumentException( "Parameter does not exist: " + name ); @@ -83,10 +82,9 @@ public

ProcedureParameterBinding

getBinding(String name) { } @Override - public

ProcedureParameterBinding

getBinding(int position) { - //noinspection unchecked + public ProcedureParameterBinding getBinding(int position) { final var parameter = - (ProcedureParameterImplementor

) + (ProcedureParameterImplementor) parameterMetadata.getQueryParameter( position ); if ( parameter == null ) { throw new IllegalArgumentException( "Parameter at position " + position + "does not exist" ); diff --git a/hibernate-core/src/main/java/org/hibernate/query/QueryArgumentTypeException.java b/hibernate-core/src/main/java/org/hibernate/query/QueryArgumentTypeException.java new file mode 100644 index 000000000000..150d782cdbd3 --- /dev/null +++ b/hibernate-core/src/main/java/org/hibernate/query/QueryArgumentTypeException.java @@ -0,0 +1,31 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * Copyright Red Hat Inc. and Hibernate Authors + */ +package org.hibernate.query; + +/** + * Thrown when an argument bound to a query parameter is of an incompatible type. + * + * @since 7.2 + * + * @author Gavin King + */ +public class QueryArgumentTypeException extends IllegalArgumentException { + private final Class expectedType; + private final Class actualType; + + public QueryArgumentTypeException(String message, Class expectedType, Class actualType) { + super( message + " (" + actualType.getName() + " is not assignable to " + expectedType.getName() + ")" ); + this.expectedType = expectedType; + this.actualType = actualType; + } + + public Class getExpectedType() { + return expectedType; + } + + public Class getActualType() { + return actualType; + } +} diff --git a/hibernate-core/src/main/java/org/hibernate/query/internal/QueryParameterBindingsImpl.java b/hibernate-core/src/main/java/org/hibernate/query/internal/QueryParameterBindingsImpl.java index 281a0b5de19e..2561fd7abd5e 100644 --- a/hibernate-core/src/main/java/org/hibernate/query/internal/QueryParameterBindingsImpl.java +++ b/hibernate-core/src/main/java/org/hibernate/query/internal/QueryParameterBindingsImpl.java @@ -129,30 +129,29 @@ public

QueryParameterBinding

getBinding(QueryParameterImplementor

para "Cannot create binding for parameter reference [" + parameter + "] - reference is not a parameter of this query" ); } + //TODO: typecheck! //noinspection unchecked return (QueryParameterBinding

) binding; } @Override - public

QueryParameterBinding

getBinding(int position) { + public QueryParameterBinding getBinding(int position) { final var binding = parameterBindingMapByNameOrPosition.get( position ); if ( binding == null ) { // Invoke this method to throw the exception parameterMetadata.getQueryParameter( position ); } - //noinspection unchecked - return (QueryParameterBinding

) binding; + return binding; } @Override - public

QueryParameterBinding

getBinding(String name) { + public QueryParameterBinding getBinding(String name) { final var binding = parameterBindingMapByNameOrPosition.get( name ); if ( binding == null ) { // Invoke this method to throw the exception parameterMetadata.getQueryParameter( name ); } - //noinspection unchecked - return (QueryParameterBinding

) binding; + return binding; } @Override diff --git a/hibernate-core/src/main/java/org/hibernate/query/spi/AbstractCommonQueryContract.java b/hibernate-core/src/main/java/org/hibernate/query/spi/AbstractCommonQueryContract.java index a82b490e1e55..c8a0bfdd6efb 100644 --- a/hibernate-core/src/main/java/org/hibernate/query/spi/AbstractCommonQueryContract.java +++ b/hibernate-core/src/main/java/org/hibernate/query/spi/AbstractCommonQueryContract.java @@ -35,10 +35,10 @@ import org.hibernate.query.QueryFlushMode; import org.hibernate.query.QueryParameter; import org.hibernate.query.TypedParameterValue; +import org.hibernate.query.QueryArgumentTypeException; import org.hibernate.query.criteria.JpaExpression; import org.hibernate.query.internal.QueryOptionsImpl; import org.hibernate.query.sqm.NodeBuilder; -import org.hibernate.query.sqm.SqmExpressible; import org.hibernate.query.sqm.tree.expression.SqmLiteral; import org.hibernate.query.sqm.tree.expression.SqmParameter; import org.hibernate.query.sqm.tree.select.SqmSelectStatement; @@ -51,10 +51,12 @@ import java.util.Collection; import java.util.Date; import java.util.HashMap; +import java.util.List; import java.util.Map; import java.util.Set; -import static java.util.Arrays.asList; +import static java.lang.Math.round; +import static java.util.Collections.unmodifiableSet; import static java.util.Locale.ROOT; import static org.hibernate.internal.log.DeprecationLogger.DEPRECATION_LOGGER; import static org.hibernate.jpa.HibernateHints.HINT_CACHEABLE; @@ -282,7 +284,7 @@ public final boolean applyHint(String hintName, Object value) { //fall through: case HINT_SPEC_QUERY_TIMEOUT: // convert milliseconds to seconds - int timeout = (int) Math.round( getInteger( value ).doubleValue() / 1000.0 ); + final int timeout = (int) round( getInteger( value ).doubleValue() / 1000.0 ); applyTimeoutHint( timeout ); return true; case HINT_COMMENT: @@ -621,15 +623,13 @@ public int getFirstResult() { @Override public abstract ParameterMetadataImplementor getParameterMetadata(); - @SuppressWarnings({"unchecked", "rawtypes"}) public Set> getParameters() { checkOpenNoRollback(); - return (Set) getParameterMetadata().getRegistrations(); + return unmodifiableSet( getParameterMetadata().getRegistrations() ); } public QueryParameterImplementor getParameter(String name) { checkOpenNoRollback(); - try { return getParameterMetadata().getQueryParameter( name ); } @@ -638,21 +638,20 @@ public QueryParameterImplementor getParameter(String name) { } } - @SuppressWarnings("unchecked") public QueryParameterImplementor getParameter(String name, Class type) { checkOpenNoRollback(); - try { - //noinspection rawtypes - final QueryParameterImplementor parameter = getParameterMetadata().getQueryParameter( name ); + final var parameter = getParameterMetadata().getQueryParameter( name ); if ( !type.isAssignableFrom( parameter.getParameterType() ) ) { - throw new IllegalArgumentException( - "The type [" + parameter.getParameterType().getName() + - "] associated with the parameter corresponding to name [" + name + - "] is not assignable to requested Java type [" + type.getName() + "]" + throw new QueryArgumentTypeException( + "Type specified for parameter named '" + name + "' is incompatible", + parameter.getParameterType(), + type ); } - return parameter; + @SuppressWarnings("unchecked") // safe, just checked + var castParameter = (QueryParameterImplementor) parameter; + return castParameter; } catch ( HibernateException e ) { throw getExceptionConverter().convert( e ); @@ -661,7 +660,6 @@ public QueryParameterImplementor getParameter(String name, Class type) public QueryParameterImplementor getParameter(int position) { checkOpenNoRollback(); - try { return getParameterMetadata().getQueryParameter( position ); } @@ -670,20 +668,20 @@ public QueryParameterImplementor getParameter(int position) { } } - @SuppressWarnings( {"unchecked", "rawtypes"} ) public QueryParameterImplementor getParameter(int position, Class type) { checkOpenNoRollback(); - try { - final QueryParameterImplementor parameter = getParameterMetadata().getQueryParameter( position ); + final var parameter = getParameterMetadata().getQueryParameter( position ); if ( !type.isAssignableFrom( parameter.getParameterType() ) ) { - throw new IllegalArgumentException( - "The type [" + parameter.getParameterType().getName() + - "] associated with the parameter corresponding to position [" + position + - "] is not assignable to requested Java type [" + type.getName() + "]" + throw new QueryArgumentTypeException( + "Type specified for parameter at position " + position + " is incompatible", + parameter.getParameterType(), + type ); } - return parameter; + @SuppressWarnings("unchecked") // safe, just checked + var castParameter = (QueryParameterImplementor) parameter; + return castParameter; } catch ( HibernateException e ) { throw getExceptionConverter().convert( e ); @@ -703,15 +701,31 @@ private

JavaType

getJavaType(Class

javaType) { .resolveDescriptor( javaType ); } - protected

QueryParameterBinding

locateBinding(Parameter

parameter) { + protected

QueryParameterBinding

locateBinding(Parameter

parameter, P value) { if ( parameter instanceof QueryParameterImplementor

parameterImplementor ) { return locateBinding( parameterImplementor ); } else if ( parameter.getName() != null ) { - return locateBinding( parameter.getName() ); + return locateBinding( parameter.getName(), parameter.getParameterType(), value ); } else if ( parameter.getPosition() != null ) { - return locateBinding( parameter.getPosition() ); + return locateBinding( parameter.getPosition(), parameter.getParameterType(), value ); + } + + throw getExceptionConverter().convert( + new IllegalArgumentException( "Could not resolve binding for given parameter reference [" + parameter + "]" ) + ); + } + + protected

QueryParameterBinding

locateBinding(Parameter

parameter, Collection values) { + if ( parameter instanceof QueryParameterImplementor

parameterImplementor ) { + return locateBinding( parameterImplementor ); + } + else if ( parameter.getName() != null ) { + return locateBinding( parameter.getName(), parameter.getParameterType(), values ); + } + else if ( parameter.getPosition() != null ) { + return locateBinding( parameter.getPosition(), parameter.getParameterType(), values ); } throw getExceptionConverter().convert( @@ -724,14 +738,76 @@ protected

QueryParameterBinding

locateBinding(QueryParameterImplementor

QueryParameterBinding

locateBinding(String name) { + protected

QueryParameterBinding

locateBinding(String name, Class

javaType, P value) { + getCheckOpen(); + final var binding = getQueryParameterBindings().getBinding( name ); + final var parameterType = binding.getBindType(); + if ( parameterType != null ) { + final var parameterJavaType = parameterType.getJavaType(); + if ( !parameterJavaType.isAssignableFrom( javaType ) + && !isInstance( parameterType, value ) ) { + throw new QueryArgumentTypeException( + "Argument to parameter named '" + name + "' has an incompatible type", + parameterJavaType, javaType); + } + } + @SuppressWarnings("unchecked") // safe, just checked + var castBinding = (QueryParameterBinding

) binding; + return castBinding; + } + + protected

QueryParameterBinding

locateBinding(int position, Class

javaType, P value) { + getCheckOpen(); + final var binding = getQueryParameterBindings().getBinding( position ); + final var parameterType = binding.getBindType(); + if ( parameterType != null ) { + final var parameterJavaType = parameterType.getJavaType(); + if ( !parameterJavaType.isAssignableFrom( javaType ) + && !isInstance( parameterType, value ) ) { + throw new QueryArgumentTypeException( + "Argument to parameter at position " + position + " has an incompatible type", + parameterJavaType, javaType ); + } + } + @SuppressWarnings("unchecked") // safe, just checked + var castBinding = (QueryParameterBinding

) binding; + return castBinding; + } + + protected

QueryParameterBinding

locateBinding(String name, Class

javaType, Collection values) { getCheckOpen(); - return getQueryParameterBindings().getBinding( name ); + final var binding = getQueryParameterBindings().getBinding( name ); + final var parameterType = binding.getBindType(); + if ( parameterType != null ) { + final var parameterJavaType = parameterType.getJavaType(); + if ( !parameterJavaType.isAssignableFrom( javaType ) + && !areInstances( parameterType, values ) ) { + throw new QueryArgumentTypeException( + "Argument to parameter named '" + name + "' has an incompatible type", + parameterJavaType, javaType); + } + } + @SuppressWarnings("unchecked") // safe, just checked + var castBinding = (QueryParameterBinding

) binding; + return castBinding; } - protected

QueryParameterBinding

locateBinding(int position) { + protected

QueryParameterBinding

locateBinding(int position, Class

javaType, Collection values) { getCheckOpen(); - return getQueryParameterBindings().getBinding( position ); + final var binding = getQueryParameterBindings().getBinding( position ); + final var parameterType = binding.getBindType(); + if ( parameterType != null ) { + final var parameterJavaType = parameterType.getJavaType(); + if ( !parameterJavaType.isAssignableFrom( javaType ) + && !areInstances( parameterType, values ) ) { + throw new QueryArgumentTypeException( + "Argument to parameter at position " + position + " has an incompatible type", + parameterJavaType, javaType ); + } + } + @SuppressWarnings("unchecked") // safe, just checked + var castBinding = (QueryParameterBinding

) binding; + return castBinding; } protected

QueryParameterImplementor

getQueryParameter(QueryParameterImplementor

parameter) { @@ -741,23 +817,24 @@ protected

QueryParameterImplementor

getQueryParameter(QueryParameterImple public boolean isBound(Parameter param) { getCheckOpen(); final var parameter = getParameterMetadata().resolve( param ); - return parameter != null && getQueryParameterBindings().isBound( getQueryParameter( parameter ) ); + return parameter != null + && getQueryParameterBindings().isBound( getQueryParameter( parameter ) ); } public T getParameterValue(Parameter param) { checkOpenNoRollback(); - final var parameter = getParameterMetadata().resolve( param ); if ( parameter == null ) { throw new IllegalArgumentException( "The parameter [" + param + "] is not part of this Query" ); } - - final var binding = getQueryParameterBindings().getBinding( getQueryParameter( parameter ) ); + final var binding = + getQueryParameterBindings() + .getBinding( getQueryParameter( parameter ) ); if ( binding == null || !binding.isBound() ) { throw new IllegalStateException( "Parameter value not yet bound : " + param.toString() ); } - if ( binding.isMultiValued() ) { + // TODO: this cast is complete garbage //noinspection unchecked return (T) binding.getBindValues(); } @@ -768,54 +845,69 @@ public T getParameterValue(Parameter param) { public Object getParameterValue(String name) { checkOpenNoRollback(); - - final QueryParameterBinding binding = getQueryParameterBindings().getBinding( name ); + final var binding = getQueryParameterBindings().getBinding( name ); if ( !binding.isBound() ) { throw new IllegalStateException( "The parameter [" + name + "] has not yet been bound" ); } - - if ( binding.isMultiValued() ) { - return binding.getBindValues(); - } - else { - return binding.getBindValue(); - } + return binding.isMultiValued() ? binding.getBindValues() : binding.getBindValue(); } public Object getParameterValue(int position) { checkOpenNoRollback(); - - final QueryParameterBinding binding = getQueryParameterBindings().getBinding( position ); + final var binding = getQueryParameterBindings().getBinding( position ); if ( !binding.isBound() ) { throw new IllegalStateException( "The parameter [" + position + "] has not yet been bound" ); } - return binding.isMultiValued() ? binding.getBindValues() : binding.getBindValue(); } + + private

void setParameterValue(Object value, QueryParameterBinding

binding) { + final var parameterType = binding.getBindType(); + if ( parameterType != null + && !isInstanceOrAreInstances( value, binding, parameterType ) ) { + throw new QueryArgumentTypeException( "Argument to query parameter has an incompatible type", + parameterType.getJavaType(), value.getClass() ); + } + @SuppressWarnings("unchecked") // safe, just checked + final var castValue = (P) value; + binding.setBindValue( castValue, resolveJdbcParameterTypeIfNecessary() ); + } + + private

boolean isInstanceOrAreInstances( + Object value, QueryParameterBinding

binding, BindableType parameterType) { + return binding.isMultiValued() && value instanceof Collection values + ? areInstances( parameterType, values ) + : isInstance( parameterType, value ); + } + @Override public CommonQueryContract setParameter(String name, Object value) { checkOpenNoRollback(); - if ( value instanceof TypedParameterValue typedParameterValue ) { setTypedParameter( name, typedParameterValue ); } else { - final QueryParameterBinding binding = getQueryParameterBindings().getBinding( name ); - if ( multipleBinding( binding.getQueryParameter(), value ) - && value instanceof Collection collectionValue - && !isRegisteredAsBasicType( value.getClass() ) ) { - return setParameterList( name, collectionValue ); + final var binding = getQueryParameterBindings().getBinding( name ); + if ( multipleBinding( binding.getQueryParameter(), value ) ) { + setParameterList( name, (Collection) value ); + } + else { + setParameterValue( value, binding ); } - binding.setBindValue( value, resolveJdbcParameterTypeIfNecessary() ); } - return this; } - private boolean multipleBinding(QueryParameter parameter, Object value){ - if ( parameter.allowsMultiValuedBinding() ) { + private boolean multipleBinding(QueryParameter parameter, Object value){ + if ( parameter.allowsMultiValuedBinding() + && value instanceof Collection values + // this check only needed for some native queries + && !isRegisteredAsBasicType( value.getClass() ) ) { final var hibernateType = parameter.getHibernateType(); - return hibernateType == null || value == null || isInstance( hibernateType, value ); + return hibernateType == null + || values.isEmpty() + || !isInstance( hibernateType, value ) + || isInstance( hibernateType, values.iterator().next() ); } else { return false; @@ -831,12 +923,67 @@ private void setTypedParameter(int position, TypedParameterValue typedVal } private boolean isInstance(Type parameterType, Object value) { - final SqmExpressible sqmExpressible = getNodeuilder().resolveExpressible( parameterType ); + if ( value == null ) { + return true; + } + final var sqmExpressible = getNodeBuilder().resolveExpressible( parameterType ); + assert sqmExpressible != null; + final var javaType = sqmExpressible.getExpressibleJavaType(); + if ( !javaType.isInstance( value ) ) { + try { + // if this succeeds, we are good + javaType.wrap( value, session ); + } + catch ( HibernateException|UnsupportedOperationException e) { + return false; + } + } + return true; + } + + private boolean areInstances(Type parameterType, Collection values) { + if ( values.isEmpty() ) { + return true; + } + final var sqmExpressible = getNodeBuilder().resolveExpressible( parameterType ); assert sqmExpressible != null; - return sqmExpressible.getExpressibleJavaType().isInstance( value ); + final var javaType = sqmExpressible.getExpressibleJavaType(); + for ( Object value : values ) { + if ( !javaType.isInstance( value ) ) { + try { + // if this succeeds, we are good + javaType.wrap( value, session ); + } + catch (HibernateException | UnsupportedOperationException e) { + return false; + } + } + } + return true; + } + + private boolean areInstances(Type parameterType, Object[] values) { + if ( values.length == 0 ) { + return true; + } + final var sqmExpressible = getNodeBuilder().resolveExpressible( parameterType ); + assert sqmExpressible != null; + final var javaType = sqmExpressible.getExpressibleJavaType(); + for ( Object value : values ) { + if ( !javaType.isInstance( value ) ) { + try { + // if this succeeds, we are good + javaType.wrap( value, session ); + } + catch (HibernateException | UnsupportedOperationException e) { + return false; + } + } + } + return true; } - private NodeBuilder getNodeuilder() { + private NodeBuilder getNodeBuilder() { return getSessionFactory().getQueryEngine().getCriteriaBuilder(); } @@ -854,31 +1001,30 @@ public

CommonQueryContract setParameter(String name, P value, Class

javaT @Override public

CommonQueryContract setParameter(String name, P value, Type

type) { - this.

locateBinding( name ).setBindValue( value, (BindableType

) type ); + locateBinding( name, type.getJavaType(), value ).setBindValue( value, (BindableType

) type ); return this; } @Override @Deprecated(since = "7") public CommonQueryContract setParameter(String name, Instant value, TemporalType temporalType) { - this.locateBinding( name ).setBindValue( value, temporalType ); + locateBinding( name, Instant.class, value ).setBindValue( value, temporalType ); return this; } @Override public CommonQueryContract setParameter(int position, Object value) { checkOpenNoRollback(); - if ( value instanceof TypedParameterValue typedParameterValue ) { setTypedParameter( position, typedParameterValue ); } else { - final QueryParameterBinding binding = getQueryParameterBindings().getBinding( position ); - if ( multipleBinding( binding.getQueryParameter(), value ) - && value instanceof Collection collectionValue - && !isRegisteredAsBasicType( value.getClass() ) ) { - return setParameterList( position, collectionValue ); + final var binding = getQueryParameterBindings().getBinding( position ); + if ( multipleBinding( binding.getQueryParameter(), value ) ) { + setParameterList( position, (Collection) value ); + } + else { + setParameterValue( value, binding ); } - binding.setBindValue( value, resolveJdbcParameterTypeIfNecessary() ); } return this; } @@ -901,19 +1047,20 @@ public

CommonQueryContract setParameter(int position, P value, Class

java @Override public

CommonQueryContract setParameter(int position, P value, Type

type) { - this.

locateBinding( position ).setBindValue( value, (BindableType

) type ); + locateBinding( position, type.getJavaType(), value ).setBindValue( value, (BindableType

) type ); return this; } @Override @Deprecated(since = "7") public CommonQueryContract setParameter(int position, Instant value, TemporalType temporalType) { - this.locateBinding( position ).setBindValue( value, temporalType ); + locateBinding( position, Instant.class, value ).setBindValue( value, temporalType ); return this; } @Override public

CommonQueryContract setParameter(QueryParameter

parameter, P value) { - locateBinding( parameter ).setBindValue( value, resolveJdbcParameterTypeIfNecessary() ); + locateBinding( parameter, value ) + .setBindValue( value, resolveJdbcParameterTypeIfNecessary() ); return this; } @@ -931,7 +1078,8 @@ public

CommonQueryContract setParameter(QueryParameter

parameter, P value @Override public

CommonQueryContract setParameter(QueryParameter

parameter, P value, Type

type) { - locateBinding( parameter ).setBindValue( value, (BindableType

) type ); + locateBinding( parameter, value ) + .setBindValue( value, (BindableType

) type ); return this; } @@ -950,75 +1098,67 @@ public

CommonQueryContract setParameter(Parameter

parameter, P value) { } @SuppressWarnings("unchecked") // safe, because we just checked final var typedValue = (TypedParameterValue

) value; - setParameter( parameter, typedValue.value(), typedValue.type() ); + final var castValue = typedValue.value(); + final var castType = typedValue.type(); + if ( parameter instanceof QueryParameter

queryParameter ) { + setParameter( queryParameter, castValue, castType ); + } + else { + locateBinding( parameter, castValue ) + .setBindValue( castValue, castType ); + } } else { - locateBinding( parameter ).setBindValue( value, resolveJdbcParameterTypeIfNecessary() ); + locateBinding( parameter, value ) + .setBindValue( value, resolveJdbcParameterTypeIfNecessary() ); } return this; } - private

void setParameter(Parameter

parameter, P value, Type

type) { - if ( parameter instanceof QueryParameter

queryParameter ) { - setParameter( queryParameter, value, type ); - } - else if ( value == null ) { - locateBinding( parameter ).setBindValue( null, (BindableType

) type ); - } - else if ( value instanceof Collection ) { - //TODO: this looks wrong to me: how can value be both a P and a (Collection

)? - locateBinding( parameter ).setBindValues( (Collection

) value ); - } - else { - locateBinding( parameter ).setBindValue( value, (BindableType

) type ); - } - } - - @Override @Deprecated public CommonQueryContract setParameter(Parameter param, Calendar value, TemporalType temporalType) { - locateBinding( param ).setBindValue( value, temporalType ); + locateBinding( param, value ).setBindValue( value, temporalType ); return this; } @Override @Deprecated public CommonQueryContract setParameter(Parameter param, Date value, TemporalType temporalType) { - locateBinding( param ).setBindValue( value, temporalType ); + locateBinding( param, value ).setBindValue( value, temporalType ); return this; } @Override @Deprecated public CommonQueryContract setParameter(String name, Calendar value, TemporalType temporalType) { - locateBinding( name ).setBindValue( value, temporalType ); + locateBinding( name, Calendar.class, value ).setBindValue( value, temporalType ); return this; } @Override @Deprecated public CommonQueryContract setParameter(String name, Date value, TemporalType temporalType) { - locateBinding( name ).setBindValue( value, temporalType ); + locateBinding( name, Date.class, value ).setBindValue( value, temporalType ); return this; } @Override @Deprecated public CommonQueryContract setParameter(int position, Calendar value, TemporalType temporalType) { - locateBinding( position ).setBindValue( value, temporalType ); + locateBinding( position, Calendar.class, value ).setBindValue( value, temporalType ); return this; } @Override @Deprecated public CommonQueryContract setParameter(int position, Date value, TemporalType temporalType) { - locateBinding( position ).setBindValue( value, temporalType ); + locateBinding( position, Date.class, value ).setBindValue( value, temporalType ); return this; } @Override public CommonQueryContract setParameterList(String name, @SuppressWarnings("rawtypes") Collection values) { - locateBinding( name ).setBindValues( values ); + getQueryParameterBindings().getBinding( name ).setBindValues( values ); return this; } public

CommonQueryContract setParameterList(String name, Collection values, Class

javaType) { - final JavaType

javaDescriptor = getJavaType( javaType ); + final var javaDescriptor = getJavaType( javaType ); if ( javaDescriptor == null ) { setParameterList( name, values ); } @@ -1031,16 +1171,35 @@ public

CommonQueryContract setParameterList(String name, Collection CommonQueryContract setParameterList(String name, Collection values, Type

type) { - this.

locateBinding( name ).setBindValues( values, (BindableType

) type ); + locateBinding( name, type.getJavaType(), values ) + .setBindValues( values, (BindableType

) type ); return this; } @Override public CommonQueryContract setParameterList(String name, Object[] values) { - locateBinding( name ).setBindValues( asList( values ) ); + final var binding = getQueryParameterBindings().getBinding( name ); + setParameterValues( values, binding ); return this; } + private

void setParameterValues(Object[] values, QueryParameterBinding

binding) { + final var parameterType = binding.getBindType(); + if ( parameterType != null + && !areInstances( values, parameterType ) ) { + throw new QueryArgumentTypeException( "Argument to query parameter has an incompatible type", + parameterType.getJavaType(), values.getClass().getComponentType() ); + } + @SuppressWarnings("unchecked") // safe, just checked + final var castArray = (P[]) values; + binding.setBindValues( List.of( castArray ) ); + } + + private

boolean areInstances(Object[] values, BindableType parameterType) { + return parameterType.getJavaType().isAssignableFrom( values.getClass().getComponentType() ) + || areInstances( parameterType, values ); + } + @Override public

CommonQueryContract setParameterList(String name, P[] values, Class

javaType) { final var javaDescriptor = getJavaType( javaType ); @@ -1054,13 +1213,16 @@ public

CommonQueryContract setParameterList(String name, P[] values, Class

CommonQueryContract setParameterList(String name, P[] values, Type

type) { - this.

locateBinding( name ).setBindValues( asList( values ), (BindableType

) type ); + final var list = List.of( values ); + locateBinding( name, type.getJavaType(), list ) + .setBindValues( list, (BindableType

) type ); return this; } @Override public CommonQueryContract setParameterList(int position, @SuppressWarnings("rawtypes") Collection values) { - locateBinding( position ).setBindValues( values ); + //TODO: type checking? + getQueryParameterBindings().getBinding( position ).setBindValues( values ); return this; } @@ -1098,13 +1260,15 @@ private

Type

getParamType(Class

javaType) { @Override public

CommonQueryContract setParameterList(int position, Collection values, Type

type) { - this.

locateBinding( position ).setBindValues( values, (BindableType

) type ); + locateBinding( position, type.getJavaType(), values ) + .setBindValues( values, (BindableType

) type ); return this; } @Override public CommonQueryContract setParameterList(int position, Object[] values) { - locateBinding( position ).setBindValues( asList( values ) ); + final var binding = getQueryParameterBindings().getBinding( position ); + setParameterValues( values, binding ); return this; } @@ -1117,20 +1281,20 @@ public

CommonQueryContract setParameterList(int position, P[] values, Class< else { setParameterList( position, values, getParamType( javaType ) ); } - return this; } - @SuppressWarnings({ "unchecked", "rawtypes" }) public

CommonQueryContract setParameterList(int position, P[] values, Type

type) { - locateBinding( position ).setBindValues( asList( values ), (BindableType) type ); + final var list = List.of( values ); + locateBinding( position, type.getJavaType(), list ) + .setBindValues( list, (BindableType

) type ); return this; } @Override public

CommonQueryContract setParameterList(QueryParameter

parameter, Collection values) { - locateBinding( parameter ).setBindValues( values ); + locateBinding( parameter, values ).setBindValues( values ); return this; } @@ -1148,13 +1312,15 @@ public

CommonQueryContract setParameterList(QueryParameter

parameter, Col @Override public

CommonQueryContract setParameterList(QueryParameter

parameter, Collection values, Type

type) { - locateBinding( parameter ).setBindValues( values, (BindableType

) type ); + locateBinding( parameter, values ) + .setBindValues( values, (BindableType

) type ); return this; } @Override public

CommonQueryContract setParameterList(QueryParameter

parameter, P[] values) { - locateBinding( parameter ).setBindValues( values == null ? null : asList( values ) ); + final var list = List.of( values ); + locateBinding( parameter, list ).setBindValues( list ); return this; } @@ -1167,13 +1333,14 @@ public

CommonQueryContract setParameterList(QueryParameter

parameter, P[] else { setParameterList( parameter, values, getParamType( javaType ) ); } - return this; } @Override public

CommonQueryContract setParameterList(QueryParameter

parameter, P[] values, Type

type) { - locateBinding( parameter ).setBindValues( asList( values ), (BindableType

) type ); + final var list = List.of( values ); + locateBinding( parameter, list ) + .setBindValues( list, (BindableType

) type ); return this; } @@ -1183,7 +1350,8 @@ public CommonQueryContract setProperties(@SuppressWarnings("rawtypes") Map map) final Object object = map.get( paramName ); if ( object == null ) { if ( map.containsKey( paramName ) ) { - setParameter( paramName, null, determineType( paramName, null ) ); + setParameter( paramName, null, + determineType( paramName, null ) ); } } else { @@ -1194,7 +1362,8 @@ else if ( object instanceof Object[] array ) { setParameterList( paramName, array ); } else { - setParameter( paramName, object, determineType( paramName, object.getClass() ) ); + setParameter( paramName, object, + determineType( paramName, object.getClass() ) ); } } } @@ -1202,14 +1371,15 @@ else if ( object instanceof Object[] array ) { } protected Type determineType(String namedParam, Class retType) { - BindableType type = locateBinding( namedParam ).getBindType(); + var type = getQueryParameterBindings().getBinding( namedParam ).getBindType(); if ( type == null ) { type = getParameterMetadata().getQueryParameter( namedParam ).getHibernateType(); } if ( type == null && retType != null ) { type = getSessionFactory().getMappingMetamodel().resolveParameterBindType( retType ); } - if ( retType!= null && !retType.isAssignableFrom( type.getJavaType() ) ) { + if ( retType != null && !retType.isAssignableFrom( type.getJavaType() ) ) { + // TODO: is this really the correct exception type? throw new IllegalStateException( "Parameter not of expected type: " + retType.getName() ); } //noinspection unchecked diff --git a/hibernate-core/src/main/java/org/hibernate/query/spi/QueryParameterBindings.java b/hibernate-core/src/main/java/org/hibernate/query/spi/QueryParameterBindings.java index 782eecfc15ff..faffc2c44f27 100644 --- a/hibernate-core/src/main/java/org/hibernate/query/spi/QueryParameterBindings.java +++ b/hibernate-core/src/main/java/org/hibernate/query/spi/QueryParameterBindings.java @@ -58,7 +58,7 @@ default

QueryParameterBinding

getBinding(QueryParameter

parameter) { * * @return The binding, or {@code null} if not yet bound */ -

QueryParameterBinding

getBinding(String name); + QueryParameterBinding getBinding(String name); /** * Access to the binding via position @@ -67,7 +67,7 @@ default

QueryParameterBinding

getBinding(QueryParameter

parameter) { * * @return The binding, or {@code null} if not yet bound */ -

QueryParameterBinding

getBinding(int position); + QueryParameterBinding getBinding(int position); /** * Validate the bindings. Called just before execution @@ -94,15 +94,15 @@ default

QueryParameterBinding

getBinding(QueryParameter

parameter) { * Currently unused and can be safely removed. */ @Deprecated(forRemoval = true, since = "6.6") - @SuppressWarnings({"rawtypes", "unchecked"}) +// @SuppressWarnings({"rawtypes", "unchecked"}) QueryParameterBindings NO_PARAM_BINDINGS = new QueryParameterBindings() { @Override - public boolean isBound(QueryParameterImplementor parameter) { + public boolean isBound(QueryParameterImplementor parameter) { return false; } @Override - public QueryParameterBinding getBinding(QueryParameterImplementor parameter) { + public

QueryParameterBinding

getBinding(QueryParameterImplementor

parameter) { return null; } @@ -117,7 +117,7 @@ public QueryParameterBinding getBinding(int position) { } @Override - public void visitBindings(BiConsumer action) { + public void visitBindings(BiConsumer, ? super QueryParameterBinding> action) { } @Override diff --git a/hibernate-core/src/main/java/org/hibernate/query/sqm/tree/expression/SqmNamedParameter.java b/hibernate-core/src/main/java/org/hibernate/query/sqm/tree/expression/SqmNamedParameter.java index 3b847e1df9d3..43bd225a3f06 100644 --- a/hibernate-core/src/main/java/org/hibernate/query/sqm/tree/expression/SqmNamedParameter.java +++ b/hibernate-core/src/main/java/org/hibernate/query/sqm/tree/expression/SqmNamedParameter.java @@ -75,7 +75,7 @@ public String getName() { @Override public SqmParameter copy() { - return new SqmNamedParameter<>( getName(), allowMultiValuedBinding(), this.getNodeType(), nodeBuilder() ); + return new SqmNamedParameter<>( getName(), allowMultiValuedBinding(), getNodeType(), nodeBuilder() ); } @Override diff --git a/hibernate-core/src/main/java/org/hibernate/type/descriptor/java/EnumJavaType.java b/hibernate-core/src/main/java/org/hibernate/type/descriptor/java/EnumJavaType.java index 6f8edb9dfe94..26a14b490a43 100644 --- a/hibernate-core/src/main/java/org/hibernate/type/descriptor/java/EnumJavaType.java +++ b/hibernate-core/src/main/java/org/hibernate/type/descriptor/java/EnumJavaType.java @@ -113,7 +113,12 @@ else if ( Short.class.equals( type ) ) { else if ( Byte.class.equals( type ) ) { return (X) toByte( value ); } - return (X) value; + else if ( type.isInstance( value )) { + return (X) value; + } + else { + throw unknownUnwrap( type ); + } } @Override @@ -140,8 +145,11 @@ else if ( value instanceof Byte byteValue ) { else if ( value instanceof Number number ) { return fromLong( number.longValue() ); } + else if ( getJavaType().isInstance( value ) ) { + return (T) value; + } else { - return (T) value; + throw unknownWrap( value.getClass() ); } } diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/hql/ParameterIsNullTest.java b/hibernate-core/src/test/java/org/hibernate/orm/test/hql/ParameterIsNullTest.java index 3fdfdc50434e..45cabc140e67 100644 --- a/hibernate-core/src/test/java/org/hibernate/orm/test/hql/ParameterIsNullTest.java +++ b/hibernate-core/src/test/java/org/hibernate/orm/test/hql/ParameterIsNullTest.java @@ -70,7 +70,7 @@ public void testEmptyCollectionParam(SessionFactoryScope scope) { } catch (Exception e) { assertThat( e ).isInstanceOf( IllegalArgumentException.class ); - assertThat( e.getMessage() ).contains( "Parameter value", "did not match expected type" ); + assertThat( e.getMessage() ).contains( "Argument to query parameter has an incompatible type", "is not assignable to" ); } } ); } diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/query/sqm/mutation/multitable/IdSelectionTests.java b/hibernate-core/src/test/java/org/hibernate/orm/test/query/sqm/mutation/multitable/IdSelectionTests.java index 4b21421868fa..288780b1dd95 100644 --- a/hibernate-core/src/test/java/org/hibernate/orm/test/query/sqm/mutation/multitable/IdSelectionTests.java +++ b/hibernate-core/src/test/java/org/hibernate/orm/test/query/sqm/mutation/multitable/IdSelectionTests.java @@ -12,6 +12,7 @@ import org.hibernate.query.internal.QueryParameterBindingsImpl; import org.hibernate.query.spi.DomainQueryExecutionContext; import org.hibernate.query.spi.QueryOptions; +import org.hibernate.query.spi.QueryParameterBinding; import org.hibernate.query.spi.QueryParameterBindings; import org.hibernate.query.sqm.internal.DomainParameterXref; import org.hibernate.query.sqm.mutation.internal.MatchingIdSelectionHelper; @@ -61,7 +62,8 @@ public void testSecondaryTableRestrictedOnRootTable(SessionFactoryScope scope) { parameterMetadata, scope.getSessionFactory() ); - domainParamBindings.getBinding( "n" ).setBindValue( "abc" ); + ((QueryParameterBinding) domainParamBindings.getBinding( "n" )) + .setBindValue( "abc" ); scope.inTransaction( session -> { @@ -86,7 +88,8 @@ public void testSecondaryTableRestrictedOnNonRootTable(SessionFactoryScope scope parameterMetadata, scope.getSessionFactory() ); - domainParamBindings.getBinding( "d" ).setBindValue( "123" ); + ((QueryParameterBinding) domainParamBindings.getBinding( "d" )) + .setBindValue( "123" ); scope.inTransaction( session -> { @@ -111,7 +114,8 @@ public void testJoinedSubclassRestrictedOnRootTable(SessionFactoryScope scope) { parameterMetadata, scope.getSessionFactory() ); - domainParamBindings.getBinding( "n" ).setBindValue( "Acme" ); + ((QueryParameterBinding) domainParamBindings.getBinding( "n" )) + .setBindValue( "Acme" ); scope.inTransaction( session -> { @@ -136,7 +140,8 @@ public void testJoinedSubclassRestrictedOnNonPrimaryRootTable(SessionFactoryScop parameterMetadata, scope.getSessionFactory() ); - domainParamBindings.getBinding( "n" ).setBindValue( "Acme" ); + ((QueryParameterBinding) domainParamBindings.getBinding( "n" )) + .setBindValue( "Acme" ); scope.inTransaction( session -> { @@ -161,7 +166,8 @@ public void testJoinedSubclassRestrictedOnPrimaryNonRootTable(SessionFactoryScop parameterMetadata, scope.getSessionFactory() ); - domainParamBindings.getBinding( "v" ).setBindValue( "123" ); + ((QueryParameterBinding) domainParamBindings.getBinding( "v" )) + .setBindValue( "123" ); scope.inTransaction( session -> { diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/type/LongListTypeContributorTest.java b/hibernate-core/src/test/java/org/hibernate/orm/test/type/LongListTypeContributorTest.java index b158d39cc1d7..8a64c726fcad 100644 --- a/hibernate-core/src/test/java/org/hibernate/orm/test/type/LongListTypeContributorTest.java +++ b/hibernate-core/src/test/java/org/hibernate/orm/test/type/LongListTypeContributorTest.java @@ -59,7 +59,7 @@ public void testParameterRegisteredCollection(SessionFactoryScope factoryScope) } ); factoryScope.inTransaction( em -> { - SpecialItem item = (SpecialItem) em.createNativeQuery( + SpecialItem item = em.createNativeQuery( "SELECT * FROM special_table WHERE long_list = :longList", SpecialItem.class ) .setParameter( "longList", longList ) .getSingleResult(); From 55df58903559ac20841c5322417737cce8be9f8f Mon Sep 17 00:00:00 2001 From: Gavin King Date: Mon, 1 Dec 2025 00:08:13 +0100 Subject: [PATCH 2/5] fix a user-written test that asserted some unreasonable things --- .../test/embeddable/EmbeddableWithJavaTypeTest.java | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/embeddable/EmbeddableWithJavaTypeTest.java b/hibernate-core/src/test/java/org/hibernate/orm/test/embeddable/EmbeddableWithJavaTypeTest.java index b077457e9ea6..a241312e6899 100644 --- a/hibernate-core/src/test/java/org/hibernate/orm/test/embeddable/EmbeddableWithJavaTypeTest.java +++ b/hibernate-core/src/test/java/org/hibernate/orm/test/embeddable/EmbeddableWithJavaTypeTest.java @@ -48,10 +48,10 @@ public void injectSessionFactoryScope(SessionFactoryScope scope) { @ValueSource(strings = { "select z from EntityEmbedCustom z where embedCustom.value=:datum", "select z from EntityEmbedCustom z where :datum=embedCustom.value", - "select z from EntityEmbedCustom z where embedCustom=:datum", // this query failed with the bug - "select z from EntityEmbedCustom z where :datum=embedCustom", + "select z from EntityEmbedCustom z where embedCustom.value=:datum", // this query failed with the bug + "select z from EntityEmbedCustom z where :datum=embedCustom.value", "select z from EntityEmbedCustom z where embedCustom.value in (:datum)", - "select z from EntityEmbedCustom z where embedCustom in (:datum)" // failed as well + "select z from EntityEmbedCustom z where embedCustom.value in (:datum)" // failed as well }) void hhh18898Test_embedCustom(String hql) { @@ -81,10 +81,10 @@ void hhh18898Test_embedCustom(String hql) { @ValueSource(strings = { "select z from EntityEmbedNative z where embedNative.value=:datum", "select z from EntityEmbedNative z where :datum=embedNative.value", - "select z from EntityEmbedNative z where embedNative=:datum", // this query failed with the bug - "select z from EntityEmbedNative z where :datum=embedNative", + "select z from EntityEmbedNative z where embedNative.value=:datum", // this query failed with the bug + "select z from EntityEmbedNative z where :datum=embedNative.value", "select z from EntityEmbedNative z where embedNative.value in (:datum)", - "select z from EntityEmbedNative z where embedNative in (:datum)" // failed as well + "select z from EntityEmbedNative z where embedNative.value in (:datum)" // failed as well }) void hhh18898Test_embedSingle(String hql) { From 8c21efd3938c1c7eee799271ac86c653bb6176ab Mon Sep 17 00:00:00 2001 From: Gavin King Date: Mon, 1 Dec 2025 00:36:04 +0100 Subject: [PATCH 3/5] simplify the exception model for query arguments/parameters --- .../query/QueryArgumentException.java | 15 +++- .../query/QueryArgumentTypeException.java | 31 ------- .../spi/AbstractCommonQueryContract.java | 39 ++++----- .../spi/QueryParameterBindingValidator.java | 87 +++++-------------- 4 files changed, 56 insertions(+), 116 deletions(-) delete mode 100644 hibernate-core/src/main/java/org/hibernate/query/QueryArgumentTypeException.java diff --git a/hibernate-core/src/main/java/org/hibernate/query/QueryArgumentException.java b/hibernate-core/src/main/java/org/hibernate/query/QueryArgumentException.java index 828f3ac61e17..1b6705d39364 100644 --- a/hibernate-core/src/main/java/org/hibernate/query/QueryArgumentException.java +++ b/hibernate-core/src/main/java/org/hibernate/query/QueryArgumentException.java @@ -15,11 +15,20 @@ */ public class QueryArgumentException extends IllegalArgumentException { private final Class parameterType; + private final Class argumentType; private final Object argument; public QueryArgumentException(String message, Class parameterType, Object argument) { - super(message); + super( message + " (argument [" + argument + "] is not assignable to " + parameterType.getName() + ")" ); this.parameterType = parameterType; + this.argumentType = argument == null ? null : argument.getClass(); + this.argument = argument; + } + + public QueryArgumentException(String message, Class parameterType, Class argumentType, Object argument) { + super( message + " (" + argumentType.getName() + " is not assignable to " + parameterType.getName() + ")" ); + this.parameterType = parameterType; + this.argumentType = argumentType; this.argument = argument; } @@ -27,6 +36,10 @@ public Class getParameterType() { return parameterType; } + public Class getArgumentType() { + return argumentType; + } + public Object getArgument() { return argument; } diff --git a/hibernate-core/src/main/java/org/hibernate/query/QueryArgumentTypeException.java b/hibernate-core/src/main/java/org/hibernate/query/QueryArgumentTypeException.java deleted file mode 100644 index 150d782cdbd3..000000000000 --- a/hibernate-core/src/main/java/org/hibernate/query/QueryArgumentTypeException.java +++ /dev/null @@ -1,31 +0,0 @@ -/* - * SPDX-License-Identifier: Apache-2.0 - * Copyright Red Hat Inc. and Hibernate Authors - */ -package org.hibernate.query; - -/** - * Thrown when an argument bound to a query parameter is of an incompatible type. - * - * @since 7.2 - * - * @author Gavin King - */ -public class QueryArgumentTypeException extends IllegalArgumentException { - private final Class expectedType; - private final Class actualType; - - public QueryArgumentTypeException(String message, Class expectedType, Class actualType) { - super( message + " (" + actualType.getName() + " is not assignable to " + expectedType.getName() + ")" ); - this.expectedType = expectedType; - this.actualType = actualType; - } - - public Class getExpectedType() { - return expectedType; - } - - public Class getActualType() { - return actualType; - } -} diff --git a/hibernate-core/src/main/java/org/hibernate/query/spi/AbstractCommonQueryContract.java b/hibernate-core/src/main/java/org/hibernate/query/spi/AbstractCommonQueryContract.java index c8a0bfdd6efb..3c9b7c9d6a49 100644 --- a/hibernate-core/src/main/java/org/hibernate/query/spi/AbstractCommonQueryContract.java +++ b/hibernate-core/src/main/java/org/hibernate/query/spi/AbstractCommonQueryContract.java @@ -35,7 +35,6 @@ import org.hibernate.query.QueryFlushMode; import org.hibernate.query.QueryParameter; import org.hibernate.query.TypedParameterValue; -import org.hibernate.query.QueryArgumentTypeException; import org.hibernate.query.criteria.JpaExpression; import org.hibernate.query.internal.QueryOptionsImpl; import org.hibernate.query.sqm.NodeBuilder; @@ -643,10 +642,9 @@ public QueryParameterImplementor getParameter(String name, Class type) try { final var parameter = getParameterMetadata().getQueryParameter( name ); if ( !type.isAssignableFrom( parameter.getParameterType() ) ) { - throw new QueryArgumentTypeException( - "Type specified for parameter named '" + name + "' is incompatible", - parameter.getParameterType(), - type + throw new IllegalArgumentException( + "Type specified for parameter named '" + name + "' is incompatible" + + " (" + parameter.getParameterType().getName() + " is not assignable to " + type.getName() + ")" ); } @SuppressWarnings("unchecked") // safe, just checked @@ -673,10 +671,9 @@ public QueryParameterImplementor getParameter(int position, Class type try { final var parameter = getParameterMetadata().getQueryParameter( position ); if ( !type.isAssignableFrom( parameter.getParameterType() ) ) { - throw new QueryArgumentTypeException( - "Type specified for parameter at position " + position + " is incompatible", - parameter.getParameterType(), - type + throw new IllegalArgumentException( + "Type specified for parameter at position " + position + " is incompatible" + + " (" + parameter.getParameterType().getName() + " is not assignable to " + type.getName() + ")" ); } @SuppressWarnings("unchecked") // safe, just checked @@ -746,9 +743,9 @@ protected

QueryParameterBinding

locateBinding(String name, Class

javaT final var parameterJavaType = parameterType.getJavaType(); if ( !parameterJavaType.isAssignableFrom( javaType ) && !isInstance( parameterType, value ) ) { - throw new QueryArgumentTypeException( + throw new QueryArgumentException( "Argument to parameter named '" + name + "' has an incompatible type", - parameterJavaType, javaType); + parameterJavaType, javaType, value ); } } @SuppressWarnings("unchecked") // safe, just checked @@ -764,9 +761,9 @@ protected

QueryParameterBinding

locateBinding(int position, Class

java final var parameterJavaType = parameterType.getJavaType(); if ( !parameterJavaType.isAssignableFrom( javaType ) && !isInstance( parameterType, value ) ) { - throw new QueryArgumentTypeException( + throw new QueryArgumentException( "Argument to parameter at position " + position + " has an incompatible type", - parameterJavaType, javaType ); + parameterJavaType, javaType, value ); } } @SuppressWarnings("unchecked") // safe, just checked @@ -782,9 +779,9 @@ protected

QueryParameterBinding

locateBinding(String name, Class

javaT final var parameterJavaType = parameterType.getJavaType(); if ( !parameterJavaType.isAssignableFrom( javaType ) && !areInstances( parameterType, values ) ) { - throw new QueryArgumentTypeException( + throw new QueryArgumentException( "Argument to parameter named '" + name + "' has an incompatible type", - parameterJavaType, javaType); + parameterJavaType, javaType, values ); } } @SuppressWarnings("unchecked") // safe, just checked @@ -800,9 +797,9 @@ protected

QueryParameterBinding

locateBinding(int position, Class

java final var parameterJavaType = parameterType.getJavaType(); if ( !parameterJavaType.isAssignableFrom( javaType ) && !areInstances( parameterType, values ) ) { - throw new QueryArgumentTypeException( + throw new QueryArgumentException( "Argument to parameter at position " + position + " has an incompatible type", - parameterJavaType, javaType ); + parameterJavaType, javaType, values ); } } @SuppressWarnings("unchecked") // safe, just checked @@ -865,8 +862,8 @@ private

void setParameterValue(Object value, QueryParameterBinding

bindin final var parameterType = binding.getBindType(); if ( parameterType != null && !isInstanceOrAreInstances( value, binding, parameterType ) ) { - throw new QueryArgumentTypeException( "Argument to query parameter has an incompatible type", - parameterType.getJavaType(), value.getClass() ); + throw new QueryArgumentException( "Argument to query parameter has an incompatible type", + parameterType.getJavaType(), value.getClass(), value ); } @SuppressWarnings("unchecked") // safe, just checked final var castValue = (P) value; @@ -1187,8 +1184,8 @@ private

void setParameterValues(Object[] values, QueryParameterBinding

bi final var parameterType = binding.getBindType(); if ( parameterType != null && !areInstances( values, parameterType ) ) { - throw new QueryArgumentTypeException( "Argument to query parameter has an incompatible type", - parameterType.getJavaType(), values.getClass().getComponentType() ); + throw new QueryArgumentException( "Argument to query parameter has an incompatible type", + parameterType.getJavaType(), values.getClass().getComponentType(), values ); } @SuppressWarnings("unchecked") // safe, just checked final var castArray = (P[]) values; diff --git a/hibernate-core/src/main/java/org/hibernate/query/spi/QueryParameterBindingValidator.java b/hibernate-core/src/main/java/org/hibernate/query/spi/QueryParameterBindingValidator.java index 83d1d482e522..ee4b10cf97bd 100644 --- a/hibernate-core/src/main/java/org/hibernate/query/spi/QueryParameterBindingValidator.java +++ b/hibernate-core/src/main/java/org/hibernate/query/spi/QueryParameterBindingValidator.java @@ -9,7 +9,6 @@ import java.util.Date; import org.hibernate.query.QueryArgumentException; -import org.hibernate.query.sqm.SqmBindableType; import org.hibernate.type.BindableType; import org.hibernate.type.BindingContext; import org.hibernate.type.descriptor.java.JavaType; @@ -40,8 +39,8 @@ public void validate( return; } - final SqmBindableType sqmExpressible = bindingContext.resolveExpressible( paramType ); - final Class parameterJavaType = + final var sqmExpressible = bindingContext.resolveExpressible( paramType ); + final var parameterJavaType = paramType.getJavaType() != null ? paramType.getJavaType() : sqmExpressible.getJavaType(); @@ -50,9 +49,9 @@ public void validate( if ( bind instanceof Collection collection && !Collection.class.isAssignableFrom( parameterJavaType ) ) { // we have a collection passed in where we are expecting a non-collection. - // NOTE : this can happen in Hibernate's notion of "parameter list" binding - // NOTE2 : the case of a collection value and an expected collection (if that can even happen) - // will fall through to the main check. + // NOTE: this can happen in Hibernate's notion of "parameter list" binding + // NOTE2: the case of a collection value and an expected collection (if that can even happen) + // will fall through to the main check. validateCollectionValuedParameterBinding( parameterJavaType, collection, @@ -72,17 +71,8 @@ else if ( !isValidBindValue( bind, temporalPrecision ) ) { - throw new QueryArgumentException( - String.format( - "Argument [%s] of type [%s] did not match parameter type [%s (%s)]", - bind, - bind.getClass().getName(), - parameterJavaType.getName(), - extractName( temporalPrecision ) - ), - parameterJavaType, - bind - ); + throw new QueryArgumentException( "Argument did not match parameter type", + parameterJavaType, bind ); } } // else nothing we can check @@ -99,17 +89,8 @@ private void validateCollectionValuedParameterBinding( // validate the elements... for ( Object element : value ) { if ( !isValidBindValue( parameterType, element, temporalType ) ) { - throw new QueryArgumentException( - String.format( - "Parameter value element [%s] did not match expected type [%s (%s)]", - element, - parameterType.getName(), - extractName( temporalType ) - ) - , - parameterType, - element - ); + throw new QueryArgumentException( "Argument element did not match parameter type", + parameterType, element ); } } } @@ -175,11 +156,12 @@ else if ( expectedType.isInstance( value ) ) { return true; } else if ( temporalType != null ) { - final boolean parameterDeclarationIsTemporal = Date.class.isAssignableFrom( expectedType ) + final boolean parameterDeclarationIsTemporal = + Date.class.isAssignableFrom( expectedType ) || Calendar.class.isAssignableFrom( expectedType ); - final boolean bindIsTemporal = value instanceof Date + final boolean bindIsTemporal = + value instanceof Date || value instanceof Calendar; - return parameterDeclarationIsTemporal && bindIsTemporal; } @@ -191,31 +173,18 @@ private void validateArrayValuedParameterBinding( Object value, TemporalType temporalType) { if ( !parameterType.isArray() ) { - throw new QueryArgumentException( - String.format( - "Encountered array-valued parameter binding, but was expecting [%s (%s)]", - parameterType.getName(), - extractName( temporalType ) - ), - parameterType, - value - ); + throw new QueryArgumentException( "Unexpected array-valued parameter binding", + parameterType, value ); } - if ( value.getClass().getComponentType().isPrimitive() ) { + final var componentType = value.getClass().getComponentType(); + final var parameterComponentType = parameterType.getComponentType(); + if ( componentType.isPrimitive() ) { // we have a primitive array. we validate that the actual array has the component type (type of elements) // we expect based on the component type of the parameter specification - if ( !parameterType.getComponentType().isAssignableFrom( value.getClass().getComponentType() ) ) { - throw new QueryArgumentException( - String.format( - "Primitive array-valued parameter bind value type [%s] did not match expected type [%s (%s)]", - value.getClass().getComponentType().getName(), - parameterType.getName(), - extractName( temporalType ) - ), - parameterType, - value - ); + if ( !parameterComponentType.isAssignableFrom( componentType ) ) { + throw new QueryArgumentException( "Primitive array-valued argument type did not match parameter type", + parameterType, value ); } } else { @@ -223,17 +192,9 @@ private void validateArrayValuedParameterBinding( // the type we expect based on the component type of the parameter specification final Object[] array = (Object[]) value; for ( Object element : array ) { - if ( !isValidBindValue( parameterType.getComponentType(), element, temporalType ) ) { - throw new QueryArgumentException( - String.format( - "Array-valued parameter value element [%s] did not match expected type [%s (%s)]", - element, - parameterType.getName(), - extractName( temporalType ) - ), - parameterType, - array - ); + if ( !isValidBindValue( parameterComponentType, element, temporalType ) ) { + throw new QueryArgumentException( "Array-valued argument type did not match parameter type", + parameterType, array ); } } } From 8c3c33633212cbe9d7ac637bb81012327eb77968 Mon Sep 17 00:00:00 2001 From: Gavin King Date: Mon, 1 Dec 2025 00:43:13 +0100 Subject: [PATCH 4/5] very minor code refactoring --- .../spi/AbstractCommonQueryContract.java | 43 ++++++++++++------- 1 file changed, 28 insertions(+), 15 deletions(-) diff --git a/hibernate-core/src/main/java/org/hibernate/query/spi/AbstractCommonQueryContract.java b/hibernate-core/src/main/java/org/hibernate/query/spi/AbstractCommonQueryContract.java index 3c9b7c9d6a49..954d12fd244a 100644 --- a/hibernate-core/src/main/java/org/hibernate/query/spi/AbstractCommonQueryContract.java +++ b/hibernate-core/src/main/java/org/hibernate/query/spi/AbstractCommonQueryContract.java @@ -641,10 +641,11 @@ public QueryParameterImplementor getParameter(String name, Class type) checkOpenNoRollback(); try { final var parameter = getParameterMetadata().getQueryParameter( name ); - if ( !type.isAssignableFrom( parameter.getParameterType() ) ) { + final var parameterType = parameter.getParameterType(); + if ( !type.isAssignableFrom( parameterType ) ) { throw new IllegalArgumentException( "Type specified for parameter named '" + name + "' is incompatible" - + " (" + parameter.getParameterType().getName() + " is not assignable to " + type.getName() + ")" + + " (" + parameterType.getName() + " is not assignable to " + type.getName() + ")" ); } @SuppressWarnings("unchecked") // safe, just checked @@ -670,10 +671,11 @@ public QueryParameterImplementor getParameter(int position, Class type checkOpenNoRollback(); try { final var parameter = getParameterMetadata().getQueryParameter( position ); - if ( !type.isAssignableFrom( parameter.getParameterType() ) ) { + final var parameterType = parameter.getParameterType(); + if ( !type.isAssignableFrom( parameterType ) ) { throw new IllegalArgumentException( "Type specified for parameter at position " + position + " is incompatible" - + " (" + parameter.getParameterType().getName() + " is not assignable to " + type.getName() + ")" + + " (" + parameterType.getName() + " is not assignable to " + type.getName() + ")" ); } @SuppressWarnings("unchecked") // safe, just checked @@ -702,11 +704,16 @@ protected

QueryParameterBinding

locateBinding(Parameter

parameter, P v if ( parameter instanceof QueryParameterImplementor

parameterImplementor ) { return locateBinding( parameterImplementor ); } - else if ( parameter.getName() != null ) { - return locateBinding( parameter.getName(), parameter.getParameterType(), value ); - } - else if ( parameter.getPosition() != null ) { - return locateBinding( parameter.getPosition(), parameter.getParameterType(), value ); + else { + final String name = parameter.getName(); + final Integer position = parameter.getPosition(); + final var parameterType = parameter.getParameterType(); + if ( name != null ) { + return locateBinding( name, parameterType, value ); + } + else if ( position != null ) { + return locateBinding( position, parameterType, value ); + } } throw getExceptionConverter().convert( @@ -718,11 +725,16 @@ protected

QueryParameterBinding

locateBinding(Parameter

parameter, Col if ( parameter instanceof QueryParameterImplementor

parameterImplementor ) { return locateBinding( parameterImplementor ); } - else if ( parameter.getName() != null ) { - return locateBinding( parameter.getName(), parameter.getParameterType(), values ); - } - else if ( parameter.getPosition() != null ) { - return locateBinding( parameter.getPosition(), parameter.getParameterType(), values ); + else { + final String name = parameter.getName(); + final Integer position = parameter.getPosition(); + final var parameterType = parameter.getParameterType(); + if ( name != null ) { + return locateBinding( name, parameterType, values ); + } + else if ( position != null ) { + return locateBinding( position, parameterType, values ); + } } throw getExceptionConverter().convert( @@ -732,7 +744,8 @@ else if ( parameter.getPosition() != null ) { protected

QueryParameterBinding

locateBinding(QueryParameterImplementor

parameter) { getCheckOpen(); - return getQueryParameterBindings().getBinding( getQueryParameter( parameter ) ); + return getQueryParameterBindings() + .getBinding( getQueryParameter( parameter ) ); } protected

QueryParameterBinding

locateBinding(String name, Class

javaType, P value) { From 9b43725dacbe63508a354a1d513cb8089aa70b1f Mon Sep 17 00:00:00 2001 From: Gavin King Date: Mon, 1 Dec 2025 10:19:46 +0100 Subject: [PATCH 5/5] allow sensible conversions on query parameter arguments as defined by what our JavaTypes already know how to convert --- .../query/internal/QueryArguments.java | 90 ++++++++ .../internal/QueryParameterBindingImpl.java | 20 +- .../spi/AbstractCommonQueryContract.java | 86 +------- .../spi/QueryParameterBindingValidator.java | 208 +++++------------- .../type/descriptor/java/BooleanJavaType.java | 32 ++- .../query/QueryParametersValidationTest.java | 2 +- .../java/BooleanJavaTypeDescriptorTest.java | 3 +- .../hql/QueryParametersValidationTest.java | 13 ++ 8 files changed, 211 insertions(+), 243 deletions(-) create mode 100644 hibernate-core/src/main/java/org/hibernate/query/internal/QueryArguments.java diff --git a/hibernate-core/src/main/java/org/hibernate/query/internal/QueryArguments.java b/hibernate-core/src/main/java/org/hibernate/query/internal/QueryArguments.java new file mode 100644 index 000000000000..bdf001c496bc --- /dev/null +++ b/hibernate-core/src/main/java/org/hibernate/query/internal/QueryArguments.java @@ -0,0 +1,90 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * Copyright Red Hat Inc. and Hibernate Authors + */ +package org.hibernate.query.internal; + +import jakarta.persistence.metamodel.Type; +import org.hibernate.HibernateException; +import org.hibernate.type.BindingContext; +import org.hibernate.type.descriptor.WrapperOptions; + +import java.util.Collection; + +/** + * @since 7.3 + * + * @author Gavin King + */ +public class QueryArguments { + + public static boolean isInstance( + Type parameterType, Object value, + BindingContext bindingContext, WrapperOptions options) { + if ( value == null ) { + return true; + } + final var sqmExpressible = bindingContext.resolveExpressible( parameterType ); + assert sqmExpressible != null; + final var javaType = sqmExpressible.getExpressibleJavaType(); + if ( !javaType.isInstance( value ) ) { + try { + // if this succeeds, we are good + javaType.wrap( value, options ); + } + catch (HibernateException | UnsupportedOperationException e) { + return false; + } + } + return true; + } + + public static boolean areInstances( + Type parameterType, Collection values, + BindingContext bindingContext, WrapperOptions options) { + if ( values.isEmpty() ) { + return true; + } + final var sqmExpressible = bindingContext.resolveExpressible( parameterType ); + assert sqmExpressible != null; + final var javaType = sqmExpressible.getExpressibleJavaType(); + for ( Object value : values ) { + if ( !javaType.isInstance( value ) ) { + try { + // if this succeeds, we are good + javaType.wrap( value, options ); + } + catch (HibernateException | UnsupportedOperationException e) { + return false; + } + } + } + return true; + } + + public static boolean areInstances( + Type parameterType, Object[] values, + BindingContext bindingContext, WrapperOptions options) { + if ( values.length == 0 ) { + return true; + } + if ( parameterType.getJavaType().isAssignableFrom( values.getClass().getComponentType() ) ) { + return true; + } + final var sqmExpressible = bindingContext.resolveExpressible( parameterType ); + assert sqmExpressible != null; + final var javaType = sqmExpressible.getExpressibleJavaType(); + for ( Object value : values ) { + if ( !javaType.isInstance( value ) ) { + try { + // if this succeeds, we are good + javaType.wrap( value, options ); + } + catch (HibernateException | UnsupportedOperationException e) { + return false; + } + } + } + return true; + } +} diff --git a/hibernate-core/src/main/java/org/hibernate/query/internal/QueryParameterBindingImpl.java b/hibernate-core/src/main/java/org/hibernate/query/internal/QueryParameterBindingImpl.java index 1341b1fdd2ff..12db03c55795 100644 --- a/hibernate-core/src/main/java/org/hibernate/query/internal/QueryParameterBindingImpl.java +++ b/hibernate-core/src/main/java/org/hibernate/query/internal/QueryParameterBindingImpl.java @@ -16,13 +16,13 @@ import org.hibernate.query.QueryParameter; import org.hibernate.query.spi.QueryParameterBinding; import org.hibernate.query.spi.QueryParameterBindingTypeResolver; -import org.hibernate.query.spi.QueryParameterBindingValidator; import org.hibernate.query.sqm.NodeBuilder; import org.hibernate.type.descriptor.java.JavaType; import org.hibernate.type.spi.TypeConfiguration; import jakarta.persistence.TemporalType; +import static org.hibernate.query.spi.QueryParameterBindingValidator.validate; import static org.hibernate.type.descriptor.java.JavaTypeHelper.isTemporal; import static org.hibernate.type.internal.BindingTypeHelper.resolveTemporalPrecision; @@ -116,7 +116,7 @@ public T getBindValue() { public void setBindValue(T value, boolean resolveJdbcTypeIfNecessary) { if ( !handleAsMultiValue( value, null ) ) { final Object coerced = coerceIfNotJpa( value ); - validate( coerced ); + validate( getBindType(), coerced, sessionFactory ); if ( value == null ) { // needed when setting a null value to the parameter of a native SQL query @@ -174,7 +174,7 @@ public void setBindValue(T value, @Nullable BindableType clarifiedType) { } final Object coerced = coerce( value ); - validate( coerced, clarifiedType ); + validate( clarifiedType, coerced, sessionFactory ); bindValue( coerced ); } } @@ -187,7 +187,7 @@ public void setBindValue(T value, TemporalType temporalTypePrecision) { } final Object coerced = coerceIfNotJpa( value ); - validate( coerced, temporalTypePrecision ); + validate( getBindType(), coerced, sessionFactory ); bindValue( coerced ); setExplicitTemporalPrecision( temporalTypePrecision ); } @@ -284,18 +284,6 @@ else if ( type instanceof BasicValuedMapping basicValuedMapping ) { return false; } - private void validate(Object value) { - QueryParameterBindingValidator.INSTANCE.validate( getBindType(), value, getCriteriaBuilder() ); - } - - private void validate(Object value, BindableType clarifiedType) { - QueryParameterBindingValidator.INSTANCE.validate( clarifiedType, value, getCriteriaBuilder() ); - } - - private void validate(Object value, TemporalType clarifiedTemporalType) { - QueryParameterBindingValidator.INSTANCE.validate( getBindType(), value, clarifiedTemporalType, getCriteriaBuilder() ); - } - @Override public TypeConfiguration getTypeConfiguration() { return sessionFactory.getTypeConfiguration(); diff --git a/hibernate-core/src/main/java/org/hibernate/query/spi/AbstractCommonQueryContract.java b/hibernate-core/src/main/java/org/hibernate/query/spi/AbstractCommonQueryContract.java index 954d12fd244a..1899d1592328 100644 --- a/hibernate-core/src/main/java/org/hibernate/query/spi/AbstractCommonQueryContract.java +++ b/hibernate-core/src/main/java/org/hibernate/query/spi/AbstractCommonQueryContract.java @@ -89,6 +89,8 @@ import static org.hibernate.jpa.internal.util.ConfigurationHelper.getCacheMode; import static org.hibernate.jpa.internal.util.ConfigurationHelper.getInteger; import static org.hibernate.query.QueryLogging.QUERY_MESSAGE_LOGGER; +import static org.hibernate.query.internal.QueryArguments.areInstances; +import static org.hibernate.query.internal.QueryArguments.isInstance; /** * Base implementation of {@link CommonQueryContract}. @@ -755,7 +757,7 @@ protected

QueryParameterBinding

locateBinding(String name, Class

javaT if ( parameterType != null ) { final var parameterJavaType = parameterType.getJavaType(); if ( !parameterJavaType.isAssignableFrom( javaType ) - && !isInstance( parameterType, value ) ) { + && !isInstance( parameterType, value, getNodeBuilder(), getSession() ) ) { throw new QueryArgumentException( "Argument to parameter named '" + name + "' has an incompatible type", parameterJavaType, javaType, value ); @@ -773,7 +775,7 @@ protected

QueryParameterBinding

locateBinding(int position, Class

java if ( parameterType != null ) { final var parameterJavaType = parameterType.getJavaType(); if ( !parameterJavaType.isAssignableFrom( javaType ) - && !isInstance( parameterType, value ) ) { + && !isInstance( parameterType, value, getNodeBuilder(), getSession() ) ) { throw new QueryArgumentException( "Argument to parameter at position " + position + " has an incompatible type", parameterJavaType, javaType, value ); @@ -791,7 +793,7 @@ protected

QueryParameterBinding

locateBinding(String name, Class

javaT if ( parameterType != null ) { final var parameterJavaType = parameterType.getJavaType(); if ( !parameterJavaType.isAssignableFrom( javaType ) - && !areInstances( parameterType, values ) ) { + && !areInstances( parameterType, values, getNodeBuilder(), getSession() ) ) { throw new QueryArgumentException( "Argument to parameter named '" + name + "' has an incompatible type", parameterJavaType, javaType, values ); @@ -809,7 +811,7 @@ protected

QueryParameterBinding

locateBinding(int position, Class

java if ( parameterType != null ) { final var parameterJavaType = parameterType.getJavaType(); if ( !parameterJavaType.isAssignableFrom( javaType ) - && !areInstances( parameterType, values ) ) { + && !areInstances( parameterType, values, getNodeBuilder(), getSession() ) ) { throw new QueryArgumentException( "Argument to parameter at position " + position + " has an incompatible type", parameterJavaType, javaType, values ); @@ -886,8 +888,8 @@ private

void setParameterValue(Object value, QueryParameterBinding

bindin private

boolean isInstanceOrAreInstances( Object value, QueryParameterBinding

binding, BindableType parameterType) { return binding.isMultiValued() && value instanceof Collection values - ? areInstances( parameterType, values ) - : isInstance( parameterType, value ); + ? areInstances( parameterType, values, getNodeBuilder(), getSession() ) + : isInstance( parameterType, value, getNodeBuilder(), getSession() ); } @Override @@ -916,8 +918,8 @@ private boolean multipleBinding(QueryParameter parameter, Object value){ final var hibernateType = parameter.getHibernateType(); return hibernateType == null || values.isEmpty() - || !isInstance( hibernateType, value ) - || isInstance( hibernateType, values.iterator().next() ); + || !isInstance( hibernateType, value, getNodeBuilder(), getSession() ) + || isInstance( hibernateType, values.iterator().next(), getNodeBuilder(), getSession() ); } else { return false; @@ -932,67 +934,6 @@ private void setTypedParameter(int position, TypedParameterValue typedVal setParameter( position, typedValue.value(), typedValue.type() ); } - private boolean isInstance(Type parameterType, Object value) { - if ( value == null ) { - return true; - } - final var sqmExpressible = getNodeBuilder().resolveExpressible( parameterType ); - assert sqmExpressible != null; - final var javaType = sqmExpressible.getExpressibleJavaType(); - if ( !javaType.isInstance( value ) ) { - try { - // if this succeeds, we are good - javaType.wrap( value, session ); - } - catch ( HibernateException|UnsupportedOperationException e) { - return false; - } - } - return true; - } - - private boolean areInstances(Type parameterType, Collection values) { - if ( values.isEmpty() ) { - return true; - } - final var sqmExpressible = getNodeBuilder().resolveExpressible( parameterType ); - assert sqmExpressible != null; - final var javaType = sqmExpressible.getExpressibleJavaType(); - for ( Object value : values ) { - if ( !javaType.isInstance( value ) ) { - try { - // if this succeeds, we are good - javaType.wrap( value, session ); - } - catch (HibernateException | UnsupportedOperationException e) { - return false; - } - } - } - return true; - } - - private boolean areInstances(Type parameterType, Object[] values) { - if ( values.length == 0 ) { - return true; - } - final var sqmExpressible = getNodeBuilder().resolveExpressible( parameterType ); - assert sqmExpressible != null; - final var javaType = sqmExpressible.getExpressibleJavaType(); - for ( Object value : values ) { - if ( !javaType.isInstance( value ) ) { - try { - // if this succeeds, we are good - javaType.wrap( value, session ); - } - catch (HibernateException | UnsupportedOperationException e) { - return false; - } - } - } - return true; - } - private NodeBuilder getNodeBuilder() { return getSessionFactory().getQueryEngine().getCriteriaBuilder(); } @@ -1196,7 +1137,7 @@ public CommonQueryContract setParameterList(String name, Object[] values) { private

void setParameterValues(Object[] values, QueryParameterBinding

binding) { final var parameterType = binding.getBindType(); if ( parameterType != null - && !areInstances( values, parameterType ) ) { + && !areInstances( parameterType, values, getNodeBuilder(), getSession() ) ) { throw new QueryArgumentException( "Argument to query parameter has an incompatible type", parameterType.getJavaType(), values.getClass().getComponentType(), values ); } @@ -1205,11 +1146,6 @@ private

void setParameterValues(Object[] values, QueryParameterBinding

bi binding.setBindValues( List.of( castArray ) ); } - private

boolean areInstances(Object[] values, BindableType parameterType) { - return parameterType.getJavaType().isAssignableFrom( values.getClass().getComponentType() ) - || areInstances( parameterType, values ); - } - @Override public

CommonQueryContract setParameterList(String name, P[] values, Class

javaType) { final var javaDescriptor = getJavaType( javaType ); diff --git a/hibernate-core/src/main/java/org/hibernate/query/spi/QueryParameterBindingValidator.java b/hibernate-core/src/main/java/org/hibernate/query/spi/QueryParameterBindingValidator.java index ee4b10cf97bd..27f1a34dc113 100644 --- a/hibernate-core/src/main/java/org/hibernate/query/spi/QueryParameterBindingValidator.java +++ b/hibernate-core/src/main/java/org/hibernate/query/spi/QueryParameterBindingValidator.java @@ -4,174 +4,86 @@ */ package org.hibernate.query.spi; -import java.util.Calendar; import java.util.Collection; -import java.util.Date; +import org.hibernate.Internal; +import org.hibernate.engine.spi.SessionFactoryImplementor; import org.hibernate.query.QueryArgumentException; import org.hibernate.type.BindableType; -import org.hibernate.type.BindingContext; -import org.hibernate.type.descriptor.java.JavaType; -import jakarta.persistence.TemporalType; +import static org.hibernate.query.internal.QueryArguments.areInstances; +import static org.hibernate.query.internal.QueryArguments.isInstance; /** * @author Andrea Boriero */ +@Internal public class QueryParameterBindingValidator { - public static final QueryParameterBindingValidator INSTANCE = new QueryParameterBindingValidator(); - private QueryParameterBindingValidator() { } - public void validate(BindableType paramType, Object bind, BindingContext bindingContext) { - validate( paramType, bind, null, bindingContext ); - } - - public void validate( - BindableType paramType, - Object bind, - TemporalType temporalPrecision, - BindingContext bindingContext) { - if ( bind == null || paramType == null ) { - // nothing we can check - return; - } - - final var sqmExpressible = bindingContext.resolveExpressible( paramType ); - final var parameterJavaType = - paramType.getJavaType() != null - ? paramType.getJavaType() - : sqmExpressible.getJavaType(); - - if ( parameterJavaType != null ) { - if ( bind instanceof Collection collection - && !Collection.class.isAssignableFrom( parameterJavaType ) ) { - // we have a collection passed in where we are expecting a non-collection. - // NOTE: this can happen in Hibernate's notion of "parameter list" binding - // NOTE2: the case of a collection value and an expected collection (if that can even happen) - // will fall through to the main check. - validateCollectionValuedParameterBinding( - parameterJavaType, - collection, - temporalPrecision - ); - } - else if ( bind.getClass().isArray() ) { - validateArrayValuedParameterBinding( - parameterJavaType, - bind, - temporalPrecision - ); - } - else if ( !isValidBindValue( - sqmExpressible.getExpressibleJavaType(), - parameterJavaType, - bind, - temporalPrecision - ) ) { - throw new QueryArgumentException( "Argument did not match parameter type", - parameterJavaType, bind ); + public static void validate(BindableType parameterType, Object argument, SessionFactoryImplementor factory) { + if ( argument != null && parameterType != null ) { + final var parameterJavaType = getParameterJavaType( parameterType, factory ); + if ( parameterJavaType != null ) { + if ( argument instanceof Collection collection + && !Collection.class.isAssignableFrom( parameterJavaType ) ) { + // we have a collection passed in where we are expecting a non-collection. + // NOTE: this can happen in Hibernate's notion of "parameter list" binding + // NOTE2: the case of a collection value and an expected collection (if that can even happen) + // will fall through to the main check. + validateCollectionValuedParameterBinding( parameterType, parameterJavaType, collection, factory ); + } + else if ( argument.getClass().isArray() ) { + validateArrayValuedParameterBinding( parameterJavaType, argument ); + } + else { + validateSingleValuedParameterBinding( parameterType, parameterJavaType, argument, factory ); + } } + // else nothing we can check } - // else nothing we can check } - private String extractName(TemporalType temporalType) { - return temporalType == null ? "n/a" : temporalType.name(); - } - - private void validateCollectionValuedParameterBinding( - Class parameterType, - Collection value, - TemporalType temporalType) { - // validate the elements... - for ( Object element : value ) { - if ( !isValidBindValue( parameterType, element, temporalType ) ) { - throw new QueryArgumentException( "Argument element did not match parameter type", - parameterType, element ); - } + private static Class getParameterJavaType(BindableType parameterType, SessionFactoryImplementor factory) { + final var javaType = parameterType.getJavaType(); + if ( javaType != null ) { + return javaType; + } + else { + return factory.getQueryEngine().getCriteriaBuilder() + .resolveExpressible( parameterType ) + .getJavaType(); } } - private static boolean isValidBindValue( - JavaType expectedJavaType, - Class expectedType, + private static void validateSingleValuedParameterBinding( + BindableType parameterType, Class parameterJavaType, Object value, - TemporalType temporalType) { - if ( value == null ) { - return true; + SessionFactoryImplementor factory) { + if ( !isInstance( parameterType, value, + factory.getQueryEngine().getCriteriaBuilder(), + factory.getWrapperOptions() ) ) { + throw new QueryArgumentException( "Argument did not match parameter type", + parameterJavaType, value ); } - else if ( expectedJavaType.isInstance( value ) ) { - return true; - } - else if ( temporalType != null ) { - final boolean parameterDeclarationIsTemporal = Date.class.isAssignableFrom( expectedType ) - || Calendar.class.isAssignableFrom( expectedType ); - final boolean bindIsTemporal = value instanceof Date - || value instanceof Calendar; - - return parameterDeclarationIsTemporal && bindIsTemporal; - } - - return false; } - private static boolean isValidBindValue( - Class expectedType, - Object value, - TemporalType temporalType) { - if ( expectedType.isPrimitive() ) { - if ( expectedType == boolean.class ) { - return value instanceof Boolean; - } - else if ( expectedType == char.class ) { - return value instanceof Character; - } - else if ( expectedType == byte.class ) { - return value instanceof Byte; - } - else if ( expectedType == short.class ) { - return value instanceof Short; - } - else if ( expectedType == int.class ) { - return value instanceof Integer; - } - else if ( expectedType == long.class ) { - return value instanceof Long; - } - else if ( expectedType == float.class ) { - return value instanceof Float; - } - else if ( expectedType == double.class ) { - return value instanceof Double; - } - return false; - } - else if ( value == null) { - return true; - } - else if ( expectedType.isInstance( value ) ) { - return true; - } - else if ( temporalType != null ) { - final boolean parameterDeclarationIsTemporal = - Date.class.isAssignableFrom( expectedType ) - || Calendar.class.isAssignableFrom( expectedType ); - final boolean bindIsTemporal = - value instanceof Date - || value instanceof Calendar; - return parameterDeclarationIsTemporal && bindIsTemporal; - } + private static void validateCollectionValuedParameterBinding( + BindableType parameterType, Class parameterJavaType, + Collection values, + SessionFactoryImplementor factory) { + if ( !areInstances( parameterType, values, + factory.getQueryEngine().getCriteriaBuilder(), + factory.getWrapperOptions() ) ) { + throw new QueryArgumentException( "Collection-valued argument did not match parameter type", + parameterJavaType, values ); - return false; + } } - private void validateArrayValuedParameterBinding( - Class parameterType, - Object value, - TemporalType temporalType) { + private static void validateArrayValuedParameterBinding(Class parameterType, Object value) { if ( !parameterType.isArray() ) { throw new QueryArgumentException( "Unexpected array-valued parameter binding", parameterType, value ); @@ -180,21 +92,23 @@ private void validateArrayValuedParameterBinding( final var componentType = value.getClass().getComponentType(); final var parameterComponentType = parameterType.getComponentType(); if ( componentType.isPrimitive() ) { - // we have a primitive array. we validate that the actual array has the component type (type of elements) - // we expect based on the component type of the parameter specification + // We have a primitive array. + // We validate that the actual array has the component type (type of elements) + // that we expect based on the component type of the parameter specification. if ( !parameterComponentType.isAssignableFrom( componentType ) ) { throw new QueryArgumentException( "Primitive array-valued argument type did not match parameter type", parameterType, value ); } } else { - // we have an object array. Here we loop over the array and physically check each element against - // the type we expect based on the component type of the parameter specification + // We have an object array. + // Here we loop over the array and physically check each element against the + // type we expect based on the component type of the parameter specification. final Object[] array = (Object[]) value; for ( Object element : array ) { - if ( !isValidBindValue( parameterComponentType, element, temporalType ) ) { - throw new QueryArgumentException( "Array-valued argument type did not match parameter type", - parameterType, array ); + if ( element != null && !parameterComponentType.isInstance( element ) ) { + throw new QueryArgumentException( "Array element did not match parameter element type", + parameterComponentType, element ); } } } diff --git a/hibernate-core/src/main/java/org/hibernate/type/descriptor/java/BooleanJavaType.java b/hibernate-core/src/main/java/org/hibernate/type/descriptor/java/BooleanJavaType.java index 49466879b8a3..090b15ecd57d 100644 --- a/hibernate-core/src/main/java/org/hibernate/type/descriptor/java/BooleanJavaType.java +++ b/hibernate-core/src/main/java/org/hibernate/type/descriptor/java/BooleanJavaType.java @@ -28,6 +28,7 @@ public class BooleanJavaType extends AbstractClassJavaType implements private final char characterValueFalse; private final char characterValueTrueLC; + private final char characterValueFalseLC; private final String stringValueTrue; private final String stringValueFalse; @@ -42,6 +43,7 @@ public BooleanJavaType(char characterValueTrue, char characterValueFalse) { this.characterValueFalse = toUpperCase( characterValueFalse ); characterValueTrueLC = Character.toLowerCase( characterValueTrue ); + characterValueFalseLC = Character.toLowerCase( characterValueFalse ); stringValueTrue = String.valueOf( characterValueTrue ); stringValueFalse = String.valueOf( characterValueFalse ); @@ -118,16 +120,35 @@ public Boolean wrap(X value, WrapperOptions options) { return number.intValue() != 0; } if (value instanceof Character character) { - return isTrue( character ); + if ( isTrue( character ) ) { + return true; + } + if ( isFalse( character ) ) { + return false; + } + throw new IllegalArgumentException( "Cannot convert Character value '" + character + "' to Boolean" ); } if (value instanceof String string) { - return isTrue( string ); + if ( isTrue( string ) ) { + return true; + } + if ( isFalse( string ) ) { + return false; + } + throw new IllegalArgumentException( "Cannot convert value '" + string + "' to Boolean" ); } throw unknownWrap( value.getClass() ); } private boolean isTrue(String strValue) { - return strValue != null && !strValue.isEmpty() && isTrue( strValue.charAt(0) ); + return strValue != null + && !strValue.isEmpty() + && isTrue( strValue.charAt(0) ); + } + + private boolean isFalse(String strValue) { + return strValue != null + && ( strValue.isEmpty() || isFalse( strValue.charAt(0) ) ); } private boolean isTrue(char charValue) { @@ -135,6 +156,11 @@ private boolean isTrue(char charValue) { || charValue == characterValueTrueLC; } + private boolean isFalse(char charValue) { + return charValue == characterValueFalse + || charValue == characterValueFalseLC; + } + public int toInt(Boolean value) { return value ? 1 : 0; } diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/jpa/query/QueryParametersValidationTest.java b/hibernate-core/src/test/java/org/hibernate/orm/test/jpa/query/QueryParametersValidationTest.java index 80932e0088fd..fb82b2599cf8 100644 --- a/hibernate-core/src/test/java/org/hibernate/orm/test/jpa/query/QueryParametersValidationTest.java +++ b/hibernate-core/src/test/java/org/hibernate/orm/test/jpa/query/QueryParametersValidationTest.java @@ -43,7 +43,7 @@ public void setParameterWithWrongTypeShouldThrowIllegalArgumentException(EntityM scope.inEntityManager( entityManager -> { Assertions.assertThrows( IllegalArgumentException.class, - () -> entityManager.createQuery( "select e from TestEntity e where e.id = :id" ).setParameter( "id", 1 ) + () -> entityManager.createQuery( "select e from TestEntity e where e.id = :id" ).setParameter( "id", "X" ) ); } ); } diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/mapping/type/java/BooleanJavaTypeDescriptorTest.java b/hibernate-core/src/test/java/org/hibernate/orm/test/mapping/type/java/BooleanJavaTypeDescriptorTest.java index 29ee5e6ad8c3..213e8cd965bf 100644 --- a/hibernate-core/src/test/java/org/hibernate/orm/test/mapping/type/java/BooleanJavaTypeDescriptorTest.java +++ b/hibernate-core/src/test/java/org/hibernate/orm/test/mapping/type/java/BooleanJavaTypeDescriptorTest.java @@ -7,6 +7,7 @@ import jakarta.persistence.AttributeConverter; import org.checkerframework.checker.nullness.qual.Nullable; import org.hibernate.dialect.Dialect; +import org.hibernate.testing.orm.junit.FailureExpected; import org.hibernate.testing.orm.junit.JiraKey; import org.hibernate.type.NumericBooleanConverter; import org.hibernate.type.TrueFalseConverter; @@ -92,7 +93,7 @@ public void testWrapShouldReturnFalseWhenFStringGiven() { assertFalse(result); } - @Test + @Test @FailureExpected // this was historically allowed public void testWrapShouldReturnFalseWhenRandomStringGiven() { // given // when diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/query/hql/QueryParametersValidationTest.java b/hibernate-core/src/test/java/org/hibernate/orm/test/query/hql/QueryParametersValidationTest.java index c85e0617afdf..2845927f0922 100644 --- a/hibernate-core/src/test/java/org/hibernate/orm/test/query/hql/QueryParametersValidationTest.java +++ b/hibernate-core/src/test/java/org/hibernate/orm/test/query/hql/QueryParametersValidationTest.java @@ -14,6 +14,8 @@ import org.hibernate.testing.orm.junit.SessionFactoryScope; import org.junit.jupiter.api.Test; +import static org.junit.jupiter.api.Assertions.assertThrows; + /** * @author Andrea Boriero */ @@ -40,6 +42,17 @@ public void testSetParameterWithArrayWithNullElement(SessionFactoryScope scope) ); } + @Test + public void testSetParameterWithArrayWithNullElementWrongType(SessionFactoryScope scope) { + // SimpleEntity#id is of type Integer + assertThrows( IllegalArgumentException.class, () -> + scope.inTransaction( (session) -> + session.createQuery( "from EntityWithBasicArray e where e.strings = :p" ) + .setParameter( "p", new Integer[]{null, 1} ) + ) + ); + } + @Entity(name = "EntityWithBasicArray") public static class EntityWithBasicArray { @Id