HHH-17225 Always use target table column for right / full joins

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

@@ -29,7 +29,9 @@ import org.hibernate.metamodel.mapping.EntityIdentifierMapping;
 import org.hibernate.metamodel.mapping.EntityMappingType;
 import org.hibernate.metamodel.mapping.ForeignKeyDescriptor;
 import org.hibernate.metamodel.mapping.JdbcMapping;
+import org.hibernate.metamodel.mapping.ManagedMappingType;
 import org.hibernate.metamodel.mapping.MappingModelExpressible;
+import org.hibernate.metamodel.mapping.ModelPartContainer;
 import org.hibernate.metamodel.mapping.PluralAttributeMapping;
 import org.hibernate.query.IllegalQueryOperationException;
 import org.hibernate.query.IllegalSelectQueryException;
@@ -42,7 +44,9 @@ import org.hibernate.query.sqm.NodeBuilder;
 import org.hibernate.query.sqm.SqmQuerySource;
 import org.hibernate.query.sqm.spi.JdbcParameterBySqmParameterAccess;
 import org.hibernate.query.sqm.spi.SqmParameterMappingModelResolutionAccess;
+import org.hibernate.query.sqm.sql.SqmToSqlAstConverter;
 import org.hibernate.query.sqm.tree.SqmDmlStatement;
+import org.hibernate.query.sqm.tree.SqmJoinType;
 import org.hibernate.query.sqm.tree.SqmStatement;
 import org.hibernate.query.sqm.tree.domain.SqmPath;
 import org.hibernate.query.sqm.tree.expression.JpaCriteriaParameter;
@@ -50,11 +54,14 @@ import org.hibernate.query.sqm.tree.expression.SqmAliasedNodeRef;
 import org.hibernate.query.sqm.tree.expression.SqmJpaCriteriaParameterWrapper;
 import org.hibernate.query.sqm.tree.expression.SqmParameter;
 import org.hibernate.query.sqm.tree.from.SqmFrom;
+import org.hibernate.query.sqm.tree.from.SqmJoin;
 import org.hibernate.query.sqm.tree.from.SqmRoot;
+import org.hibernate.query.sqm.tree.select.SqmQueryPart;
 import org.hibernate.query.sqm.tree.select.SqmSelectStatement;
 import org.hibernate.query.sqm.tree.select.SqmSelectableNode;
 import org.hibernate.query.sqm.tree.select.SqmSortSpecification;
 import org.hibernate.spi.NavigablePath;
+import org.hibernate.sql.ast.Clause;
 import org.hibernate.sql.ast.SqlTreeCreationException;
 import org.hibernate.sql.ast.tree.expression.JdbcParameter;
 import org.hibernate.sql.ast.tree.from.TableGroup;
@@ -120,6 +127,31 @@ public class SqmUtil {
 		);
 	}
 
+	public static boolean needsTargetTableMapping(
+			SqmPath<?> sqmPath,
+			ModelPartContainer modelPartContainer,
+			SqmToSqlAstConverter sqlAstCreationState) {
+		final Clause currentClause = sqlAstCreationState.getCurrentClauseStack().getCurrent();
+		return ( currentClause == Clause.GROUP || currentClause == Clause.SELECT || currentClause == Clause.ORDER || currentClause == Clause.HAVING )
+				&& modelPartContainer.getPartMappingType() != modelPartContainer
+				&& sqmPath.getLhs() instanceof SqmFrom<?, ?>
+				&& modelPartContainer.getPartMappingType() instanceof ManagedMappingType
+				&& ( groupByClauseContains( sqlAstCreationState.getCurrentSqmQueryPart(), sqmPath.getNavigablePath() )
+				|| isNonOptimizableJoin( sqmPath.getLhs() ) );
+	}
+
+	private static boolean groupByClauseContains(SqmQueryPart<?> sqmQueryPart, NavigablePath path) {
+		return sqmQueryPart.isSimpleQueryPart() && sqmQueryPart.getFirstQuerySpec().groupByClauseContains( path );
+	}
+
+	private static boolean isNonOptimizableJoin(SqmPath<?> sqmPath) {
+		if ( sqmPath instanceof SqmJoin<?, ?> ) {
+			final SqmJoinType sqmJoinType = ( (SqmJoin<?, ?>) sqmPath ).getSqmJoinType();
+			return sqmJoinType != SqmJoinType.INNER && sqmJoinType != SqmJoinType.LEFT;
+		}
+		return false;
+	}
+
 	public static Map<QueryParameterImplementor<?>, Map<SqmParameter<?>, List<JdbcParametersList>>> generateJdbcParamsXref(
 			DomainParameterXref domainParameterXref,
 			JdbcParameterBySqmParameterAccess jdbcParameterBySqmParameterAccess) {

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

@@ -25,10 +25,7 @@ import org.hibernate.query.sqm.sql.SqmToSqlAstConverter;
 import org.hibernate.query.sqm.tree.domain.SqmBasicValuedSimplePath;
 import org.hibernate.query.sqm.tree.domain.SqmPath;
 import org.hibernate.query.sqm.tree.domain.SqmTreatedPath;
-import org.hibernate.query.sqm.tree.from.SqmFrom;
-import org.hibernate.query.sqm.tree.select.SqmQueryPart;
 import org.hibernate.spi.NavigablePath;
-import org.hibernate.sql.ast.Clause;
 import org.hibernate.sql.ast.SqlAstWalker;
 import org.hibernate.sql.ast.tree.expression.ColumnReference;
 import org.hibernate.sql.ast.tree.expression.Expression;
@@ -37,6 +34,8 @@ import org.hibernate.sql.ast.tree.from.TableGroup;
 import org.hibernate.sql.ast.tree.from.TableReference;
 import org.hibernate.sql.ast.tree.update.Assignable;
 
+import static org.hibernate.query.sqm.internal.SqmUtil.needsTargetTableMapping;
+
 /**
  * @author Steve Ebersole
  */
@@ -83,16 +82,10 @@ public class BasicValuedPathInterpretation<T> extends AbstractSqmPathInterpretat
 		}
 
 		final BasicValuedModelPart mapping;
