Skip to content

Commit 2d5df89

Browse files
mbelladebeikov
mbellade
authored and
beikov
committed
HHH-17225 Always use target table column for right / full joins
1 parent f163462 commit 2d5df89

File tree

3 files changed

+53
-33
lines changed

3 files changed

+53
-33
lines changed

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

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,9 @@
2929
import org.hibernate.metamodel.mapping.EntityMappingType;
3030
import org.hibernate.metamodel.mapping.ForeignKeyDescriptor;
3131
import org.hibernate.metamodel.mapping.JdbcMapping;
32+
import org.hibernate.metamodel.mapping.ManagedMappingType;
3233
import org.hibernate.metamodel.mapping.MappingModelExpressible;
34+
import org.hibernate.metamodel.mapping.ModelPartContainer;
3335
import org.hibernate.metamodel.mapping.PluralAttributeMapping;
3436
import org.hibernate.query.IllegalQueryOperationException;
3537
import org.hibernate.query.IllegalSelectQueryException;
@@ -39,14 +41,21 @@
3941
import org.hibernate.query.sqm.SqmQuerySource;
4042
import org.hibernate.query.sqm.spi.JdbcParameterBySqmParameterAccess;
4143
import org.hibernate.query.sqm.spi.SqmParameterMappingModelResolutionAccess;
44+
import org.hibernate.query.sqm.sql.SqmToSqlAstConverter;
4245
import org.hibernate.query.sqm.tree.SqmDmlStatement;
46+
import org.hibernate.query.sqm.tree.SqmJoinType;
4347
import org.hibernate.query.sqm.tree.SqmStatement;
48+
import org.hibernate.query.sqm.tree.domain.SqmPath;
4449
import org.hibernate.query.sqm.tree.expression.JpaCriteriaParameter;
4550
import org.hibernate.query.sqm.tree.expression.SqmJpaCriteriaParameterWrapper;
4651
import org.hibernate.query.sqm.tree.expression.SqmParameter;
52+
import org.hibernate.query.sqm.tree.from.SqmFrom;
53+
import org.hibernate.query.sqm.tree.from.SqmJoin;
54+
import org.hibernate.query.sqm.tree.select.SqmQueryPart;
4755
import org.hibernate.query.sqm.tree.jpa.ParameterCollector;
4856
import org.hibernate.query.sqm.tree.select.SqmSelectStatement;
4957
import org.hibernate.spi.NavigablePath;
58+
import org.hibernate.sql.ast.Clause;
5059
import org.hibernate.sql.ast.SqlTreeCreationException;
5160
import org.hibernate.sql.ast.tree.expression.JdbcParameter;
5261
import org.hibernate.sql.ast.tree.from.TableGroup;
@@ -110,6 +119,31 @@ public static IllegalQueryOperationException expectingNonSelect(SqmStatement<?>
110119
);
111120
}
112121

