From 79c58bbcc873df7c9d3607f76f5788b6d119d99b Mon Sep 17 00:00:00 2001 From: Gavin King Date: Thu, 22 Jun 2023 20:41:35 +0200 Subject: [PATCH] HHH-16815 fix issues with query plan cache and ascending()/descending() --- .../org/hibernate/query/SelectionQuery.java | 24 +++++ .../query/sqm/internal/QuerySqmImpl.java | 15 ++- .../sqm/internal/SqmInterpretationsKey.java | 19 +++- .../sqm/internal/SqmSelectionQueryImpl.java | 13 ++- .../sqm/tree/select/SqmSelectStatement.java | 4 +- .../sqm/tree/select/SqmSortSpecification.java | 24 +++++ .../orm/test/query/order/OrderTest.java | 93 +++++++++++++++++++ 7 files changed, 178 insertions(+), 14 deletions(-) create mode 100644 hibernate-core/src/test/java/org/hibernate/orm/test/query/order/OrderTest.java diff --git a/hibernate-core/src/main/java/org/hibernate/query/SelectionQuery.java b/hibernate-core/src/main/java/org/hibernate/query/SelectionQuery.java index 6189677d45..8c425e46d9 100644 --- a/hibernate-core/src/main/java/org/hibernate/query/SelectionQuery.java +++ b/hibernate-core/src/main/java/org/hibernate/query/SelectionQuery.java @@ -471,12 +471,36 @@ public interface SelectionQuery extends CommonQueryContract { */ SelectionQuery setLockMode(String alias, LockMode lockMode); + /** + * If the result type of this query is an entity class, add an attribute + * of the entity to be used to order the query results in ascending order. + * + * @param attribute an attribute of the entity class returned by this query + * + * @since 6.3 + */ @Incubating SelectionQuery ascending(SingularAttribute attribute); + /** + * If the result type of this query is an entity class, add an attribute + * of the entity to be used to order the query results in descending order. + * + * @param attribute an attribute of the entity class returned by this query + * + * @since 6.3 + */ @Incubating SelectionQuery descending(SingularAttribute attribute); + /** + * Clear the ordering conditions for this query. + * + * @see #ascending(SingularAttribute) + * @see #descending(SingularAttribute) + * + * @since 6.3 + */ @Incubating SelectionQuery unordered(); diff --git a/hibernate-core/src/main/java/org/hibernate/query/sqm/internal/QuerySqmImpl.java b/hibernate-core/src/main/java/org/hibernate/query/sqm/internal/QuerySqmImpl.java index 64b0fae537..6ceb480d33 100644 --- a/hibernate-core/src/main/java/org/hibernate/query/sqm/internal/QuerySqmImpl.java +++ b/hibernate-core/src/main/java/org/hibernate/query/sqm/internal/QuerySqmImpl.java @@ -140,7 +140,7 @@ public class QuerySqmImpl private static final CoreMessageLogger LOG = CoreLogging.messageLogger( QuerySqmImpl.class ); private final String hql; - private final SqmStatement sqm; + private SqmStatement sqm; private final ParameterMetadataImplementor parameterMetadata; private final DomainParameterXref domainParameterXref; @@ -963,14 +963,15 @@ public class QuerySqmImpl @Override public SqmQueryImplementor addOrdering(SingularAttribute attribute, SortOrder order) { if ( sqm instanceof SqmSelectStatement ) { + sqm = sqm.copy( SqmCopyContext.simpleContext() ); + NodeBuilder nodeBuilder = sqm.nodeBuilder(); SqmSelectStatement select = (SqmSelectStatement) sqm; List orders = new ArrayList<>( select.getOrderList() ); select.getQuerySpec().getRoots().forEach( root -> { @SuppressWarnings("unchecked") SqmRoot singleRoot = (SqmRoot) root; SqmPath ref = singleRoot.get( attribute ); - NodeBuilder nodeBuilder = select.nodeBuilder(); - orders.add( order==SortOrder.ASCENDING ? nodeBuilder.asc(ref) : nodeBuilder.desc(ref) ); + orders.add( nodeBuilder.sort( ref, order) ); } ); select.orderBy( orders ); @@ -984,6 +985,7 @@ public class QuerySqmImpl @Override public SqmQueryImplementor unordered() { if ( sqm instanceof SqmSelectStatement ) { + sqm = sqm.copy( SqmCopyContext.simpleContext() ); SqmSelectStatement select = (SqmSelectStatement) sqm; select.getQueryPart().setOrderByClause( null ); @@ -991,6 +993,13 @@ public class QuerySqmImpl return this; } + @Override + public List getOrder() { + return sqm instanceof SqmSelectStatement + ? ((SqmSelectStatement) sqm).getOrderList() + : null; + } + // ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ // hints diff --git a/hibernate-core/src/main/java/org/hibernate/query/sqm/internal/SqmInterpretationsKey.java b/hibernate-core/src/main/java/org/hibernate/query/sqm/internal/SqmInterpretationsKey.java index 75da511444..989d77486a 100644 --- a/hibernate-core/src/main/java/org/hibernate/query/sqm/internal/SqmInterpretationsKey.java +++ b/hibernate-core/src/main/java/org/hibernate/query/sqm/internal/SqmInterpretationsKey.java @@ -8,8 +8,10 @@ package org.hibernate.query.sqm.internal; import java.util.Collection; import java.util.HashSet; +import java.util.List; import java.util.function.Supplier; +import jakarta.persistence.criteria.Order; import org.hibernate.LockOptions; import org.hibernate.engine.spi.LoadQueryInfluencers; import org.hibernate.query.ResultListTransformer; @@ -36,6 +38,7 @@ public class SqmInterpretationsKey implements QueryInterpretationCache.Key { public interface InterpretationsKeySource extends CacheabilityInfluencers { Class getResultType(); + List getOrder(); } public static SqmInterpretationsKey createInterpretationsKey(InterpretationsKeySource keySource) { @@ -45,6 +48,7 @@ public class SqmInterpretationsKey implements QueryInterpretationCache.Key { ? keySource.getSqmStatement() : keySource.getQueryString(), keySource.getResultType(), + keySource.getOrder(), keySource.getQueryOptions().getLockOptions(), keySource.getQueryOptions().getTupleTransformer(), keySource.getQueryOptions().getResultListTransformer(), @@ -86,6 +90,7 @@ public class SqmInterpretationsKey implements QueryInterpretationCache.Key { private final Object query; private final Class resultType; + private final List order; private final LockOptions lockOptions; private final TupleTransformer tupleTransformer; private final ResultListTransformer resultListTransformer; @@ -94,12 +99,14 @@ public class SqmInterpretationsKey implements QueryInterpretationCache.Key { private SqmInterpretationsKey( Object query, Class resultType, + List order, LockOptions lockOptions, TupleTransformer tupleTransformer, ResultListTransformer resultListTransformer, Collection enabledFetchProfiles) { this.query = query; this.resultType = resultType; + this.order = order; this.lockOptions = lockOptions; this.tupleTransformer = tupleTransformer; this.resultListTransformer = resultListTransformer; @@ -112,6 +119,7 @@ public class SqmInterpretationsKey implements QueryInterpretationCache.Key { query, resultType, // Since lock options are mutable, we need a copy for the cache key + order, lockOptions.makeCopy(), tupleTransformer, resultListTransformer, @@ -135,11 +143,12 @@ public class SqmInterpretationsKey implements QueryInterpretationCache.Key { final SqmInterpretationsKey that = (SqmInterpretationsKey) o; return query.equals( that.query ) - && areEqual( resultType, that.resultType ) - && areEqual( lockOptions, that.lockOptions ) - && areEqual( tupleTransformer, that.tupleTransformer ) - && areEqual( resultListTransformer, that.resultListTransformer ) - && areEqual( enabledFetchProfiles, that.enabledFetchProfiles ); + && areEqual( resultType, that.resultType ) + && areEqual( order, that.order ) + && areEqual( lockOptions, that.lockOptions ) + && areEqual( tupleTransformer, that.tupleTransformer ) + && areEqual( resultListTransformer, that.resultListTransformer ) + && areEqual( enabledFetchProfiles, that.enabledFetchProfiles ); } private boolean areEqual(T o1, T o2) { 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 6b2b9320f0..405051092c 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 @@ -88,7 +88,7 @@ import static org.hibernate.query.sqm.internal.SqmInterpretationsKey.createInter public class SqmSelectionQueryImpl extends AbstractSelectionQuery implements SqmSelectionQuery, InterpretationsKeySource { private final String hql; - private final SqmSelectStatement sqm; + private SqmSelectStatement sqm; private final ParameterMetadataImplementor parameterMetadata; private final DomainParameterXref domainParameterXref; @@ -568,13 +568,14 @@ public class SqmSelectionQueryImpl extends AbstractSelectionQuery } private void addOrdering(SingularAttribute attribute, SortOrder order) { + sqm = sqm.copy( SqmCopyContext.simpleContext() ); + NodeBuilder nodeBuilder = sqm.nodeBuilder(); List orders = new ArrayList<>( sqm.getOrderList() ); sqm.getQuerySpec().getRoots().forEach( root -> { @SuppressWarnings("unchecked") SqmRoot singleRoot = (SqmRoot) root; SqmPath ref = singleRoot.get( attribute ); - NodeBuilder nodeBuilder = sqm.nodeBuilder(); - orders.add( order==SortOrder.ASCENDING ? nodeBuilder.asc(ref) : nodeBuilder.desc(ref) ); + orders.add( nodeBuilder.sort( ref, order ) ); } ); sqm.orderBy( orders ); @@ -582,10 +583,16 @@ public class SqmSelectionQueryImpl extends AbstractSelectionQuery @Override public SqmSelectionQuery unordered() { + sqm = sqm.copy( SqmCopyContext.simpleContext() ); sqm.getQueryPart().setOrderByClause( null ); return this; } + @Override + public List getOrder() { + return sqm.getOrderList(); + } + // ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ // hints 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 780f5f43af..6bb7adb0c1 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 @@ -237,7 +237,6 @@ public class SqmSelectStatement extends AbstractSqmSelectQuery implements } @Override - @SuppressWarnings("unchecked") public Set> getParameters() { // At this level, the number of parameters may still be growing as // nodes are added to the Criteria - so we re-calculate this every @@ -245,8 +244,7 @@ public class SqmSelectStatement extends AbstractSqmSelectQuery implements // // for a "finalized" set of parameters, use `#resolveParameters` instead assert querySource == SqmQuerySource.CRITERIA; - final Set> sqmParameters = (Set>) (Set) getSqmParameters(); - return sqmParameters.stream() + return getSqmParameters().stream() .filter( parameterExpression -> !( parameterExpression instanceof ValueBindJpaCriteriaParameter ) ) .collect( Collectors.toSet() ); } diff --git a/hibernate-core/src/main/java/org/hibernate/query/sqm/tree/select/SqmSortSpecification.java b/hibernate-core/src/main/java/org/hibernate/query/sqm/tree/select/SqmSortSpecification.java index ebad20c17f..841d0d82f8 100644 --- a/hibernate-core/src/main/java/org/hibernate/query/sqm/tree/select/SqmSortSpecification.java +++ b/hibernate-core/src/main/java/org/hibernate/query/sqm/tree/select/SqmSortSpecification.java @@ -13,6 +13,8 @@ import org.hibernate.query.criteria.JpaOrder; import org.hibernate.query.sqm.tree.SqmCopyContext; import org.hibernate.query.sqm.tree.expression.SqmExpression; +import java.util.Objects; + /** * @author Steve Ebersole */ @@ -108,4 +110,26 @@ public class SqmSortSpecification implements JpaOrder { } } } + + @Override + public boolean equals(Object o) { + if ( this == o ) { + return true; + } + else if ( !(o instanceof SqmSortSpecification) ) { + return false; + } + else { + // used in SqmInterpretationsKey.equals() + SqmSortSpecification that = (SqmSortSpecification) o; + return Objects.equals( sortExpression, that.sortExpression ) + && sortOrder == that.sortOrder + && nullPrecedence == that.nullPrecedence; + } + } + + @Override + public int hashCode() { + return Objects.hash( sortExpression, sortOrder, nullPrecedence ); + } } diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/query/order/OrderTest.java b/hibernate-core/src/test/java/org/hibernate/orm/test/query/order/OrderTest.java new file mode 100644 index 0000000000..69e1e6522a --- /dev/null +++ b/hibernate-core/src/test/java/org/hibernate/orm/test/query/order/OrderTest.java @@ -0,0 +1,93 @@ +package org.hibernate.orm.test.query.order; + +import jakarta.persistence.Entity; +import jakarta.persistence.Id; +import jakarta.persistence.metamodel.SingularAttribute; +import org.hibernate.metamodel.model.domain.EntityDomainType; +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; + +import java.util.List; + +import static java.util.stream.Collectors.toList; +import static org.junit.jupiter.api.Assertions.assertEquals; + +@SessionFactory +@DomainModel(annotatedClasses = OrderTest.Book.class) +public class OrderTest { + + @Test void testAscendingDescending(SessionFactoryScope scope) { + scope.inTransaction( session -> { + session.persist(new Book("9781932394153", "Hibernate in Action")); + session.persist(new Book("9781617290459", "Java Persistence with Hibernate")); + }); + EntityDomainType bookType = scope.getSessionFactory().getJpaMetamodel().findEntityType(Book.class); + SingularAttribute title = bookType.findSingularAttribute("title"); + SingularAttribute isbn = bookType.findSingularAttribute("isbn"); + scope.inSession(session -> { + List titlesAsc = session.createSelectionQuery("from Book", Book.class) + .ascending(title) + .getResultList() + .stream().map(book -> book.title) + .collect(toList()); + assertEquals("Hibernate in Action", titlesAsc.get(0)); + assertEquals("Java Persistence with Hibernate", titlesAsc.get(1)); + + List titlesDesc = session.createSelectionQuery("from Book", Book.class) + .descending(title) + .getResultList() + .stream().map(book -> book.title) + .collect(toList()); + assertEquals("Hibernate in Action", titlesDesc.get(1)); + assertEquals("Java Persistence with Hibernate", titlesDesc.get(0)); + + List isbnAsc = session.createSelectionQuery("from Book", Book.class) + .ascending(isbn).descending(title) + .getResultList() + .stream().map(book -> book.title) + .collect(toList()); + assertEquals("Hibernate in Action", isbnAsc.get(1)); + assertEquals("Java Persistence with Hibernate", isbnAsc.get(0)); + + List isbnDesc = session.createSelectionQuery("from Book", Book.class) + .descending(isbn).descending(title) + .getResultList() + .stream().map(book -> book.title) + .collect(toList()); + assertEquals("Hibernate in Action", isbnDesc.get(0)); + assertEquals("Java Persistence with Hibernate", isbnDesc.get(1)); + + titlesAsc = session.createSelectionQuery("from Book order by isbn asc", Book.class) + .ascending(title) + .getResultList() + .stream().map(book -> book.title) + .collect(toList()); + assertEquals("Hibernate in Action", titlesAsc.get(1)); + assertEquals("Java Persistence with Hibernate", titlesAsc.get(0)); + titlesAsc = session.createSelectionQuery("from Book order by isbn asc", Book.class) + .unordered() + .ascending(title) + .getResultList() + .stream().map(book -> book.title) + .collect(toList()); + assertEquals("Hibernate in Action", titlesAsc.get(0)); + assertEquals("Java Persistence with Hibernate", titlesAsc.get(1)); + }); + } + + @Entity(name="Book") + static class Book { + @Id String isbn; + String title; + + Book(String isbn, String title) { + this.isbn = isbn; + this.title = title; + } + + Book() { + } + } +}