diff --git a/hibernate-core/src/main/java/org/hibernate/query/sqm/internal/SqmUtil.java b/hibernate-core/src/main/java/org/hibernate/query/sqm/internal/SqmUtil.java index 43d8444395c6..69922cc8889e 100644 --- a/hibernate-core/src/main/java/org/hibernate/query/sqm/internal/SqmUtil.java +++ b/hibernate-core/src/main/java/org/hibernate/query/sqm/internal/SqmUtil.java @@ -24,6 +24,7 @@ import org.hibernate.engine.spi.SessionFactoryImplementor; import org.hibernate.engine.spi.SharedSessionContractImplementor; import org.hibernate.internal.util.collections.CollectionHelper; +import org.hibernate.internal.util.collections.Stack; import org.hibernate.metamodel.MappingMetamodel; import org.hibernate.metamodel.mapping.BasicValuedMapping; import org.hibernate.metamodel.mapping.Bindable; @@ -174,38 +175,66 @@ public static ModelPartContainer getTargetMappingIfNeeded( SqmPath sqmPath, ModelPartContainer modelPartContainer, SqmToSqlAstConverter sqlAstCreationState) { - final SqmQueryPart queryPart = sqlAstCreationState.getCurrentSqmQueryPart(); - if ( queryPart != null ) { - // We only need to do this for queries - final Clause clause = sqlAstCreationState.getCurrentClauseStack().getCurrent(); - if ( clause != Clause.FROM && modelPartContainer.getPartMappingType() != modelPartContainer && sqmPath.getLhs() instanceof SqmFrom ) { - final ModelPart modelPart; - if ( modelPartContainer instanceof PluralAttributeMapping ) { - modelPart = getCollectionPart( - (PluralAttributeMapping) modelPartContainer, - castNonNull( sqmPath.getNavigablePath().getParent() ) - ); - } - else { - modelPart = modelPartContainer; - } - if ( modelPart instanceof EntityAssociationMapping ) { - final EntityAssociationMapping association = (EntityAssociationMapping) modelPart; - // If the path is one of the association's target key properties, - // we need to render the target side if in group/order by - if ( association.getTargetKeyPropertyNames().contains( sqmPath.getReferencedPathSource().getPathName() ) - && ( clause == Clause.GROUP || clause == Clause.ORDER - || !isFkOptimizationAllowed( sqmPath.getLhs(), association ) - || queryPart.getFirstQuerySpec().groupByClauseContains( sqmPath.getNavigablePath(), sqlAstCreationState ) - || queryPart.getFirstQuerySpec().orderByClauseContains( sqmPath.getNavigablePath(), sqlAstCreationState ) ) ) { - return association.getAssociatedEntityMappingType(); - } + // We only need to do this for queries + if ( sqlAstCreationState.getCurrentClauseStack().getCurrent() != Clause.FROM + && modelPartContainer.getPartMappingType() != modelPartContainer && sqmPath.getLhs() instanceof SqmFrom ) { + final ModelPart modelPart; + if ( modelPartContainer instanceof PluralAttributeMapping ) { + modelPart = getCollectionPart( + (PluralAttributeMapping) modelPartContainer, + castNonNull( sqmPath.getNavigablePath().getParent() ) + ); + } + else { + modelPart = modelPartContainer; + } + if ( modelPart instanceof EntityAssociationMapping ) { + final EntityAssociationMapping association = (EntityAssociationMapping) modelPart; + if ( shouldRenderTargetSide( sqmPath, association, sqlAstCreationState ) ) { + return association.getAssociatedEntityMappingType(); } } } return modelPartContainer; } + private static boolean shouldRenderTargetSide( + SqmPath sqmPath, + EntityAssociationMapping association, + SqmToSqlAstConverter sqlAstCreationState) { + if ( !association.getTargetKeyPropertyNames().contains( sqmPath.getReferencedPathSource().getPathName() ) ) { + return false; + } + // If the path is one of the association's target key properties, + // we need to render the target side if in group/order by + final Clause clause = sqlAstCreationState.getCurrentClauseStack().getCurrent(); + return clause == Clause.GROUP || clause == Clause.ORDER + || !isFkOptimizationAllowed( sqmPath.getLhs(), association ) + || clauseContainsPath( Clause.GROUP, sqmPath, sqlAstCreationState ) + || clauseContainsPath( Clause.ORDER, sqmPath, sqlAstCreationState ); + } + + private static boolean clauseContainsPath( + Clause clauseToCheck, + SqmPath sqmPath, + SqmToSqlAstConverter sqlAstCreationState) { + final Stack queryPartStack = sqlAstCreationState.getSqmQueryPartStack(); + final NavigablePath navigablePath = sqmPath.getNavigablePath(); + final Boolean found = queryPartStack.findCurrentFirst( queryPart -> { + final SqmQuerySpec spec = queryPart.getFirstQuerySpec(); + if ( clauseToCheck == Clause.GROUP && spec.groupByClauseContains( navigablePath, sqlAstCreationState ) ) { + return true; + } + else if ( clauseToCheck == Clause.ORDER && spec.orderByClauseContains( navigablePath, sqlAstCreationState ) ) { + return true; + } + else { + return null; + } + } ); + return Boolean.TRUE.equals( found ); + } + private static CollectionPart getCollectionPart(PluralAttributeMapping attribute, NavigablePath path) { final CollectionPart.Nature nature = CollectionPart.Nature.fromNameExact( path.getLocalName() ); if ( nature != null ) { @@ -214,6 +243,8 @@ private static CollectionPart getCollectionPart(PluralAttributeMapping attribute return attribute.getElementDescriptor(); case INDEX: return attribute.getIndexDescriptor(); + case ID: + return attribute.getIdentifierDescriptor(); } } return null; 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 15fe3b56cb79..44533cd18134 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 @@ -478,7 +478,7 @@ public abstract class BaseSqmToSqlAstConverter extends Base private boolean deduplicateSelectionItems; private ForeignKeyDescriptor.Nature currentlyResolvingForeignKeySide; private SqmStatement currentSqmStatement; - private SqmQueryPart currentSqmQueryPart; + private Stack sqmQueryPartStack = new StandardStack<>( SqmQueryPart.class ); private CteContainer cteContainer; /** * A map from {@link SqmCteTable#getCteName()} to the final SQL name. @@ -784,9 +784,10 @@ public Stack getCurrentClauseStack() { return currentClauseStack; } + @SuppressWarnings("rawtypes") @Override - public SqmQueryPart getCurrentSqmQueryPart() { - return currentSqmQueryPart; + public Stack getSqmQueryPartStack() { + return sqmQueryPartStack; } // ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ @@ -1735,8 +1736,7 @@ public CteStatement visitCteStatement(SqmCteStatement sqmCteStatement) { ); final DelegatingSqmAliasedNodeCollector collector = (DelegatingSqmAliasedNodeCollector) processingState .getSqlExpressionResolver(); - final SqmQueryPart oldSqmQueryPart = currentSqmQueryPart; - currentSqmQueryPart = queryGroup; + sqmQueryPartStack.push( queryGroup ); pushProcessingState( processingState ); try { @@ -1780,7 +1780,7 @@ public CteStatement visitCteStatement(SqmCteStatement sqmCteStatement) { } finally { popProcessingStateStack(); - currentSqmQueryPart = oldSqmQueryPart; + sqmQueryPartStack.pop(); } } finally { @@ -1993,8 +1993,7 @@ public QueryGroup visitQueryGroup(SqmQueryGroup queryGroup) { ); final DelegatingSqmAliasedNodeCollector collector = (DelegatingSqmAliasedNodeCollector) processingState .getSqlExpressionResolver(); - final SqmQueryPart sqmQueryPart = currentSqmQueryPart; - currentSqmQueryPart = queryGroup; + sqmQueryPartStack.push( queryGroup ); pushProcessingState( processingState ); try { @@ -2016,7 +2015,7 @@ public QueryGroup visitQueryGroup(SqmQueryGroup queryGroup) { } finally { popProcessingStateStack(); - currentSqmQueryPart = sqmQueryPart; + sqmQueryPartStack.pop(); } } @@ -2076,9 +2075,8 @@ else if ( sqmQuerySpec.hasPositionalGroupItem() ) { ); } - final SqmQueryPart sqmQueryPart = currentSqmQueryPart; final boolean originalDeduplicateSelectionItems = deduplicateSelectionItems; - currentSqmQueryPart = sqmQuerySpec; + sqmQueryPartStack.push( sqmQuerySpec ); // In sub-queries, we can never deduplicate the selection items as that might change semantics deduplicateSelectionItems = false; pushProcessingState( processingState ); @@ -2145,7 +2143,7 @@ else if ( sqmQuerySpec.hasPositionalGroupItem() ) { inNestedContext = oldInNestedContext; popProcessingStateStack(); queryTransformers.pop(); - currentSqmQueryPart = sqmQueryPart; + sqmQueryPartStack.pop(); deduplicateSelectionItems = originalDeduplicateSelectionItems; } } @@ -2211,7 +2209,7 @@ public SelectClause visitSelectClause(SqmSelectClause selectClause) { try { final SelectClause sqlSelectClause = currentQuerySpec().getSelectClause(); if ( selectClause == null ) { - final SqmFrom implicitSelection = determineImplicitSelection( (SqmQuerySpec) currentSqmQueryPart ); + final SqmFrom implicitSelection = determineImplicitSelection( (SqmQuerySpec) getCurrentSqmQueryPart() ); visitSelection( 0, new SqmSelection<>( implicitSelection, implicitSelection.nodeBuilder() ) ); } else { @@ -2236,7 +2234,7 @@ public SelectClause visitSelectClause(SqmSelectClause selectClause) { @Override public Void visitSelection(SqmSelection sqmSelection) { return visitSelection( - currentSqmQueryPart.getFirstQuerySpec().getSelectClause().getSelections().indexOf( sqmSelection ), + getCurrentSqmQueryPart().getFirstQuerySpec().getSelectClause().getSelections().indexOf( sqmSelection ), sqmSelection ); } @@ -2355,7 +2353,7 @@ else if ( statement.getQuerySource() == SqmQuerySource.CRITERIA && currentClause // To avoid this issue, we determine the position and let the SqlAstTranslator handle the rest. // Usually it will render `select ?, count(*) from dual group by 1` if supported // or force rendering the parameter as literal instead so that the database can see the grouping is fine - final SqmQuerySpec querySpec = currentSqmQueryPart.getFirstQuerySpec(); + final SqmQuerySpec querySpec = getCurrentSqmQueryPart().getFirstQuerySpec(); sqmPosition = indexOfExpression( querySpec.getSelectClause().getSelections(), groupByClauseExpression ); path = null; } @@ -2383,7 +2381,7 @@ else if ( statement.getQuerySource() == SqmQuerySource.CRITERIA && currentClause continue OUTER; } } - if ( currentSqmQueryPart instanceof SqmQueryGroup ) { + if ( getCurrentSqmQueryPart() instanceof SqmQueryGroup ) { // Reusing the SqlSelection for query groups would be wrong because the aliases do no exist // So we have to use a literal expression in a new SqlSelection instance to refer to the position expressions.add( diff --git a/hibernate-core/src/main/java/org/hibernate/query/sqm/sql/FakeSqmToSqlAstConverter.java b/hibernate-core/src/main/java/org/hibernate/query/sqm/sql/FakeSqmToSqlAstConverter.java index 02ffca60282d..d997c0f7c3f9 100644 --- a/hibernate-core/src/main/java/org/hibernate/query/sqm/sql/FakeSqmToSqlAstConverter.java +++ b/hibernate-core/src/main/java/org/hibernate/query/sqm/sql/FakeSqmToSqlAstConverter.java @@ -95,6 +95,11 @@ public Stack getCurrentClauseStack() { return null; } + @Override + public Stack getSqmQueryPartStack() { + return null; + } + @Override public SqmQueryPart getCurrentSqmQueryPart() { return null; diff --git a/hibernate-core/src/main/java/org/hibernate/query/sqm/sql/SqmToSqlAstConverter.java b/hibernate-core/src/main/java/org/hibernate/query/sqm/sql/SqmToSqlAstConverter.java index e932d9a4a9a4..3442b0958398 100644 --- a/hibernate-core/src/main/java/org/hibernate/query/sqm/sql/SqmToSqlAstConverter.java +++ b/hibernate-core/src/main/java/org/hibernate/query/sqm/sql/SqmToSqlAstConverter.java @@ -35,7 +35,11 @@ public interface SqmToSqlAstConverter extends SemanticQueryWalker, SqlAstCreationState { Stack getCurrentClauseStack(); - SqmQueryPart getCurrentSqmQueryPart(); + Stack getSqmQueryPartStack(); + + default SqmQueryPart getCurrentSqmQueryPart() { + return getSqmQueryPartStack().getCurrent(); + } void registerQueryTransformer(QueryTransformer transformer); diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/query/hql/ExistsSubqueryForeignKeyTest.java b/hibernate-core/src/test/java/org/hibernate/orm/test/query/hql/ExistsSubqueryForeignKeyTest.java new file mode 100644 index 000000000000..a5f86d55ebd8 --- /dev/null +++ b/hibernate-core/src/test/java/org/hibernate/orm/test/query/hql/ExistsSubqueryForeignKeyTest.java @@ -0,0 +1,112 @@ +/* + * SPDX-License-Identifier: LGPL-2.1-or-later + * Copyright Red Hat Inc. and Hibernate Authors + */ +package org.hibernate.orm.test.query.hql; + +import org.hibernate.dialect.DerbyDialect; +import org.hibernate.dialect.HSQLDialect; + +import org.hibernate.testing.orm.junit.DomainModel; +import org.hibernate.testing.orm.junit.Jira; +import org.hibernate.testing.orm.junit.SessionFactory; +import org.hibernate.testing.orm.junit.SessionFactoryScope; +import org.hibernate.testing.orm.junit.SkipForDialect; +import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Test; + +import jakarta.persistence.Entity; +import jakarta.persistence.Id; +import jakarta.persistence.ManyToOne; +import jakarta.persistence.Tuple; + +import static org.assertj.core.api.Assertions.assertThat; + +/** + * @author Marco Belladelli + */ +@DomainModel(annotatedClasses = { + ExistsSubqueryForeignKeyTest.Person.class, + ExistsSubqueryForeignKeyTest.Document.class, +}) +@SessionFactory +@SkipForDialect(dialectClass = HSQLDialect.class, reason = "HSQLDB doesn't like the case-when selection not being in the group-by") +@SkipForDialect(dialectClass = DerbyDialect.class, reason = "Derby doesn't like the case-when selection not being in the group-by") +@Jira( "https://hibernate.atlassian.net/browse/HHH-18816" ) +public class ExistsSubqueryForeignKeyTest { + @Test + public void testWhereClause(SessionFactoryScope scope) { + scope.inTransaction( session -> { + final Long result = session.createQuery( + "select count(*) from Document d join d.owner o " + + "where exists(select p.id from Person p where p.id = o.id) group by o.id", + Long.class + ).getSingleResult(); + assertThat( result ).isEqualTo( 1L ); + } ); + } + + @Test + public void testSelectCaseWhen(SessionFactoryScope scope) { + scope.inTransaction( session -> { + final Tuple result = session.createQuery( + "select case when exists(select p.id from Person p where p.id = o.id) then 1 else 0 end," + + "count(*) from Document d join d.owner o group by o.id", + Tuple.class + ).getSingleResult(); + assertThat( result.get( 0, Integer.class ) ).isEqualTo( 1 ); + assertThat( result.get( 1, Long.class ) ).isEqualTo( 1L ); + } ); + } + + @BeforeAll + public void setUp(SessionFactoryScope scope) { + scope.inTransaction( session -> { + final Person person1 = new Person( 1L, "person_1" ); + session.persist( person1 ); + session.persist( new Document( 1L, "doc_1", person1 ) ); + } ); + } + + @AfterAll + public void tearDown(SessionFactoryScope scope) { + scope.getSessionFactory().getSchemaManager().truncateMappedObjects(); + } + + @Entity(name = "Person") + static class Person { + @Id + private Long id; + + private String name; + + public Person() { + } + + public Person(Long id, String name) { + this.id = id; + this.name = name; + } + } + + @Entity(name = "Document") + static class Document { + @Id + private Long id; + + private String title; + + @ManyToOne + private Person owner; + + public Document() { + } + + public Document(Long id, String title, Person owner) { + this.id = id; + this.title = title; + this.owner = owner; + } + } +}