From 1dcd357ee379e68d4f2dd94780253d6603bcf9a1 Mon Sep 17 00:00:00 2001 From: Steve Ebersole Date: Fri, 30 Oct 2020 14:38:22 -0500 Subject: [PATCH] Various fixes from Search integration testing - support for "in( parameter list )" syntax from Criteria --- .../internal/QueryParameterBindingImpl.java | 38 +++++ .../sqm/internal/SqmCriteriaNodeBuilder.java | 64 ++++++++ .../sqm/sql/BaseSqmToSqlAstConverter.java | 137 ++++++++++++----- .../SqmJpaCriteriaParameterWrapper.java | 5 +- .../sqm/tree/select/SqmSelectStatement.java | 52 +++---- .../criteria/CriteriaParameterTests.java | 144 ++++++++++++++++++ 6 files changed, 377 insertions(+), 63 deletions(-) create mode 100644 hibernate-core/src/test/java/org/hibernate/orm/test/query/criteria/CriteriaParameterTests.java 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 7d761461ae..f223c3d2d0 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 @@ -6,6 +6,7 @@ */ package org.hibernate.query.internal; +import java.util.Arrays; import java.util.Collection; import javax.persistence.TemporalType; @@ -22,6 +23,7 @@ * @author Steve Ebersole */ public class QueryParameterBindingImpl implements QueryParameterBinding { + private final QueryParameter queryParameter; private final QueryParameterBindingTypeResolver typeResolver; private final boolean isBindingValidationRequired; @@ -40,6 +42,7 @@ public QueryParameterBindingImpl( QueryParameter queryParameter, QueryParameterBindingTypeResolver typeResolver, boolean isBindingValidationRequired) { + this.queryParameter = queryParameter; this.typeResolver = typeResolver; this.isBindingValidationRequired = isBindingValidationRequired; this.bindType = queryParameter.getHibernateType(); @@ -50,6 +53,7 @@ public QueryParameterBindingImpl( QueryParameterBindingTypeResolver typeResolver, AllowableParameterType bindType, boolean isBindingValidationRequired) { + this.queryParameter = queryParameter; this.typeResolver = typeResolver; this.isBindingValidationRequired = isBindingValidationRequired; this.bindType = bindType; @@ -90,6 +94,10 @@ public T getBindValue() { @Override public void setBindValue(T value) { + if ( handleAsMultiValue( value ) ) { + return; + } + if ( isBindingValidationRequired ) { validate( value ); } @@ -97,6 +105,28 @@ public void setBindValue(T value) { bindValue( value ); } + private boolean handleAsMultiValue(T value) { + if ( ! queryParameter.allowsMultiValuedBinding() ) { + return false; + } + + if ( value == null ) { + return false; + } + + if ( value instanceof Collection ) { + setBindValues( (Collection) value ); + return true; + } + + if ( value.getClass().isArray() ) { + setBindValues( (Collection) Arrays.asList( (Object[]) value ) ); + return true; + } + + return false; + } + private void bindValue(T value) { this.isBound = true; this.bindValue = value; @@ -109,6 +139,10 @@ private void bindValue(T value) { @Override public void setBindValue(T value, AllowableParameterType clarifiedType) { + if ( handleAsMultiValue( value ) ) { + return; + } + if ( isBindingValidationRequired ) { validate( value, clarifiedType ); } @@ -122,6 +156,10 @@ public void setBindValue(T value, AllowableParameterType clarifiedType) { @Override public void setBindValue(T value, TemporalType temporalTypePrecision) { + if ( handleAsMultiValue( value ) ) { + return; + } + if ( isBindingValidationRequired ) { validate( value, temporalTypePrecision ); } diff --git a/hibernate-core/src/main/java/org/hibernate/query/sqm/internal/SqmCriteriaNodeBuilder.java b/hibernate-core/src/main/java/org/hibernate/query/sqm/internal/SqmCriteriaNodeBuilder.java index 3be55ac0f0..6ca16a538c 100644 --- a/hibernate-core/src/main/java/org/hibernate/query/sqm/internal/SqmCriteriaNodeBuilder.java +++ b/hibernate-core/src/main/java/org/hibernate/query/sqm/internal/SqmCriteriaNodeBuilder.java @@ -39,6 +39,7 @@ import org.hibernate.engine.spi.SessionFactoryImplementor; import org.hibernate.internal.util.collections.ArrayHelper; import org.hibernate.metamodel.model.domain.AllowableFunctionReturnType; +import org.hibernate.metamodel.model.domain.AllowableParameterType; import org.hibernate.metamodel.model.domain.BasicDomainType; import org.hibernate.metamodel.model.domain.DomainType; import org.hibernate.metamodel.model.domain.JpaMetamodel; @@ -104,6 +105,7 @@ import org.hibernate.query.sqm.tree.update.SqmUpdateStatement; import org.hibernate.service.ServiceRegistry; import org.hibernate.type.StandardBasicTypes; +import org.hibernate.type.descriptor.java.JavaTypeDescriptor; import static java.util.Arrays.asList; import static org.hibernate.query.internal.QueryHelper.highestPrecedenceType; @@ -777,8 +779,52 @@ public SqmExpression nullLiteral(Class resultClass) { return new SqmLiteralNull( getTypeConfiguration().standardBasicTypeForJavaType( resultClass ), this ); } + class MultiValueParameterType implements AllowableParameterType { + private final JavaTypeDescriptor javaTypeDescriptor; + + public MultiValueParameterType(Class type) { + this.javaTypeDescriptor = domainModelAccess.get() + .getTypeConfiguration() + .getJavaTypeDescriptorRegistry() + .getDescriptor( type ); + } + + @Override + public JavaTypeDescriptor getExpressableJavaTypeDescriptor() { + return javaTypeDescriptor; + } + + @Override + public PersistenceType getPersistenceType() { + return PersistenceType.BASIC; + } + + @Override + public Class getJavaType() { + return javaTypeDescriptor.getJavaType(); + } + } + @Override public JpaCriteriaParameter parameter(Class paramClass) { + if ( Collection.class.isAssignableFrom( paramClass ) ) { + // a Collection-valued, multi-valued parameter + return new JpaCriteriaParameter( + new MultiValueParameterType( Collection.class ), + true, + this + ); + } + + if ( paramClass.isArray() ) { + // an array-valued, multi-valued parameter + return new JpaCriteriaParameter( + new MultiValueParameterType( Object[].class ), + true, + this + ); + } + //noinspection unchecked return new JpaCriteriaParameter<>( getTypeConfiguration().standardBasicTypeForJavaType( paramClass ), @@ -789,6 +835,24 @@ public JpaCriteriaParameter parameter(Class paramClass) { @Override public JpaCriteriaParameter parameter(Class paramClass, String name) { + if ( Collection.class.isAssignableFrom( paramClass ) ) { + // a multi-valued parameter + return new JpaCriteriaParameter( + new MultiValueParameterType<>( Collection.class ), + true, + this + ); + } + + if ( paramClass.isArray() ) { + // an array-valued, multi-valued parameter + return new JpaCriteriaParameter( + new MultiValueParameterType( Object[].class ), + true, + this + ); + } + //noinspection unchecked return new JpaCriteriaParameter<>( name, diff --git a/hibernate-core/src/main/java/org/hibernate/query/sqm/sql/BaseSqmToSqlAstConverter.java b/hibernate-core/src/main/java/org/hibernate/query/sqm/sql/BaseSqmToSqlAstConverter.java index ef100f7866..1ec65695a0 100644 --- a/hibernate-core/src/main/java/org/hibernate/query/sqm/sql/BaseSqmToSqlAstConverter.java +++ b/hibernate-core/src/main/java/org/hibernate/query/sqm/sql/BaseSqmToSqlAstConverter.java @@ -2447,52 +2447,24 @@ public NullnessPredicate visitIsNullPredicate(SqmNullnessPredicate predicate) { @Override public InListPredicate visitInListPredicate(SqmInListPredicate predicate) { - // special case: - // if there is just a single element and it is an SqmParameter - // and the corresponding QueryParameter binding is multi-valued... - // lets expand the SQL AST for each bind value + // special case: if there is just a single "value" element and it is a parameter + // and the binding for that parameter is multi-valued we need special + // handling for "expansion" if ( predicate.getListExpressions().size() == 1 ) { final SqmExpression sqmExpression = predicate.getListExpressions().get( 0 ); if ( sqmExpression instanceof SqmParameter ) { final SqmParameter sqmParameter = (SqmParameter) sqmExpression; - final QueryParameterImplementor domainParam = domainParameterXref.getQueryParameter( sqmParameter ); - final QueryParameterBinding domainParamBinding = domainParameterBindings.getBinding( domainParam ); - if ( domainParamBinding.isMultiValued() ) { - final InListPredicate inListPredicate = new InListPredicate( - (Expression) predicate.getTestExpression().accept( this ) - ); - - inferableTypeAccessStack.push( - () -> determineValueMapping( predicate.getTestExpression() ) ); - - try { - boolean first = true; - for ( Object bindValue : domainParamBinding.getBindValues() ) { - final SqmParameter sqmParamToConsume; - // for each bind value do the following: - // 1) create a pseudo-SqmParameter (though re-use the original for the first value) - if ( first ) { - sqmParamToConsume = sqmParameter; - first = false; - } - else { - sqmParamToConsume = sqmParameter.copy(); - domainParameterXref.addExpansion( domainParam, sqmParameter, sqmParamToConsume ); - } - - inListPredicate.addExpression( consumeSqmParameter( sqmParamToConsume ) ); - } + if ( sqmParameter.allowMultiValuedBinding() ) { + final InListPredicate specialCase = processInListWithSingleParameter( predicate, sqmParameter ); + if ( specialCase != null ) { + return specialCase; } - finally { - inferableTypeAccessStack.pop(); - } - - return inListPredicate; } } } + // otherwise - no special case... final InListPredicate inPredicate = new InListPredicate( (Expression) predicate.getTestExpression().accept( this ), @@ -2506,6 +2478,99 @@ public InListPredicate visitInListPredicate(SqmInListPredicate predicate) { return inPredicate; } + private InListPredicate processInListWithSingleParameter(SqmInListPredicate sqmPredicate, SqmParameter sqmParameter) { + assert sqmParameter.allowMultiValuedBinding(); + + if ( sqmParameter instanceof JpaCriteriaParameter ) { + return processInSingleCriteriaParameter( sqmPredicate, (JpaCriteriaParameter) sqmParameter ); + } + + return processInSingleHqlParameter( sqmPredicate, sqmParameter ); + } + + private InListPredicate processInSingleHqlParameter(SqmInListPredicate sqmPredicate, SqmParameter sqmParameter) { + final QueryParameterImplementor domainParam = domainParameterXref.getQueryParameter( sqmParameter ); + final QueryParameterBinding domainParamBinding = domainParameterBindings.getBinding( domainParam ); + + if ( ! domainParamBinding.isMultiValued() ) { + // triggers normal processing + return null; + } + + final InListPredicate inListPredicate = new InListPredicate( + (Expression) sqmPredicate.getTestExpression().accept( this ) + ); + + inferableTypeAccessStack.push( + () -> determineValueMapping( sqmPredicate.getTestExpression() ) + ); + + try { + boolean first = true; + for ( Object bindValue : domainParamBinding.getBindValues() ) { + final SqmParameter sqmParamToConsume; + // for each bind value create an "expansion" + if ( first ) { + sqmParamToConsume = sqmParameter; + first = false; + } + else { + sqmParamToConsume = sqmParameter.copy(); + domainParameterXref.addExpansion( domainParam, sqmParameter, sqmParamToConsume ); + } + + inListPredicate.addExpression( consumeSqmParameter( sqmParamToConsume ) ); + } + } + finally { + inferableTypeAccessStack.pop(); + } + + return inListPredicate; + } + + private InListPredicate processInSingleCriteriaParameter(SqmInListPredicate sqmPredicate, JpaCriteriaParameter jpaCriteriaParameter) { + assert jpaCriteriaParameter.allowsMultiValuedBinding(); + + final QueryParameterBinding domainParamBinding = domainParameterBindings.getBinding( jpaCriteriaParameter ); + if ( ! domainParamBinding.isMultiValued() ) { + return null; + } + + final InListPredicate inListPredicate = new InListPredicate( + (Expression) sqmPredicate.getTestExpression().accept( this ) + ); + + inferableTypeAccessStack.push( + () -> determineValueMapping( sqmPredicate.getTestExpression() ) + ); + + final SqmJpaCriteriaParameterWrapper sqmWrapper = jpaCriteriaParamResolutions.get( jpaCriteriaParameter ).get(); + + try { + boolean first = true; + for ( Object bindValue : domainParamBinding.getBindValues() ) { + final SqmParameter sqmParamToConsume; + // for each bind value create an "expansion" + if ( first ) { + sqmParamToConsume = sqmWrapper; + first = false; + } + else { + sqmParamToConsume = sqmWrapper.copy(); + domainParameterXref.addExpansion( jpaCriteriaParameter, sqmWrapper, sqmParamToConsume ); + } + + inListPredicate.addExpression( consumeSqmParameter( sqmParamToConsume ) ); + } + } + finally { + inferableTypeAccessStack.pop(); + } + + return inListPredicate; + } + @Override public InSubQueryPredicate visitInSubQueryPredicate(SqmInSubQueryPredicate predicate) { return new InSubQueryPredicate( diff --git a/hibernate-core/src/main/java/org/hibernate/query/sqm/tree/expression/SqmJpaCriteriaParameterWrapper.java b/hibernate-core/src/main/java/org/hibernate/query/sqm/tree/expression/SqmJpaCriteriaParameterWrapper.java index c9e4a54b95..62f48b5310 100644 --- a/hibernate-core/src/main/java/org/hibernate/query/sqm/tree/expression/SqmJpaCriteriaParameterWrapper.java +++ b/hibernate-core/src/main/java/org/hibernate/query/sqm/tree/expression/SqmJpaCriteriaParameterWrapper.java @@ -15,7 +15,10 @@ import org.hibernate.sql.ast.tree.expression.JdbcParameter; /** - * @author Steve Ebersole + * Acts as the per-use wrapper for a JpaCriteriaParameter ({@link javax.persistence.criteria.CriteriaBuilder#parameter}). + * + * JpaCriteriaParameter is the "domain query parameter" ({@link org.hibernate.query.QueryParameter} + * while SqmJpaCriteriaParameterWrapper is the {@link SqmParameter} */ public class SqmJpaCriteriaParameterWrapper extends AbstractSqmExpression diff --git a/hibernate-core/src/main/java/org/hibernate/query/sqm/tree/select/SqmSelectStatement.java b/hibernate-core/src/main/java/org/hibernate/query/sqm/tree/select/SqmSelectStatement.java index c3a9c90767..f4eca11801 100644 --- a/hibernate-core/src/main/java/org/hibernate/query/sqm/tree/select/SqmSelectStatement.java +++ b/hibernate-core/src/main/java/org/hibernate/query/sqm/tree/select/SqmSelectStatement.java @@ -81,6 +81,7 @@ public Set> getSqmParameters() { nodeBuilder().getServiceRegistry() ); } + return parameters == null ? Collections.emptySet() : Collections.unmodifiableSet( parameters ); } @@ -121,15 +122,13 @@ public void process(SqmParameter parameter) { sqmParameters = new HashSet<>(); } - sqmParameters.add( parameter ); - if ( parameter instanceof SqmJpaCriteriaParameterWrapper ) { if ( jpaCriteriaParamResolutions == null ) { jpaCriteriaParamResolutions = new IdentityHashMap<>(); } final SqmJpaCriteriaParameterWrapper wrapper = (SqmJpaCriteriaParameterWrapper) parameter; - final JpaCriteriaParameter criteriaParameter = wrapper.getJpaCriteriaParameter(); + final JpaCriteriaParameter criteriaParameter = wrapper.getJpaCriteriaParameter(); final List> sqmParametersForCriteriaParameter = jpaCriteriaParamResolutions.computeIfAbsent( criteriaParameter, @@ -139,29 +138,30 @@ public void process(SqmParameter parameter) { sqmParametersForCriteriaParameter.add( wrapper ); sqmParameters.add( wrapper ); } - - if ( parameter instanceof JpaCriteriaParameter ) { - final JpaCriteriaParameter criteriaParameter = (JpaCriteriaParameter) parameter; - - if ( jpaCriteriaParamResolutions == null ) { - jpaCriteriaParamResolutions = new IdentityHashMap<>(); - } - - final List> sqmParametersForCriteriaParameter = jpaCriteriaParamResolutions.computeIfAbsent( - criteriaParameter, - jcp -> new ArrayList<>() - ); - - - //noinspection unchecked - final SqmJpaCriteriaParameterWrapper wrapper = new SqmJpaCriteriaParameterWrapper<>( - criteriaParameter.getHibernateType(), - criteriaParameter, - criteriaParameter.nodeBuilder() - ); - - sqmParametersForCriteriaParameter.add( wrapper ); - sqmParameters.add( wrapper ); + else if ( parameter instanceof JpaCriteriaParameter ) { + throw new UnsupportedOperationException(); +// final JpaCriteriaParameter criteriaParameter = (JpaCriteriaParameter) parameter; +// +// if ( jpaCriteriaParamResolutions == null ) { +// jpaCriteriaParamResolutions = new IdentityHashMap<>(); +// } +// +// final List> sqmParametersForCriteriaParameter = jpaCriteriaParamResolutions.computeIfAbsent( +// criteriaParameter, +// jcp -> new ArrayList<>() +// ); +// +// final SqmJpaCriteriaParameterWrapper wrapper = new SqmJpaCriteriaParameterWrapper( +// criteriaParameter.getHibernateType(), +// criteriaParameter, +// criteriaParameter.nodeBuilder() +// ); +// +// sqmParametersForCriteriaParameter.add( wrapper ); +// sqmParameters.add( wrapper ); + } + else { + sqmParameters.add( parameter ); } } diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/query/criteria/CriteriaParameterTests.java b/hibernate-core/src/test/java/org/hibernate/orm/test/query/criteria/CriteriaParameterTests.java new file mode 100644 index 0000000000..8c4d64c3c7 --- /dev/null +++ b/hibernate-core/src/test/java/org/hibernate/orm/test/query/criteria/CriteriaParameterTests.java @@ -0,0 +1,144 @@ +/* + * Hibernate, Relational Persistence for Idiomatic Java + * + * License: GNU Lesser General Public License (LGPL), version 2.1 or later + * See the lgpl.txt file in the root directory or http://www.gnu.org/licenses/lgpl-2.1.html + */ +package org.hibernate.orm.test.query.criteria; + +import java.util.Arrays; +import java.util.Collection; +import javax.persistence.criteria.CriteriaBuilder; +import javax.persistence.criteria.CriteriaQuery; +import javax.persistence.criteria.ParameterExpression; +import javax.persistence.criteria.Root; + +import org.hibernate.query.criteria.HibernateCriteriaBuilder; +import org.hibernate.query.spi.QueryImplementor; + +import org.hibernate.testing.orm.domain.gambit.BasicEntity; +import org.hibernate.testing.orm.junit.DomainModel; +import org.hibernate.testing.orm.junit.SessionFactory; +import org.hibernate.testing.orm.junit.SessionFactoryScope; +import org.junit.jupiter.api.Test; + +@DomainModel( annotatedClasses = BasicEntity.class ) +@SessionFactory +public class CriteriaParameterTests { + @Test + public void testParameterBaseline(SessionFactoryScope scope) { + scope.inTransaction( + session -> { + final CriteriaBuilder criteriaBuilder = session.getCriteriaBuilder(); + final ParameterExpression parameter = criteriaBuilder.parameter( String.class ); + final CriteriaQuery criteria = criteriaBuilder.createQuery( BasicEntity.class ); + final Root root = criteria.from( BasicEntity.class ); + criteria.where( criteriaBuilder.equal( root.get( "data" ), parameter ) ); + + final QueryImplementor query = session.createQuery( criteria ); + query.setParameter( parameter, "fe" ); + query.list(); + } + ); + } + + @Test + public void testMultiValuedParameterBaseline(SessionFactoryScope scope) { + scope.inTransaction( + session -> { + final CriteriaBuilder criteriaBuilder = session.getCriteriaBuilder(); + final ParameterExpression parameter = criteriaBuilder.parameter( String.class ); + final CriteriaQuery criteria = criteriaBuilder.createQuery( BasicEntity.class ); + final Root root = criteria.from( BasicEntity.class ); + + final CriteriaBuilder.In inPredicate = criteriaBuilder.in( root.get( "data" ) ); + inPredicate.value( "fe" ); + inPredicate.value( "fi" ); + inPredicate.value( "fo" ); + inPredicate.value( "fum" ); + + criteria.where( inPredicate ); + + final QueryImplementor query = session.createQuery( criteria ); + + query.list(); + } + ); + } + + @Test + public void testMultiValuedParameterBaseline2(SessionFactoryScope scope) { + scope.inTransaction( + session -> { + final CriteriaBuilder criteriaBuilder = session.getCriteriaBuilder(); + final CriteriaQuery criteria = criteriaBuilder.createQuery( BasicEntity.class ); + final Root root = criteria.from( BasicEntity.class ); + + final CriteriaBuilder.In inPredicate = criteriaBuilder.in( root.get( "data" ) ); + final ParameterExpression p1 = criteriaBuilder.parameter( String.class ); + inPredicate.value( p1 ); + final ParameterExpression p2 = criteriaBuilder.parameter( String.class ); + inPredicate.value( p2 ); + + criteria.where( inPredicate ); + + final QueryImplementor query = session.createQuery( criteria ); + query.setParameter( p1, "fe" ); + query.setParameter( p2, "fi" ); + query.list(); + } + ); + } + + @Test + public void testMultiValuedParameterBaseline3(SessionFactoryScope scope) { + scope.inTransaction( + session -> { + final HibernateCriteriaBuilder criteriaBuilder = session.getCriteriaBuilder(); + final ParameterExpression parameter = criteriaBuilder.parameter( String.class ); + final CriteriaQuery criteria = criteriaBuilder.createQuery( BasicEntity.class ); + final Root root = criteria.from( BasicEntity.class ); + + final CriteriaBuilder.In inPredicate = criteriaBuilder.in( root.get( "data" ), parameter ); + + criteria.where( inPredicate ); + + final QueryImplementor query = session.createQuery( criteria ); + query.setParameter( parameter, "fe" ); + query.list(); + } + ); + } + + @Test + public void testMultiValuedParameterBaseline3HQL(SessionFactoryScope scope) { + scope.inTransaction( + session -> { + final QueryImplementor query = session.createQuery( "from BasicEntity where data in (:p)" ); + query.setParameter( "p", "fe" ); + query.list(); + } + ); + } + + @Test + public void testMultiValuedParameter(SessionFactoryScope scope) { + scope.inTransaction( + session -> { + final CriteriaBuilder criteriaBuilder = session.getCriteriaBuilder(); + final ParameterExpression parameter = criteriaBuilder.parameter( Collection.class ); + final CriteriaQuery criteria = criteriaBuilder.createQuery( BasicEntity.class ); + final Root root = criteria.from( BasicEntity.class ); + + final CriteriaBuilder.In inPredicate = criteriaBuilder.in( root.get( "data" ) ); + inPredicate.value( parameter ); + + criteria.where( inPredicate ); + + final QueryImplementor query = session.createQuery( criteria ); + query.setParameter( parameter, Arrays.asList( "fe", "fi", "fo", "fum" ) ); + query.list(); + } + ); + } +}