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 6542eaec3d88..6da178f30f3e 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 @@ -1813,11 +1813,14 @@ public CteContainer visitCteContainer(SqmCteContainer consumer) { final Collection> sqmCteStatements = consumer.getCteStatements(); cteContainer = new CteContainerImpl( cteContainer ); if ( !sqmCteStatements.isEmpty() ) { + final boolean originalDeduplicateSelectionItems = deduplicateSelectionItems; + deduplicateSelectionItems = false; currentClauseStack.push( Clause.WITH ); for ( SqmCteStatement sqmCteStatement : sqmCteStatements ) { visitCteStatement( sqmCteStatement ); } currentClauseStack.pop(); + deduplicateSelectionItems = originalDeduplicateSelectionItems; // Avoid leaking the processing state from CTEs to upper levels lastPoppedFromClauseIndex = null; lastPoppedProcessingState = null; diff --git a/hibernate-core/src/main/java/org/hibernate/query/sqm/tree/cte/SqmCteTable.java b/hibernate-core/src/main/java/org/hibernate/query/sqm/tree/cte/SqmCteTable.java index f50113fde074..a505a79fc1bb 100644 --- a/hibernate-core/src/main/java/org/hibernate/query/sqm/tree/cte/SqmCteTable.java +++ b/hibernate-core/src/main/java/org/hibernate/query/sqm/tree/cte/SqmCteTable.java @@ -17,7 +17,6 @@ import org.hibernate.query.sqm.tuple.internal.CteTupleTableGroupProducer; import org.hibernate.query.sqm.SqmPathSource; import org.hibernate.query.sqm.tree.select.SqmSelectQuery; -import org.hibernate.query.sqm.tree.select.SqmSelectableNode; import org.hibernate.sql.ast.spi.FromClauseAccess; import org.hibernate.sql.ast.spi.SqlSelection; import org.hibernate.type.BasicType; @@ -34,8 +33,8 @@ public class SqmCteTable extends AnonymousTupleType implements JpaCteCrite private SqmCteTable( String name, SqmCteStatement cteStatement, - SqmSelectableNode[] sqmSelectableNodes) { - super( sqmSelectableNodes ); + SqmSelectQuery selectStatement) { + super(selectStatement); this.name = name; this.cteStatement = cteStatement; final List columns = new ArrayList<>( componentCount() ); @@ -49,12 +48,7 @@ public static SqmCteTable createStatementTable( String name, SqmCteStatement cteStatement, SqmSelectQuery selectStatement) { - final SqmSelectableNode[] sqmSelectableNodes = selectStatement.getQueryPart() - .getFirstQuerySpec() - .getSelectClause() - .getSelectionItems() - .toArray( SqmSelectableNode[]::new ); - return new SqmCteTable<>( name, cteStatement, sqmSelectableNodes ); + return new SqmCteTable<>( name, cteStatement, selectStatement ); } @Override diff --git a/hibernate-core/src/main/java/org/hibernate/query/sqm/tuple/internal/AnonymousTupleType.java b/hibernate-core/src/main/java/org/hibernate/query/sqm/tuple/internal/AnonymousTupleType.java index 819d0be474dc..5e3551ada6fb 100644 --- a/hibernate-core/src/main/java/org/hibernate/query/sqm/tuple/internal/AnonymousTupleType.java +++ b/hibernate-core/src/main/java/org/hibernate/query/sqm/tuple/internal/AnonymousTupleType.java @@ -22,13 +22,14 @@ import org.hibernate.query.sqm.SqmPathSource; import org.hibernate.query.sqm.tree.domain.SqmDomainType; import org.hibernate.query.sqm.tree.domain.SqmPluralPersistentAttribute; +import org.hibernate.query.sqm.tree.select.SqmSelectQuery; +import org.hibernate.query.sqm.tree.select.SqmSelection; import org.hibernate.query.sqm.tuple.TupleType; import org.hibernate.query.SemanticException; import org.hibernate.query.sqm.SqmExpressible; import org.hibernate.query.sqm.tree.domain.SqmPath; import org.hibernate.query.sqm.tree.select.SqmSelectClause; import org.hibernate.query.sqm.tree.select.SqmSelectableNode; -import org.hibernate.query.sqm.tree.select.SqmSubQuery; import org.hibernate.spi.NavigablePath; import org.hibernate.sql.ast.spi.FromClauseAccess; import org.hibernate.sql.ast.spi.SqlSelection; @@ -55,26 +56,47 @@ public class AnonymousTupleType private final String[] componentNames; private final Map componentIndexMap; - public AnonymousTupleType(SqmSubQuery subQuery) { - this( extractSqmExpressibles( subQuery ) ); - } + public AnonymousTupleType(SqmSelectQuery selectQuery) { + final SqmSelectClause selectClause = selectQuery.getQueryPart() + .getFirstQuerySpec() + .getSelectClause(); - public AnonymousTupleType(SqmSelectableNode[] components) { - expressibles = new SqmBindableType[components.length]; - componentSourcePaths = new NavigablePath[components.length]; - for ( int i = 0; i < components.length; i++ ) { - expressibles[i] = components[i].getNodeType(); - if ( components[i] instanceof SqmPath path ) { - componentSourcePaths[i] = path.getNavigablePath(); + if ( selectClause == null || selectClause.getSelections().isEmpty() ) { + throw new IllegalArgumentException( "selectQuery has no selection items" ); + } + // todo: right now, we "snapshot" the state of the selectQuery when creating this type, but maybe we shouldn't? + // i.e. what if the selectQuery changes later on? Or should we somehow mark the selectQuery to signal, + // that changes to the select clause are invalid after a certain point? + + final List> selections = selectClause.getSelections(); + final List> selectableNodes = new ArrayList<>(); + final List aliases = new ArrayList<>(); + for ( SqmSelection selection : selections ) { + final boolean compound = selection.getSelectableNode().isCompoundSelection(); + selection.getSelectableNode().visitSubSelectableNodes( node -> { + selectableNodes.add( node ); + if ( compound ) { + aliases.add( node.getAlias() ); + } + } ); + if ( !compound ) { + // for compound selections we use the sub-selectable nodes aliases + aliases.add( selection.getAlias() ); } } - componentNames = new String[components.length]; + + expressibles = new SqmBindableType[selectableNodes.size()]; + componentSourcePaths = new NavigablePath[selectableNodes.size()]; + componentNames = new String[selectableNodes.size()]; //noinspection unchecked - javaTypeDescriptor = (JavaType) new ObjectArrayJavaType( getTypeDescriptors( components ) ); - componentIndexMap = linkedMapOfSize( components.length ); - for ( int i = 0; i < components.length; i++ ) { - final SqmSelectableNode component = components[i]; - final String alias = component.getAlias(); + javaTypeDescriptor = (JavaType) new ObjectArrayJavaType( getTypeDescriptors( selectableNodes ) ); + componentIndexMap = linkedMapOfSize( selectableNodes.size() ); + for ( int i = 0; i < selectableNodes.size(); i++ ) { + expressibles[i] = selectableNodes.get( i ).getNodeType(); + if ( selectableNodes.get( i ) instanceof SqmPath path ) { + componentSourcePaths[i] = path.getNavigablePath(); + } + String alias = aliases.get( i ); if ( alias == null ) { throw new SemanticException( "Select item at position " + (i+1) + " in select list has no alias" + " (aliases are required in CTEs and in subqueries occurring in from clause)" ); @@ -110,21 +132,10 @@ public String getTypeName() { return SqmDomainType.super.getTypeName(); } - private static SqmSelectableNode[] extractSqmExpressibles(SqmSubQuery subQuery) { - final SqmSelectClause selectClause = subQuery.getQuerySpec().getSelectClause(); - if ( selectClause == null || selectClause.getSelectionItems().isEmpty() ) { - throw new IllegalArgumentException( "subquery has no selection items" ); - } - // todo: right now, we "snapshot" the state of the subquery when creating this type, but maybe we shouldn't? - // i.e. what if the subquery changes later on? Or should we somehow mark the subquery to signal, - // that changes to the select clause are invalid after a certain point? - return selectClause.getSelectionItems().toArray( SqmSelectableNode[]::new ); - } - - private static JavaType[] getTypeDescriptors(SqmSelectableNode[] components) { - final JavaType[] typeDescriptors = new JavaType[components.length]; - for ( int i = 0; i < components.length; i++ ) { - typeDescriptors[i] = components[i].getExpressible().getExpressibleJavaType(); + private static JavaType[] getTypeDescriptors(List> components) { + final JavaType[] typeDescriptors = new JavaType[components.size()]; + for ( int i = 0; i < components.size(); i++ ) { + typeDescriptors[i] = components.get( i ).getExpressible().getExpressibleJavaType(); } return typeDescriptors; } diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/subquery/MultipleIdenticalColumnsInSubqueryTest.java b/hibernate-core/src/test/java/org/hibernate/orm/test/subquery/MultipleIdenticalColumnsInSubqueryTest.java new file mode 100644 index 000000000000..23b3366bf8d4 --- /dev/null +++ b/hibernate-core/src/test/java/org/hibernate/orm/test/subquery/MultipleIdenticalColumnsInSubqueryTest.java @@ -0,0 +1,76 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * Copyright Red Hat Inc. and Hibernate Authors + */ +package org.hibernate.orm.test.subquery; + +import jakarta.persistence.Entity; +import jakarta.persistence.GeneratedValue; +import jakarta.persistence.Id; +import jakarta.persistence.Tuple; +import org.hibernate.testing.orm.junit.DomainModel; +import org.hibernate.testing.orm.junit.JiraKey; +import org.hibernate.testing.orm.junit.SessionFactory; +import org.hibernate.testing.orm.junit.SessionFactoryScope; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertEquals; + +@DomainModel(annotatedClasses = MultipleIdenticalColumnsInSubqueryTest.Something.class) +@SessionFactory +@JiraKey("HHH-19396") +class MultipleIdenticalColumnsInSubqueryTest { + + @BeforeEach + void init(SessionFactoryScope scope) { + scope.inTransaction( session -> session.persist( new Something() ) ); + } + + @AfterEach + void clean(SessionFactoryScope scope) { + scope.inTransaction( session -> session.createMutationQuery( "delete from Something" ).executeUpdate() ); + } + + @Test + @DisplayName("Temporary table with same column selected twice, deduplication should be turned off") + void CTE_with_same_column_selected_twice(SessionFactoryScope scope) { + var r = scope.fromSession( session -> + session.createSelectionQuery( + "WITH S0 AS (SELECT foo AS foo, foo AS bar FROM Something) SELECT foo AS foo FROM S0", + String.class ).getSingleResult() ); + assertEquals( "a", r ); + } + + @Test + @DisplayName("Subquery with same column selected twice, deduplication should be turned off") + void CTE_with_same_column_selected_twice_some_aliases_removed(SessionFactoryScope scope) { + var r = scope.fromSession( session -> + session.createSelectionQuery( + "SELECT foo AS foo FROM (SELECT foo AS foo, foo AS foo2 FROM Something)", + String.class ).getSingleResult() ); + assertEquals( "a", r ); + } + + @Test + @DisplayName("Simple query with same column selected twice, deduplication should be turned on") + void simple_query_with_same_column_selected_twice(SessionFactoryScope scope) { + var tuple = scope.fromSession( session -> + session.createSelectionQuery( + "SELECT foo AS foo, foo as bar FROM Something", + Tuple.class ).getSingleResult() ); + assertEquals( 2, tuple.getElements().size() ); + assertEquals( "a", tuple.get( "foo" ) ); + assertEquals( "a", tuple.get( "bar" ) ); + } + + @Entity(name = "Something") + static class Something { + @Id + @GeneratedValue + private Long id; + private String foo = "a"; + } +}