122+
public static boolean needsTargetTableMapping(
123+
SqmPath<?> sqmPath,
124+
ModelPartContainer modelPartContainer,
125+
SqmToSqlAstConverter sqlAstCreationState) {
126+
final Clause currentClause = sqlAstCreationState.getCurrentClauseStack().getCurrent();
127+
return ( currentClause == Clause.GROUP || currentClause == Clause.SELECT || currentClause == Clause.ORDER || currentClause == Clause.HAVING )
128+
&& modelPartContainer.getPartMappingType() != modelPartContainer
129+
&& sqmPath.getLhs() instanceof SqmFrom<?, ?>
130+
&& modelPartContainer.getPartMappingType() instanceof ManagedMappingType
131+
&& ( groupByClauseContains( sqlAstCreationState.getCurrentSqmQueryPart(), sqmPath.getNavigablePath() )
132+
|| isNonOptimizableJoin( sqmPath.getLhs() ) );
133+
}
134+
135+
private static boolean groupByClauseContains(SqmQueryPart<?> sqmQueryPart, NavigablePath path) {
136+
return sqmQueryPart.isSimpleQueryPart() && sqmQueryPart.getFirstQuerySpec().groupByClauseContains( path );
137+
}
138+
139+
private static boolean isNonOptimizableJoin(SqmPath<?> sqmPath) {
140+
if ( sqmPath instanceof SqmJoin<?, ?> ) {
141+
final SqmJoinType sqmJoinType = ( (SqmJoin<?, ?>) sqmPath ).getSqmJoinType();
142+
return sqmJoinType != SqmJoinType.INNER && sqmJoinType != SqmJoinType.LEFT;
143+
}
144+
return false;
145+
}
146+
113147
public static Map<QueryParameterImplementor<?>, Map<SqmParameter<?>, List<JdbcParametersList>>> generateJdbcParamsXref(
114148
DomainParameterXref domainParameterXref,
115149
JdbcParameterBySqmParameterAccess jdbcParameterBySqmParameterAccess) {

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

Lines changed: 9 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,10 @@
66
*/
77
package org.hibernate.query.sqm.sql.internal;
88

9+
import java.util.Collections;
10+
import java.util.List;
11+
import java.util.function.Consumer;
12+
913
import org.hibernate.metamodel.MappingMetamodel;
1014
import org.hibernate.metamodel.mapping.*;
1115
import org.hibernate.metamodel.model.domain.EntityDomainType;
@@ -16,10 +20,7 @@
1620
import org.hibernate.query.sqm.tree.domain.SqmBasicValuedSimplePath;
1721
import org.hibernate.query.sqm.tree.domain.SqmPath;
1822
import org.hibernate.query.sqm.tree.domain.SqmTreatedPath;
19-
import org.hibernate.query.sqm.tree.from.SqmFrom;
20-
import org.hibernate.query.sqm.tree.select.SqmQueryPart;
2123
import org.hibernate.spi.NavigablePath;
22-
import org.hibernate.sql.ast.Clause;
2324
import org.hibernate.sql.ast.SqlAstWalker;
2425
import org.hibernate.sql.ast.tree.expression.ColumnReference;
2526
import org.hibernate.sql.ast.tree.expression.Expression;
@@ -28,9 +29,7 @@
2829
import org.hibernate.sql.ast.tree.from.TableReference;
2930
import org.hibernate.sql.ast.tree.update.Assignable;
3031

31-
import java.util.Collections;
32-
import java.util.List;
33-
import java.util.function.Consumer;
32+
import static org.hibernate.query.sqm.internal.SqmUtil.needsTargetTableMapping;
3433

3534
/**
3635
* @author Steve Ebersole
@@ -78,16 +77,10 @@ public static <T> BasicValuedPathInterpretation<T> from(
7877
}
7978

8079
final BasicValuedModelPart mapping;
81-
// In the select, group by, order by and having clause we have to make sure we render the column of the target table,
82-
// never the FK column, if the lhs is a SqmFrom i.e. something explicitly queried/joined
83-
// and if this basic path is part of the group by clause
84-
final Clause currentClause = sqlAstCreationState.getCurrentClauseStack().getCurrent();
85-
final SqmQueryPart<?> sqmQueryPart = sqlAstCreationState.getCurrentSqmQueryPart();
86-
if ( ( currentClause == Clause.GROUP || currentClause == Clause.SELECT || currentClause == Clause.ORDER || currentClause == Clause.HAVING )
87-
&& lhs instanceof SqmFrom<?, ?>
88-
&& modelPartContainer.getPartMappingType() instanceof ManagedMappingType
89-
&& sqmQueryPart.isSimpleQueryPart()
90-
&& sqmQueryPart.getFirstQuerySpec().groupByClauseContains( sqmPath.getNavigablePath() ) ) {
80+
if ( needsTargetTableMapping( sqmPath, modelPartContainer, sqlAstCreationState ) ) {
81+
// In the select, group by, order by and having clause we have to make sure we render
82+
// the column of the target table, never the FK column, if the lhs is a join type that
83+
// requires it (right, full) or if this path is contained in group by clause
9184
mapping = (BasicValuedModelPart) ( (ManagedMappingType) modelPartContainer.getPartMappingType() ).findSubPart(
9285
sqmPath.getReferencedPathSource().getPathName(),
9386
treatTarget

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

Lines changed: 10 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -20,10 +20,7 @@
2020
import org.hibernate.query.sqm.tree.domain.SqmEmbeddedValuedSimplePath;
2121
import org.hibernate.query.sqm.tree.domain.SqmPath;
2222
import org.hibernate.query.sqm.tree.domain.SqmTreatedPath;
23-
import org.hibernate.query.sqm.tree.from.SqmFrom;
24-
import org.hibernate.query.sqm.tree.select.SqmQueryPart;
2523
import org.hibernate.spi.NavigablePath;
26-
import org.hibernate.sql.ast.Clause;
2724
import org.hibernate.sql.ast.SqlAstWalker;
2825
import org.hibernate.sql.ast.tree.expression.ColumnReference;
2926
import org.hibernate.sql.ast.tree.expression.Expression;
@@ -32,6 +29,8 @@
3229
import org.hibernate.sql.ast.tree.from.TableGroup;
3330
import org.hibernate.sql.ast.tree.update.Assignable;
3431

32+
import static org.hibernate.query.sqm.internal.SqmUtil.needsTargetTableMapping;
33+
3534
/**
3635
* @author Steve Ebersole
3736
*/
@@ -65,25 +64,19 @@ else if ( lhs.getNodeType() instanceof EntityDomainType ) {
6564
}
6665
}
6766