-		// In the select, group by, order by and having clause we have to make sure we render the column of the target table,
+		if ( needsTargetTableMapping( sqmPath, modelPartContainer, sqlAstCreationState ) ) {
-		// never the FK column, if the lhs is a SqmFrom i.e. something explicitly queried/joined
+			// In the select, group by, order by and having clause we have to make sure we render
-		// and if this basic path is part of the group by clause
+			// the column of the target table, never the FK column, if the lhs is a join type that
-		final Clause currentClause = sqlAstCreationState.getCurrentClauseStack().getCurrent();
+			// requires it (right, full) or if this path is contained in group by clause
-		final SqmQueryPart<?> sqmQueryPart = sqlAstCreationState.getCurrentSqmQueryPart();
-		if ( ( currentClause == Clause.GROUP || currentClause == Clause.SELECT || currentClause == Clause.ORDER || currentClause == Clause.HAVING )
-				&& lhs instanceof SqmFrom<?, ?>
-				&& modelPartContainer.getPartMappingType() instanceof ManagedMappingType
-				&& sqmQueryPart.isSimpleQueryPart()
-				&& sqmQueryPart.getFirstQuerySpec().groupByClauseContains( sqmPath.getNavigablePath() ) ) {
 			mapping = (BasicValuedModelPart) ( (ManagedMappingType) modelPartContainer.getPartMappingType() ).findSubPart(
 					sqmPath.getReferencedPathSource().getPathName(),
 					treatTarget

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

@@ -20,10 +20,7 @@ import org.hibernate.query.sqm.sql.SqmToSqlAstConverter;
 import org.hibernate.query.sqm.tree.domain.SqmEmbeddedValuedSimplePath;
 import org.hibernate.query.sqm.tree.domain.SqmPath;
 import org.hibernate.query.sqm.tree.domain.SqmTreatedPath;
-import org.hibernate.query.sqm.tree.from.SqmFrom;
-import org.hibernate.query.sqm.tree.select.SqmQueryPart;
 import org.hibernate.spi.NavigablePath;
-import org.hibernate.sql.ast.Clause;
 import org.hibernate.sql.ast.SqlAstWalker;
 import org.hibernate.sql.ast.tree.expression.ColumnReference;
 import org.hibernate.sql.ast.tree.expression.Expression;
@@ -32,6 +29,8 @@ import org.hibernate.sql.ast.tree.expression.SqlTupleContainer;
 import org.hibernate.sql.ast.tree.from.TableGroup;
 import org.hibernate.sql.ast.tree.update.Assignable;
 
+import static org.hibernate.query.sqm.internal.SqmUtil.needsTargetTableMapping;
+
 /**
  * @author Steve Ebersole
  */
@@ -65,25 +64,19 @@ public class EmbeddableValuedPathInterpretation<T> extends AbstractSqmPathInterp
 			}
 		}
 
-		final ModelPartContainer modelPart = tableGroup.getModelPart();
+		final ModelPartContainer modelPartContainer = tableGroup.getModelPart();
 		final EmbeddableValuedModelPart mapping;
-		// In the select, group by, order by and having clause we have to make sure we render the column of the target table,
+		if ( needsTargetTableMapping( sqmPath, modelPartContainer, sqlAstCreationState ) ) {
-		// never the FK column, if the lhs is a SqmFrom i.e. something explicitly queried/joined
+			// In the select, group by, order by and having clause we have to make sure we render
-		// and if this basic path is part of the group by clause
+			// the column of the target table, never the FK column, if the lhs is a join type that
-		final Clause currentClause = sqlAstCreationState.getCurrentClauseStack().getCurrent();
+			// requires it (right, full) or if this path is contained in group by clause
-		final SqmQueryPart<?> sqmQueryPart = sqlAstCreationState.getCurrentSqmQueryPart();
+			mapping = (EmbeddableValuedModelPart) ( (ManagedMappingType) modelPartContainer.getPartMappingType() ).findSubPart(
-		if ( ( currentClause == Clause.GROUP || currentClause == Clause.SELECT || currentClause == Clause.ORDER || currentClause == Clause.HAVING )
-				&& lhs instanceof SqmFrom<?, ?>
-				&& modelPart.getPartMappingType() instanceof ManagedMappingType
-				&& sqmQueryPart.isSimpleQueryPart()
-				&& sqmQueryPart.getFirstQuerySpec().groupByClauseContains( sqmPath.getNavigablePath() ) ) {
-			mapping = (EmbeddableValuedModelPart) ( (ManagedMappingType) modelPart.getPartMappingType() ).findSubPart(
 					sqmPath.getReferencedPathSource().getPathName(),
 					treatTarget
 			);
 		}
 		else {
-			mapping = (EmbeddableValuedModelPart) modelPart.findSubPart(
+			mapping = (EmbeddableValuedModelPart) modelPartContainer.findSubPart(
 					sqmPath.getReferencedPathSource().getPathName(),
 					treatTarget
 			);
@@ -92,7 +85,7 @@ public class EmbeddableValuedPathInterpretation<T> extends AbstractSqmPathInterp
 		return new EmbeddableValuedPathInterpretation<>(
 				mapping.toSqlExpression(
 						tableGroup,
-						currentClause,
+						sqlAstCreationState.getCurrentClauseStack().getCurrent(),
 						sqlAstCreationState,
 						sqlAstCreationState
 				),