Skip to content

Commit 1eb5c69

Browse files
mbelladembellade
committed
HHH-18816 FK rendering: check parent query specs for group by / order by
1 parent 563a748 commit 1eb5c69

File tree

4 files changed

+87
-53
lines changed

4 files changed

+87
-53
lines changed

‎hibernate-core/src/main/java/org/hibernate/query/sqm/internal/SqmUtil.java‎

Lines changed: 63 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@
44
*/
55
package org.hibernate.query.sqm.internal;
66

7+
import java.sql.Time;
8+
import java.sql.Timestamp;
79
import java.sql.Types;
810
import java.util.ArrayList;
911
import java.util.Collection;
@@ -21,6 +23,7 @@
2123
import org.hibernate.engine.spi.SessionFactoryImplementor;
2224
import org.hibernate.engine.spi.SharedSessionContractImplementor;
2325
import org.hibernate.internal.util.collections.CollectionHelper;
26+
import org.hibernate.internal.util.collections.Stack;
2427
import org.hibernate.jpa.spi.JpaCompliance;
2528
import org.hibernate.metamodel.mapping.BasicValuedMapping;
2629
import org.hibernate.metamodel.mapping.Bindable;
@@ -168,48 +171,72 @@ public static ModelPartContainer getTargetMappingIfNeeded(
168171
SqmPath<?> sqmPath,
169172
ModelPartContainer modelPartContainer,
170173
SqmToSqlAstConverter sqlAstCreationState) {
171-
final SqmQueryPart<?> queryPart = sqlAstCreationState.getCurrentSqmQueryPart();
172-
if ( queryPart != null ) {
173-
// We only need to do this for queries
174-
final Clause clause = sqlAstCreationState.getCurrentClauseStack().getCurrent();
175-
if ( clause != Clause.FROM && modelPartContainer.getPartMappingType() != modelPartContainer && sqmPath.getLhs() instanceof SqmFrom<?, ?> ) {
176-
final ModelPart modelPart;
177-
if ( modelPartContainer instanceof PluralAttributeMapping ) {
178-
modelPart = getCollectionPart(
179-
(PluralAttributeMapping) modelPartContainer,
180-
castNonNull( sqmPath.getNavigablePath().getParent() )
181-
);
182-
}
183-
else {
184-
modelPart = modelPartContainer;
185-
}
186-
if ( modelPart instanceof EntityAssociationMapping association ) {
187-
// If the path is one of the association's target key properties,
188-
// we need to render the target side if in group/order by
189-
if ( association.getTargetKeyPropertyNames().contains( sqmPath.getReferencedPathSource().getPathName() )
190-
&& ( clause == Clause.GROUP || clause == Clause.ORDER
191-
|| !isFkOptimizationAllowed( sqmPath.getLhs(), association )
192-
|| queryPart.getFirstQuerySpec().groupByClauseContains( sqmPath.getNavigablePath(), sqlAstCreationState )
193-
|| queryPart.getFirstQuerySpec().orderByClauseContains( sqmPath.getNavigablePath(), sqlAstCreationState ) ) ) {
194-
return association.getAssociatedEntityMappingType();
195-
}
174+
// We only need to do this for queries
175+
if ( sqlAstCreationState.getCurrentClauseStack().getCurrent() != Clause.FROM
176+
&& modelPartContainer.getPartMappingType() != modelPartContainer && sqmPath.getLhs() instanceof SqmFrom<?, ?> ) {
177+
final ModelPart modelPart;
178+
if ( modelPartContainer instanceof PluralAttributeMapping pluralAttributeMapping ) {
179+
modelPart = getCollectionPart(
180+
pluralAttributeMapping,
181+
castNonNull( sqmPath.getNavigablePath().getParent() )
182+
);
183+
}
184+
else {
185+
modelPart = modelPartContainer;
186+
}
187+
if ( modelPart instanceof EntityAssociationMapping association ) {
188+
if ( shouldRenderTargetSide( sqmPath, association, sqlAstCreationState ) ) {
189+
return association.getAssociatedEntityMappingType();
196190
}
197191
}
198192
}
199193
return modelPartContainer;
200194
}
201195

196+
private static boolean shouldRenderTargetSide(
197+
SqmPath<?> sqmPath,
198+
EntityAssociationMapping association,
199+
SqmToSqlAstConverter sqlAstCreationState) {
200+
if ( !association.getTargetKeyPropertyNames().contains( sqmPath.getReferencedPathSource().getPathName() ) ) {
201+
return false;
202+
}
203+
// If the path is one of the association's target key properties,
204+
// we need to render the target side if in group/order by
205+
final Clause clause = sqlAstCreationState.getCurrentClauseStack().getCurrent();
206+
return clause == Clause.GROUP || clause == Clause.ORDER
207+
|| !isFkOptimizationAllowed( sqmPath.getLhs(), association )
208+
|| clauseContainsPath( Clause.GROUP, sqmPath, sqlAstCreationState )
209+
|| clauseContainsPath( Clause.ORDER, sqmPath, sqlAstCreationState );
210+
}
211+
212+
private static boolean clauseContainsPath(
213+
Clause clauseToCheck,
214+
SqmPath<?> sqmPath,
215+
SqmToSqlAstConverter sqlAstCreationState) {
216+
final Stack<SqmQueryPart> queryPartStack = sqlAstCreationState.getSqmQueryPartStack();
217+
final NavigablePath navigablePath = sqmPath.getNavigablePath();
218+
final Boolean found = queryPartStack.findCurrentFirst( queryPart -> {
219+
final SqmQuerySpec<?> spec = queryPart.getFirstQuerySpec();
220+
if ( clauseToCheck == Clause.GROUP && spec.groupByClauseContains( navigablePath, sqlAstCreationState ) ) {
221+
return true;
222+
}
223+
else if ( clauseToCheck == Clause.ORDER && spec.orderByClauseContains( navigablePath, sqlAstCreationState ) ) {
224+
return true;
225+
}
226+
else {
227+
return null;
228+
}
229+
} );
230+
return Boolean.TRUE.equals( found );
231+
}
232+
202233
private static CollectionPart getCollectionPart(PluralAttributeMapping attribute, NavigablePath path) {
203234
final CollectionPart.Nature nature = CollectionPart.Nature.fromNameExact( path.getLocalName() );
204-
if ( nature != null ) {
205-
switch ( nature ) {
206-
case ELEMENT:
207-
return attribute.getElementDescriptor();
208-
case INDEX:
209-
return attribute.getIndexDescriptor();
210-
}
211-
}
212-
return null;
235+
return nature != null ? switch ( nature ) {
236+
case ELEMENT -> attribute.getElementDescriptor();
237+
case INDEX -> attribute.getIndexDescriptor();
238+
case ID -> attribute.getIdentifierDescriptor();
239+
} : null;
213240
}
214241

215242
/**
@@ -1168,8 +1195,8 @@ private static boolean isMatchingDateJdbcType(Class<?> resultClass, JdbcType jdb
11681195
if ( jdbcType != null ) {
11691196
return switch ( jdbcType.getDefaultSqlTypeCode() ) {
11701197
case Types.DATE -> resultClass.isAssignableFrom(java.sql.Date.class);
1171-
case Types.TIME -> resultClass.isAssignableFrom(java.sql.Time.class);
1172-
case Types.TIMESTAMP -> resultClass.isAssignableFrom(java.sql.Timestamp.class);
1198+
case Types.TIME -> resultClass.isAssignableFrom( Time.class);
1199+
case Types.TIMESTAMP -> resultClass.isAssignableFrom( Timestamp.class);
11731200
default -> false;
11741201
};
11751202
}

‎hibernate-core/src/main/java/org/hibernate/query/sqm/sql/BaseSqmToSqlAstConverter.java‎

Lines changed: 14 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -488,7 +488,7 @@ public abstract class BaseSqmToSqlAstConverter<T extends Statement> extends Base
488488
private boolean deduplicateSelectionItems;
489489
private ForeignKeyDescriptor.Nature currentlyResolvingForeignKeySide;
490490
private SqmStatement<?> currentSqmStatement;
491-
private SqmQueryPart<?> currentSqmQueryPart;
491+
private Stack<SqmQueryPart> sqmQueryPartStack = new StandardStack<>( SqmQueryPart.class );
492492
private CteContainer cteContainer;
493493
/**
494494
* A map from {@link SqmCteTable#getCteName()} to the final SQL name.
@@ -792,9 +792,10 @@ public Stack<Clause> getCurrentClauseStack() {
792792
return currentClauseStack;
793793
}
794794

795+
@SuppressWarnings({ "rawtypes", "unchecked" })
795796
@Override
796-
public SqmQueryPart<?> getCurrentSqmQueryPart() {
797-
return currentSqmQueryPart;
797+
public Stack<SqmQueryPart> getSqmQueryPartStack() {
798+
return sqmQueryPartStack;
798799
}
799800

800801
// ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
@@ -1726,8 +1727,7 @@ public CteStatement visitCteStatement(SqmCteStatement<?> sqmCteStatement) {
17261727
);
17271728
final DelegatingSqmAliasedNodeCollector collector =
17281729
(DelegatingSqmAliasedNodeCollector) processingState.getSqlExpressionResolver();
1729-
final SqmQueryPart<?> oldSqmQueryPart = currentSqmQueryPart;
1730-
currentSqmQueryPart = queryGroup;
1730+
sqmQueryPartStack.push( queryGroup );
17311731
pushProcessingState( processingState );
17321732

17331733
try {
@@ -1771,7 +1771,7 @@ public CteStatement visitCteStatement(SqmCteStatement<?> sqmCteStatement) {
17711771
}
17721772
finally {
17731773
popProcessingStateStack();
1774-
currentSqmQueryPart = oldSqmQueryPart;
1774+
sqmQueryPartStack.pop();
17751775
}
17761776
}
17771777
finally {
@@ -1985,8 +1985,7 @@ public QueryGroup visitQueryGroup(SqmQueryGroup<?> queryGroup) {
19851985
);
19861986
final DelegatingSqmAliasedNodeCollector collector = (DelegatingSqmAliasedNodeCollector) processingState
19871987
.getSqlExpressionResolver();
1988-
final SqmQueryPart<?> sqmQueryPart = currentSqmQueryPart;
1989-
currentSqmQueryPart = queryGroup;
1988+
sqmQueryPartStack.push( queryGroup );
19901989
pushProcessingState( processingState );
19911990

19921991
try {
@@ -2008,7 +2007,7 @@ public QueryGroup visitQueryGroup(SqmQueryGroup<?> queryGroup) {
20082007
}
20092008
finally {
20102009
popProcessingStateStack();
2011-
currentSqmQueryPart = sqmQueryPart;
2010+
sqmQueryPartStack.pop();
20122011
}
20132012
}
20142013

@@ -2043,9 +2042,8 @@ public QuerySpec visitQuerySpec(SqmQuerySpec<?> sqmQuerySpec) {
20432042
);
20442043
}
20452044

2046-
final SqmQueryPart<?> sqmQueryPart = currentSqmQueryPart;
20472045
final boolean originalDeduplicateSelectionItems = deduplicateSelectionItems;
2048-
currentSqmQueryPart = sqmQuerySpec;
2046+
sqmQueryPartStack.push( sqmQuerySpec );
20492047
// In sub-queries, we can never deduplicate the selection items as that might change semantics
20502048
deduplicateSelectionItems = false;
20512049
pushProcessingState( processingState );
@@ -2062,7 +2060,7 @@ public QuerySpec visitQuerySpec(SqmQuerySpec<?> sqmQuerySpec) {
20622060
inNestedContext = oldInNestedContext;
20632061
popProcessingStateStack();
20642062
queryTransformers.pop();
2065-
currentSqmQueryPart = sqmQueryPart;
2063+
sqmQueryPartStack.pop();
20662064
deduplicateSelectionItems = originalDeduplicateSelectionItems;
20672065
}
20682066
}
@@ -2203,7 +2201,7 @@ public SelectClause visitSelectClause(SqmSelectClause selectClause) {
22032201
try {
22042202
final SelectClause sqlSelectClause = currentQuerySpec().getSelectClause();
22052203
if ( selectClause == null ) {
2206-
final SqmFrom<?, ?> implicitSelection = determineImplicitSelection( (SqmQuerySpec<?>) currentSqmQueryPart );
2204+
final SqmFrom<?, ?> implicitSelection = determineImplicitSelection( (SqmQuerySpec<?>) getCurrentSqmQueryPart() );
22072205
visitSelection( 0, new SqmSelection<>( implicitSelection, implicitSelection.nodeBuilder() ) );
22082206
}
22092207
else {
@@ -2228,7 +2226,7 @@ public SelectClause visitSelectClause(SqmSelectClause selectClause) {
22282226
@Override
22292227
public Void visitSelection(SqmSelection<?> sqmSelection) {
22302228
visitSelection(
2231-
currentSqmQueryPart.getFirstQuerySpec().getSelectClause().getSelections().indexOf( sqmSelection ),
2229+
getCurrentSqmQueryPart().getFirstQuerySpec().getSelectClause().getSelections().indexOf( sqmSelection ),
22322230
sqmSelection
22332231
);
22342232
return null;
@@ -2353,7 +2351,7 @@ else if ( statement.getQuerySource() == SqmQuerySource.CRITERIA && currentClause
23532351
// To avoid this issue, we determine the position and let the SqlAstTranslator handle the rest.
23542352
// Usually it will render `select ?, count(*) from dual group by 1` if supported
23552353
// or force rendering the parameter as literal instead so that the database can see the grouping is fine
2356-
final SqmQuerySpec<?> querySpec = currentSqmQueryPart.getFirstQuerySpec();
2354+
final SqmQuerySpec<?> querySpec = getCurrentSqmQueryPart().getFirstQuerySpec();
23572355
sqmPosition = indexOfExpression( querySpec.getSelectClause().getSelections(), groupByClauseExpression );
23582356
path = null;
23592357
}
@@ -2395,7 +2393,7 @@ private SqlSelectionExpression selectionExpression(List<SqlSelection> selections
23952393
return null;
23962394
}
23972395
}
2398-
return currentSqmQueryPart instanceof SqmQueryGroup<?>
2396+
return getCurrentSqmQueryPart() instanceof SqmQueryGroup<?>
23992397
// Reusing the SqlSelection for query groups would be wrong because the aliases do no exist
24002398
// So we have to use a literal expression in a new SqlSelection instance to refer to the position
24012399
? sqlSelectionExpression( selection )

‎hibernate-core/src/main/java/org/hibernate/query/sqm/sql/FakeSqmToSqlAstConverter.java‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,11 @@ public Stack<Clause> getCurrentClauseStack() {
9292
return null;
9393
}
9494

95+
@Override
96+
public Stack<SqmQueryPart> getSqmQueryPartStack() {
97+
return null;
98+
}
99+
95100
@Override
96101
public SqmQueryPart<?> getCurrentSqmQueryPart() {
97102
return null;

‎hibernate-core/src/main/java/org/hibernate/query/sqm/sql/SqmToSqlAstConverter.java‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,11 @@
3333
public interface SqmToSqlAstConverter extends SemanticQueryWalker<Object>, SqlAstCreationState {
3434
Stack<Clause> getCurrentClauseStack();
3535

36-
SqmQueryPart<?> getCurrentSqmQueryPart();
36+
Stack<SqmQueryPart> getSqmQueryPartStack();
37+
38+
default SqmQueryPart<?> getCurrentSqmQueryPart() {
39+
return getSqmQueryPartStack().getCurrent();
40+
}
3741

3842
void registerQueryTransformer(QueryTransformer transformer);
3943

0 commit comments

Comments
 (0)