From 72f03d9d0fa25405bcadcbda4a9b714a99c1c63e Mon Sep 17 00:00:00 2001 From: Gavin Date: Sun, 28 May 2023 17:38:02 +0200 Subject: [PATCH] HHH-16710 implicit instantiation of record classes --- .../AbstractSharedSessionContract.java | 2 +- .../query/spi/AbstractSelectionQuery.java | 220 ++++++++++-------- .../internal/ConcreteSqmSelectQueryPlan.java | 52 ++--- .../sqm/internal/SqmSelectionQueryImpl.java | 37 +-- .../RowTransformerConstructorImpl.java | 58 +++++ .../internal/RowTransformerListImpl.java | 4 +- .../internal/RowTransformerMapImpl.java | 5 +- ...RowTransformerTupleTransformerAdapter.java | 5 - .../query/hql/ImplicitInstantiationTest.java | 28 +++ 9 files changed, 258 insertions(+), 153 deletions(-) create mode 100644 hibernate-core/src/main/java/org/hibernate/sql/results/internal/RowTransformerConstructorImpl.java diff --git a/hibernate-core/src/main/java/org/hibernate/internal/AbstractSharedSessionContract.java b/hibernate-core/src/main/java/org/hibernate/internal/AbstractSharedSessionContract.java index 9bdb1ae701..60299bceca 100644 --- a/hibernate-core/src/main/java/org/hibernate/internal/AbstractSharedSessionContract.java +++ b/hibernate-core/src/main/java/org/hibernate/internal/AbstractSharedSessionContract.java @@ -748,7 +748,7 @@ public abstract class AbstractSharedSessionContract implements SharedSessionCont throw new QueryTypeMismatchException( String.format( Locale.ROOT, - "Query result-type error - expecting `%s`, but found `%s`", + "Incorrect query result type: query produces '%s' but type '%s' was given", expectedResultType.getName(), resultType.getName() ) diff --git a/hibernate-core/src/main/java/org/hibernate/query/spi/AbstractSelectionQuery.java b/hibernate-core/src/main/java/org/hibernate/query/spi/AbstractSelectionQuery.java index 54ee284559..8dffca6637 100644 --- a/hibernate-core/src/main/java/org/hibernate/query/spi/AbstractSelectionQuery.java +++ b/hibernate-core/src/main/java/org/hibernate/query/spi/AbstractSelectionQuery.java @@ -66,6 +66,7 @@ import org.hibernate.query.sqm.tree.select.SqmQueryGroup; import org.hibernate.query.sqm.tree.select.SqmQueryPart; import org.hibernate.query.sqm.tree.select.SqmQuerySpec; import org.hibernate.query.sqm.tree.select.SqmSelectStatement; +import org.hibernate.query.sqm.tree.select.SqmSelectableNode; import org.hibernate.query.sqm.tree.select.SqmSelection; import org.hibernate.sql.exec.internal.CallbackImpl; import org.hibernate.sql.exec.spi.Callback; @@ -104,33 +105,36 @@ public abstract class AbstractSelectionQuery super( session ); } - public static boolean isTupleResultClass(Class resultType) { - return Tuple.class.isAssignableFrom( resultType ) - || Map.class.isAssignableFrom( resultType ); - } - protected TupleMetadata buildTupleMetadata(SqmStatement statement, Class resultType) { - if ( resultType == null ) { + if ( isInstantiableWithoutMetadata( resultType ) ) { + // no need to build metadata for instantiating tuples return null; } - else if ( isTupleResultClass( resultType ) ) { - final List> selections = - ( (SqmSelectStatement) statement ).getQueryPart() - .getFirstQuerySpec() - .getSelectClause() - .getSelections(); - if ( getQueryOptions().getTupleTransformer() == null ) { - return new TupleMetadata( buildTupleElementMap( selections ) ); - } - else { - throw new IllegalArgumentException( - "Illegal combination of Tuple resultType and (non-JpaTupleBuilder) TupleTransformer : " + - getQueryOptions().getTupleTransformer() - ); - } - } else { - return null; + final SqmSelectStatement select = (SqmSelectStatement) statement; + final List> selections = + select.getQueryPart().getFirstQuerySpec().getSelectClause() + .getSelections(); + if ( Tuple.class.equals( resultType ) || selections.size() > 1 ) { + return getTupleMetadata( selections ); + } + else { + // only one element in select list, + // we don't support instantiation + return null; + } + } + } + + private TupleMetadata getTupleMetadata(List> selections) { + if ( getQueryOptions().getTupleTransformer() == null ) { + return new TupleMetadata( buildTupleElementMap( selections ) ); + } + else { + throw new IllegalArgumentException( + "Illegal combination of Tuple resultType and (non-JpaTupleBuilder) TupleTransformer : " + + getQueryOptions().getTupleTransformer() + ); } } @@ -263,55 +267,52 @@ public abstract class AbstractSelectionQuery SqmQuerySpec querySpec, Class expectedResultClass, SessionFactoryImplementor sessionFactory) { - if ( expectedResultClass == null || expectedResultClass == Object.class ) { - // nothing to check - return; - } + if ( !isResultTypeAlwaysAllowed( expectedResultClass ) ) { + final List> selections = querySpec.getSelectClause().getSelections(); + if ( selections.size() == 1 ) { + // we have one item in the select list, + // the type has to match (no instantiation) + final SqmSelection sqmSelection = selections.get(0); - final List> selections = querySpec.getSelectClause().getSelections(); - - if ( expectedResultClass.isArray() ) { - // todo (6.0) : implement - } - else if ( Tuple.class.isAssignableFrom( expectedResultClass ) - || Map.class.isAssignableFrom( expectedResultClass ) - || List.class.isAssignableFrom( expectedResultClass ) ) { - // todo (6.0) : implement - } - else { - final boolean jpaQueryComplianceEnabled = sessionFactory.getSessionFactoryOptions() - .getJpaCompliance() - .isJpaQueryComplianceEnabled(); - if ( selections.size() != 1 ) { - final String errorMessage = "Query result-type error - multiple selections: use Tuple or array"; - - if ( jpaQueryComplianceEnabled ) { - throw new IllegalArgumentException( errorMessage ); + // special case for parameters in the select list + final SqmSelectableNode selection = sqmSelection.getSelectableNode(); + if ( selection instanceof SqmParameter ) { + final SqmParameter sqmParameter = (SqmParameter) selection; + final SqmExpressible nodeType = sqmParameter.getNodeType(); + // we may not yet know a selection type + if ( nodeType == null || nodeType.getExpressibleJavaType() == null ) { + // we can't verify the result type up front + return; + } } - else { - throw new QueryTypeMismatchException( errorMessage ); + + final boolean jpaQueryComplianceEnabled = + sessionFactory.getSessionFactoryOptions() + .getJpaCompliance() + .isJpaQueryComplianceEnabled(); + if ( !jpaQueryComplianceEnabled ) { + verifyResultType( expectedResultClass, sqmSelection.getNodeType() ); } } - - final SqmSelection sqmSelection = selections.get( 0 ); - - if ( sqmSelection.getSelectableNode() instanceof SqmParameter ) { - final SqmParameter sqmParameter = (SqmParameter) sqmSelection.getSelectableNode(); - - // we may not yet know a selection type - if ( sqmParameter.getNodeType() == null || sqmParameter.getNodeType().getExpressibleJavaType() == null ) { - // we can't verify the result type up front - return; - } - } - - if ( jpaQueryComplianceEnabled ) { - return; - } - verifyResultType( expectedResultClass, sqmSelection.getNodeType() ); + // else, let's assume we can instantiate it! } } + private static boolean isInstantiableWithoutMetadata(Class resultType) { + return resultType == null + || resultType.isArray() + || Object.class == resultType + || List.class == resultType; + } + + private static boolean isResultTypeAlwaysAllowed(Class expectedResultClass) { + return expectedResultClass == null + || expectedResultClass == Object.class + || expectedResultClass == List.class + || expectedResultClass == Tuple.class + || expectedResultClass.isArray(); + } + protected static void verifyResultType(Class resultClass, SqmExpressible sqmExpressible) { assert sqmExpressible != null; final JavaType expressibleJavaType = sqmExpressible.getExpressibleJavaType(); @@ -319,48 +320,65 @@ public abstract class AbstractSelectionQuery final Class javaTypeClass = expressibleJavaType.getJavaTypeClass(); if ( !resultClass.isAssignableFrom( javaTypeClass ) ) { if ( expressibleJavaType instanceof PrimitiveJavaType ) { - if ( ( (PrimitiveJavaType) expressibleJavaType ).getPrimitiveClass() == resultClass ) { - return; + if ( ( (PrimitiveJavaType) expressibleJavaType ).getPrimitiveClass() != resultClass ) { + throwQueryTypeMismatchException( resultClass, sqmExpressible ); } + } + else if ( isMatchingDateType( javaTypeClass, resultClass, sqmExpressible ) ) { + // special case, we are good + } + else { throwQueryTypeMismatchException( resultClass, sqmExpressible ); } - // Special case for date because we always report java.util.Date as expression type - // But the expected resultClass could be a subtype of that, so we need to check the JdbcType - if ( javaTypeClass == Date.class ) { - JdbcType jdbcType = null; - if ( sqmExpressible instanceof BasicDomainType ) { - jdbcType = ( (BasicDomainType) sqmExpressible).getJdbcType(); - } - else if ( sqmExpressible instanceof SqmPathSource ) { - final DomainType domainType = ( (SqmPathSource) sqmExpressible).getSqmPathType(); - if ( domainType instanceof BasicDomainType ) { - jdbcType = ( (BasicDomainType) domainType ).getJdbcType(); - } - } - if ( jdbcType != null ) { - switch ( jdbcType.getDefaultSqlTypeCode() ) { - case Types.DATE: - if ( resultClass.isAssignableFrom( java.sql.Date.class ) ) { - return; - } - break; - case Types.TIME: - if ( resultClass.isAssignableFrom( java.sql.Time.class ) ) { - return; - } - break; - case Types.TIMESTAMP: - if ( resultClass.isAssignableFrom( java.sql.Timestamp.class ) ) { - return; - } - break; - } - } - } - throwQueryTypeMismatchException( resultClass, sqmExpressible ); } } + // Special case for date because we always report java.util.Date as expression type + // But the expected resultClass could be a subtype of that, so we need to check the JdbcType + private static boolean isMatchingDateType( + Class javaTypeClass, + Class resultClass, + SqmExpressible sqmExpressible) { + return javaTypeClass == Date.class + && isMatchingDateJdbcType( resultClass, getJdbcType( sqmExpressible ) ); + } + + private static JdbcType getJdbcType(SqmExpressible sqmExpressible) { + if ( sqmExpressible instanceof BasicDomainType ) { + return ( (BasicDomainType) sqmExpressible).getJdbcType(); + } + else if ( sqmExpressible instanceof SqmPathSource ) { + final DomainType domainType = ( (SqmPathSource) sqmExpressible).getSqmPathType(); + if ( domainType instanceof BasicDomainType ) { + return ( (BasicDomainType) domainType ).getJdbcType(); + } + } + return null; + } + + private static boolean isMatchingDateJdbcType(Class resultClass, JdbcType jdbcType) { + if ( jdbcType != null ) { + switch ( jdbcType.getDefaultSqlTypeCode() ) { + case Types.DATE: + if ( resultClass.isAssignableFrom( java.sql.Date.class ) ) { + return true; + } + break; + case Types.TIME: + if ( resultClass.isAssignableFrom( java.sql.Time.class ) ) { + return true; + } + break; + case Types.TIMESTAMP: + if ( resultClass.isAssignableFrom( java.sql.Timestamp.class ) ) { + return true; + } + break; + } + } + return false; + } + private static void throwQueryTypeMismatchException(Class resultClass, SqmExpressible sqmExpressible) { final String errorMessage = String.format( "Specified result type [%s] did not match Query selection type [%s] - multiple selections: use Tuple or array", diff --git a/hibernate-core/src/main/java/org/hibernate/query/sqm/internal/ConcreteSqmSelectQueryPlan.java b/hibernate-core/src/main/java/org/hibernate/query/sqm/internal/ConcreteSqmSelectQueryPlan.java index e2cb86d779..b0bcf9ba2e 100644 --- a/hibernate-core/src/main/java/org/hibernate/query/sqm/internal/ConcreteSqmSelectQueryPlan.java +++ b/hibernate-core/src/main/java/org/hibernate/query/sqm/internal/ConcreteSqmSelectQueryPlan.java @@ -23,7 +23,6 @@ import org.hibernate.engine.spi.SubselectFetch; import org.hibernate.internal.EmptyScrollableResults; import org.hibernate.internal.util.collections.ArrayHelper; import org.hibernate.metamodel.mapping.MappingModelExpressible; -import org.hibernate.query.IllegalQueryOperationException; import org.hibernate.query.Query; import org.hibernate.query.TupleTransformer; import org.hibernate.query.spi.DomainQueryExecutionContext; @@ -50,6 +49,7 @@ import org.hibernate.sql.exec.spi.JdbcParametersList; import org.hibernate.sql.exec.spi.JdbcSelectExecutor; import org.hibernate.sql.results.graph.entity.LoadingEntityEntry; import org.hibernate.sql.results.internal.RowTransformerArrayImpl; +import org.hibernate.sql.results.internal.RowTransformerConstructorImpl; import org.hibernate.sql.results.internal.RowTransformerJpaTupleImpl; import org.hibernate.sql.results.internal.RowTransformerListImpl; import org.hibernate.sql.results.internal.RowTransformerMapImpl; @@ -178,44 +178,40 @@ public class ConcreteSqmSelectQueryPlan implements SelectQueryPlan { if ( queryOptions.getTupleTransformer() != null ) { return makeRowTransformerTupleTransformerAdapter( sqm, queryOptions ); } - - if ( resultType == null ) { + else if ( resultType == null ) { return RowTransformerStandardImpl.instance(); } - - if ( resultType == Object[].class ) { + else if ( resultType == Object[].class ) { return (RowTransformer) RowTransformerArrayImpl.instance(); } - else if ( List.class.equals( resultType ) ) { + else if ( resultType == List.class ) { return (RowTransformer) RowTransformerListImpl.instance(); } + else { + // NOTE : if we get here : + // 1) there is no TupleTransformer specified + // 2) an explicit result-type, other than an array, was specified - // NOTE : if we get here : - // 1) there is no TupleTransformer specified - // 2) an explicit result-type, other than an array, was specified - - final List> selections = - sqm.getQueryPart().getFirstQuerySpec().getSelectClause().getSelections(); - if ( tupleMetadata != null ) { - if ( Tuple.class.equals( resultType ) ) { - return (RowTransformer) new RowTransformerJpaTupleImpl( tupleMetadata ); - } - else if ( Map.class.equals( resultType ) ) { - return (RowTransformer) new RowTransformerMapImpl( tupleMetadata ); + if ( tupleMetadata == null ) { + if ( sqm.getQueryPart().getFirstQuerySpec().getSelectClause().getSelections().size() == 1 ) { + return RowTransformerSingularReturnImpl.instance(); + } + else { + throw new AssertionFailure( "Query defined multiple selections, should have had TupleMetadata" ); + } } else { - throw new AssertionFailure( "Wrong result type for tuple handling: " + resultType ); + if ( Tuple.class.equals( resultType ) ) { + return (RowTransformer) new RowTransformerJpaTupleImpl( tupleMetadata ); + } + else if ( Map.class.equals( resultType ) ) { + return (RowTransformer) new RowTransformerMapImpl( tupleMetadata ); + } + else { + return new RowTransformerConstructorImpl<>( resultType, tupleMetadata ); + } } } - - // NOTE : if we get here we have a resultType of some kind - - if ( selections.size() > 1 ) { - throw new IllegalQueryOperationException( "Query defined multiple selections, return cannot be typed (other that Object[] or Tuple)" ); - } - else { - return RowTransformerSingularReturnImpl.instance(); - } } private static RowTransformer makeRowTransformerTupleTransformerAdapter( diff --git a/hibernate-core/src/main/java/org/hibernate/query/sqm/internal/SqmSelectionQueryImpl.java b/hibernate-core/src/main/java/org/hibernate/query/sqm/internal/SqmSelectionQueryImpl.java index e5d9302582..9082106b3d 100644 --- a/hibernate-core/src/main/java/org/hibernate/query/sqm/internal/SqmSelectionQueryImpl.java +++ b/hibernate-core/src/main/java/org/hibernate/query/sqm/internal/SqmSelectionQueryImpl.java @@ -60,6 +60,7 @@ import org.hibernate.query.sqm.tree.expression.SqmParameter; import org.hibernate.query.sqm.tree.select.SqmSelectStatement; import org.hibernate.query.sqm.tree.select.SqmSelection; import org.hibernate.sql.results.internal.TupleMetadata; +import org.hibernate.type.descriptor.java.JavaType; import static org.hibernate.jpa.HibernateHints.HINT_CACHEABLE; import static org.hibernate.jpa.HibernateHints.HINT_CACHE_MODE; @@ -114,25 +115,35 @@ public class SqmSelectionQueryImpl extends AbstractSelectionQuery implemen } private Class determineResultType(SqmSelectStatement sqm) { - if ( expectedResultType != null ) { - if ( expectedResultType.isArray() ) { + final List> selections = sqm.getQuerySpec().getSelectClause().getSelections(); + if ( selections.size() == 1 ) { + if ( Object[].class.equals( expectedResultType ) ) { + // for JPA compatibility return Object[].class; } - else if ( List.class.isAssignableFrom( expectedResultType ) ) { - return expectedResultType; - } - else if ( isTupleResultClass( expectedResultType ) ) { - return expectedResultType; - } else { - return Object[].class; + final SqmSelection selection = selections.get(0); + if ( selection!=null ) { + JavaType javaType = selection.getNodeJavaType(); + if ( javaType != null) { + return javaType.getJavaTypeClass(); + } + } + // due to some error in the query, + // we don't have any information, + // so just let it through so the + // user sees the real error + return expectedResultType; } } + else if ( expectedResultType != null ) { + // assume we can repackage the tuple as + // the given type (worry about how later) + return expectedResultType; + } else { - final List> selections = sqm.getQuerySpec().getSelectClause().getSelections(); - return selections.size() == 1 - ? selections.get(0).getNodeJavaType().getJavaTypeClass() - : Object[].class; + // for JPA compatibility + return Object[].class; } } diff --git a/hibernate-core/src/main/java/org/hibernate/sql/results/internal/RowTransformerConstructorImpl.java b/hibernate-core/src/main/java/org/hibernate/sql/results/internal/RowTransformerConstructorImpl.java new file mode 100644 index 0000000000..00b8d30b42 --- /dev/null +++ b/hibernate-core/src/main/java/org/hibernate/sql/results/internal/RowTransformerConstructorImpl.java @@ -0,0 +1,58 @@ +/* + * 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.sql.results.internal; + +import jakarta.persistence.TupleElement; +import org.hibernate.InstantiationException; +import org.hibernate.sql.results.spi.RowTransformer; + +import java.lang.reflect.Constructor; +import java.util.List; + +/** + * {@link RowTransformer} instantiating an arbitrary class + * + * @author Gavin King + */ +public class RowTransformerConstructorImpl implements RowTransformer { + private final Class type; + private final TupleMetadata tupleMetadata; + private final Constructor constructor; + + public RowTransformerConstructorImpl(Class type, TupleMetadata tupleMetadata) { + this.type = type; + this.tupleMetadata = tupleMetadata; + final List> elements = tupleMetadata.getList(); + final Class[] sig = new Class[elements.size()]; + for (int i = 0; i < elements.size(); i++) { + sig[i] = elements.get(i).getJavaType(); + } + try { + constructor = type.getDeclaredConstructor( sig ); + constructor.setAccessible( true ); + } + catch (Exception e) { + throw new InstantiationException( "Cannot instantiate query result type ", type, e ); + } + } + + @Override + public T transformRow(Object[] row) { + try { + return constructor.newInstance( row ); + } + catch (Exception e) { + throw new InstantiationException( "Cannot instantiate query result type", type, e ); + } + } + + @Override + public int determineNumberOfResultElements(int rawElementCount) { + return 1; + } +} diff --git a/hibernate-core/src/main/java/org/hibernate/sql/results/internal/RowTransformerListImpl.java b/hibernate-core/src/main/java/org/hibernate/sql/results/internal/RowTransformerListImpl.java index 3ce50e007a..98b09664a8 100644 --- a/hibernate-core/src/main/java/org/hibernate/sql/results/internal/RowTransformerListImpl.java +++ b/hibernate-core/src/main/java/org/hibernate/sql/results/internal/RowTransformerListImpl.java @@ -11,9 +11,9 @@ import org.hibernate.sql.results.spi.RowTransformer; import java.util.List; /** - * RowTransformer used when an array is explicitly specified as the return type + * {@link RowTransformer} instantiating a {@link List} * - * @author Steve Ebersole + * @author Gavin King */ public class RowTransformerListImpl implements RowTransformer> { /** diff --git a/hibernate-core/src/main/java/org/hibernate/sql/results/internal/RowTransformerMapImpl.java b/hibernate-core/src/main/java/org/hibernate/sql/results/internal/RowTransformerMapImpl.java index aa481c6ab4..316b2050ae 100644 --- a/hibernate-core/src/main/java/org/hibernate/sql/results/internal/RowTransformerMapImpl.java +++ b/hibernate-core/src/main/java/org/hibernate/sql/results/internal/RowTransformerMapImpl.java @@ -7,7 +7,6 @@ package org.hibernate.sql.results.internal; -import jakarta.persistence.Tuple; import jakarta.persistence.TupleElement; import org.hibernate.sql.results.spi.RowTransformer; @@ -16,9 +15,9 @@ import java.util.List; import java.util.Map; /** - * RowTransformer generating a JPA {@link Tuple} + * {@link RowTransformer} instantiating a {@link Map} * - * @author Steve Ebersole + * @author Gavin King */ public class RowTransformerMapImpl implements RowTransformer> { private final TupleMetadata tupleMetadata; diff --git a/hibernate-core/src/main/java/org/hibernate/sql/results/internal/RowTransformerTupleTransformerAdapter.java b/hibernate-core/src/main/java/org/hibernate/sql/results/internal/RowTransformerTupleTransformerAdapter.java index 2f9f56cd37..a14b7fe8f9 100644 --- a/hibernate-core/src/main/java/org/hibernate/sql/results/internal/RowTransformerTupleTransformerAdapter.java +++ b/hibernate-core/src/main/java/org/hibernate/sql/results/internal/RowTransformerTupleTransformerAdapter.java @@ -29,9 +29,4 @@ public class RowTransformerTupleTransformerAdapter implements RowTransformer< assert aliases == null || row.length == aliases.length; return tupleTransformer.transformTuple( row, aliases ); } - - @Override - public int determineNumberOfResultElements(int rawElementCount) { - return rawElementCount; - } } diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/query/hql/ImplicitInstantiationTest.java b/hibernate-core/src/test/java/org/hibernate/orm/test/query/hql/ImplicitInstantiationTest.java index 6d4cfad420..8e09b9442e 100644 --- a/hibernate-core/src/test/java/org/hibernate/orm/test/query/hql/ImplicitInstantiationTest.java +++ b/hibernate-core/src/test/java/org/hibernate/orm/test/query/hql/ImplicitInstantiationTest.java @@ -22,6 +22,34 @@ import static org.junit.jupiter.api.Assertions.assertEquals; @SessionFactory public class ImplicitInstantiationTest { + static class Record { + Long id; + String name; + public Record(Long id, String name) { + this.id = id; + this.name = name; + } + Long id() { + return id; + } + String name() { + return name; + } + } + + @Test + public void testRecordInstantiationWithoutAlias(SessionFactoryScope scope) { + scope.inTransaction( + session -> { + session.persist(new Thing(1L, "thing")); + Record result = session.createSelectionQuery("select id, upper(name) from Thing", Record.class).getSingleResult(); + assertEquals( result.id(), 1L ); + assertEquals( result.name(), "THING" ); + session.getTransaction().setRollbackOnly(); + } + ); + } + @Test public void testTupleInstantiationWithAlias(SessionFactoryScope scope) { scope.inTransaction(