From d59ecb633be3fc8807a3d15872ddc763abb9a9cc Mon Sep 17 00:00:00 2001 From: Marco Belladelli Date: Fri, 22 Sep 2023 11:16:48 +0200 Subject: [PATCH] HHH-17231 Reintroduce support for entity path expansion in subqueries --- .../EntityValuedPathInterpretation.java | 52 +++++++++++-------- .../sql/ast/spi/AbstractSqlAstTranslator.java | 11 ++-- 2 files changed, 38 insertions(+), 25 deletions(-) diff --git a/hibernate-core/src/main/java/org/hibernate/query/sqm/sql/internal/EntityValuedPathInterpretation.java b/hibernate-core/src/main/java/org/hibernate/query/sqm/sql/internal/EntityValuedPathInterpretation.java index 66b8d5d921..527e73cb5c 100644 --- a/hibernate-core/src/main/java/org/hibernate/query/sqm/sql/internal/EntityValuedPathInterpretation.java +++ b/hibernate-core/src/main/java/org/hibernate/query/sqm/sql/internal/EntityValuedPathInterpretation.java @@ -261,9 +261,9 @@ public class EntityValuedPathInterpretation extends AbstractSqmPathInterpreta SqmToSqlAstConverter sqlAstCreationState) { final boolean expandToAllColumns; final Clause currentClause = sqlAstCreationState.getCurrentClauseStack().getCurrent(); - if ( sqlAstCreationState.getCurrentProcessingState().isTopLevel() && - ( currentClause == Clause.GROUP || currentClause == Clause.ORDER ) ) { - final SqmQuerySpec querySpec = (SqmQuerySpec) sqlAstCreationState.getCurrentSqmQueryPart(); + if ( currentClause == Clause.GROUP || currentClause == Clause.ORDER ) { + assert sqlAstCreationState.getCurrentSqmQueryPart().isSimpleQueryPart(); + final SqmQuerySpec querySpec = sqlAstCreationState.getCurrentSqmQueryPart().getFirstQuerySpec(); if ( currentClause == Clause.ORDER && !querySpec.groupByClauseContains( navigablePath ) ) { // We must ensure that the order by expression be expanded but only if the group by // contained the same expression, and that was expanded as well @@ -272,7 +272,12 @@ public class EntityValuedPathInterpretation extends AbstractSqmPathInterpreta else { // When the table group is selected and the navigablePath is selected we need to expand // to all columns, as we must make sure we include all columns present in the select clause - expandToAllColumns = isSelected( tableGroup, navigablePath, querySpec ); + expandToAllColumns = isSelected( + tableGroup, + navigablePath, + querySpec, + sqlAstCreationState.getCurrentProcessingState().isTopLevel() + ); } } else { @@ -351,38 +356,43 @@ public class EntityValuedPathInterpretation extends AbstractSqmPathInterpreta ); } - private static boolean isSelected(TableGroup tableGroup, NavigablePath path, SqmQuerySpec sqmQuerySpec) { - // If the table group is selected (initialized), check if the entity valued - // navigable path or any child path appears in the select clause - return tableGroup.isInitialized() && selectClauseContains( path, sqmQuerySpec ); - } - - private static boolean selectClauseContains(NavigablePath path, SqmQuerySpec sqmQuerySpec) { - final List> selections = sqmQuerySpec.getSelectClause() == null - ? Collections.emptyList() - : sqmQuerySpec.getSelectClause().getSelections(); - for ( SqmSelection selection : selections ) { - if ( selectionContains( selection.getSelectableNode(), path ) ) { + private static boolean isSelected( + TableGroup tableGroup, + NavigablePath path, + SqmQuerySpec sqmQuerySpec, + boolean isTopLevel) { + // If the table group is not initialized, i.e. not selected, no need to check selections + if ( !tableGroup.isInitialized() || sqmQuerySpec.getSelectClause() == null ) { + return false; + } + final NavigablePath tableGroupPath = isTopLevel ? null : tableGroup.getNavigablePath(); + for ( SqmSelection selection : sqmQuerySpec.getSelectClause().getSelections() ) { + if ( selectionContains( selection.getSelectableNode(), path, tableGroupPath ) ) { return true; } } return false; } - private static boolean selectionContains(Selection selection, NavigablePath path) { - if ( selection instanceof SqmPath && path.isParentOrEqual( ( (SqmPath) selection ).getNavigablePath() ) ) { - return true; + private static boolean selectionContains(Selection selection, NavigablePath path, NavigablePath tableGroupPath) { + if ( selection instanceof SqmPath ) { + final SqmPath sqmPath = (SqmPath) selection; + // Expansion is needed if the table group is null, i.e. we're in a top level query where EVPs are always + // expanded to all columns, or if the selection is on the same table (lhs) as the group by expression ... + return ( tableGroupPath == null || sqmPath.getLhs().getNavigablePath().equals( tableGroupPath ) ) + // ... and if the entity valued path is selected or any of its columns are + && path.isParentOrEqual( sqmPath.getNavigablePath() ); } else if ( selection.isCompoundSelection() ) { for ( Selection compoundSelection : selection.getCompoundSelectionItems() ) { - if ( selectionContains( compoundSelection, path ) ) { + if ( selectionContains( compoundSelection, path, tableGroupPath ) ) { return true; } } } else if ( selection instanceof SqmDynamicInstantiation ) { for ( SqmDynamicInstantiationArgument argument : ( (SqmDynamicInstantiation) selection ).getArguments() ) { - if ( selectionContains( argument.getSelectableNode(), path ) ) { + if ( selectionContains( argument.getSelectableNode(), path, tableGroupPath ) ) { return true; } } diff --git a/hibernate-core/src/main/java/org/hibernate/sql/ast/spi/AbstractSqlAstTranslator.java b/hibernate-core/src/main/java/org/hibernate/sql/ast/spi/AbstractSqlAstTranslator.java index 962a060832..dced4e6541 100644 --- a/hibernate-core/src/main/java/org/hibernate/sql/ast/spi/AbstractSqlAstTranslator.java +++ b/hibernate-core/src/main/java/org/hibernate/sql/ast/spi/AbstractSqlAstTranslator.java @@ -7084,12 +7084,13 @@ public abstract class AbstractSqlAstTranslator implemen this.queryPartForRowNumberingClauseDepth = -1; this.needsSelectAliases = false; queryPartStack.push( subQuery ); - appendSql( "exists (select 1" ); - visitFromClause( subQuery.getFromClause() ); - + appendSql( "exists (" ); if ( !subQuery.getGroupByClauseExpressions().isEmpty() || subQuery.getHavingClauseRestrictions() != null ) { - // If we have a group by or having clause, we have to move the tuple comparison emulation to the HAVING clause + // If we have a group by or having clause, we have to move the tuple comparison emulation to the HAVING clause. + // Also, we need to explicitly include the selections to avoid 'invalid HAVING clause' errors + visitSelectClause( subQuery.getSelectClause() ); + visitFromClause( subQuery.getFromClause() ); visitWhereClause( subQuery.getWhereClauseRestrictions() ); visitGroupByClause( subQuery, SelectItemReferenceStrategy.EXPRESSION ); @@ -7114,6 +7115,8 @@ public abstract class AbstractSqlAstTranslator implemen } else { // If we have no group by or having clause, we can move the tuple comparison emulation to the WHERE clause + appendSql( "select 1" ); + visitFromClause( subQuery.getFromClause() ); appendSql( " where " ); clauseStack.push( Clause.WHERE ); try {