68-
final ModelPartContainer modelPart = tableGroup.getModelPart();
67+
final ModelPartContainer modelPartContainer = tableGroup.getModelPart();
6968
final EmbeddableValuedModelPart mapping;
70-
// In the select, group by, order by and having clause we have to make sure we render the column of the target table,
71-
// never the FK column, if the lhs is a SqmFrom i.e. something explicitly queried/joined
72-
// and if this basic path is part of the group by clause
73-
final Clause currentClause = sqlAstCreationState.getCurrentClauseStack().getCurrent();
74-
final SqmQueryPart<?> sqmQueryPart = sqlAstCreationState.getCurrentSqmQueryPart();
75-
if ( ( currentClause == Clause.GROUP || currentClause == Clause.SELECT || currentClause == Clause.ORDER || currentClause == Clause.HAVING )
76-
&& lhs instanceof SqmFrom<?, ?>
77-
&& modelPart.getPartMappingType() instanceof ManagedMappingType
78-
&& sqmQueryPart.isSimpleQueryPart()
79-
&& sqmQueryPart.getFirstQuerySpec().groupByClauseContains( sqmPath.getNavigablePath() ) ) {
80-
mapping = (EmbeddableValuedModelPart) ( (ManagedMappingType) modelPart.getPartMappingType() ).findSubPart(
69+
if ( needsTargetTableMapping( sqmPath, modelPartContainer, sqlAstCreationState ) ) {
70+
// In the select, group by, order by and having clause we have to make sure we render
71+
// the column of the target table, never the FK column, if the lhs is a join type that
72+
// requires it (right, full) or if this path is contained in group by clause
73+
mapping = (EmbeddableValuedModelPart) ( (ManagedMappingType) modelPartContainer.getPartMappingType() ).findSubPart(
8174
sqmPath.getReferencedPathSource().getPathName(),
8275
treatTarget
8376
);
8477
}
8578
else {
86-
mapping = (EmbeddableValuedModelPart) modelPart.findSubPart(
79+
mapping = (EmbeddableValuedModelPart) modelPartContainer.findSubPart(
8780
sqmPath.getReferencedPathSource().getPathName(),
8881
treatTarget
8982
);
@@ -92,7 +85,7 @@ else if ( lhs.getNodeType() instanceof EntityDomainType ) {
9285
return new EmbeddableValuedPathInterpretation<>(
9386
mapping.toSqlExpression(
9487
tableGroup,
95-
currentClause,
88+
sqlAstCreationState.getCurrentClauseStack().getCurrent(),
9689
sqlAstCreationState,
9790
sqlAstCreationState
9891
),

0 commit comments

Comments
 (0)