From d48ac0f0f7dc6f07764aafa63262a104349252e9 Mon Sep 17 00:00:00 2001 From: Christian Beikov Date: Mon, 19 Sep 2016 16:46:33 +0200 Subject: [PATCH] HHH-10229 - Select value from element collection results in wrong SQL being produced --- .../hql/internal/ast/tree/IdentNode.java | 60 ++++++++++++++++++- ...t2.java => MapJoinTestWithEmbeddable.java} | 40 +++++++++---- 2 files changed, 85 insertions(+), 15 deletions(-) rename hibernate-core/src/test/java/org/hibernate/jpa/test/criteria/mapjoin/{MapJoinTest2.java => MapJoinTestWithEmbeddable.java} (69%) diff --git a/hibernate-core/src/main/java/org/hibernate/hql/internal/ast/tree/IdentNode.java b/hibernate-core/src/main/java/org/hibernate/hql/internal/ast/tree/IdentNode.java index c96b851517..ace7a60da8 100644 --- a/hibernate-core/src/main/java/org/hibernate/hql/internal/ast/tree/IdentNode.java +++ b/hibernate-core/src/main/java/org/hibernate/hql/internal/ast/tree/IdentNode.java @@ -39,6 +39,11 @@ public class IdentNode extends FromReferenceNode implements SelectExpression { } private boolean nakedPropertyRef; + private String[] columns; + + public String[] getColumns() { + return columns; + } public void resolveIndex(AST parent) throws SemanticException { // An ident node can represent an index expression if the ident @@ -80,14 +85,58 @@ public class IdentNode extends FromReferenceNode implements SelectExpression { getWalker().addQuerySpaces(queryableCollection.getCollectionSpaces()); // Always add the collection's query spaces. } + protected String[] resolveColumns(QueryableCollection collectionPersister) { + final FromElement fromElement = getFromElement(); + return fromElement.toColumns( + fromElement.getCollectionTableAlias(), + "elements", // the JPA VALUE "qualifier" is the same concept as the HQL ELEMENTS function/property + getWalker().isInSelect() + ); + } + + private void initText(String[] columns) { + String text = StringHelper.join( ", ", columns ); + if ( columns.length > 1 && getWalker().isComparativeExpressionClause() ) { + text = "(" + text + ")"; + } + setText( text ); + } + public void resolve(boolean generateJoin, boolean implicitJoin, String classAlias, AST parent) { if (!isResolved()) { - if (getWalker().getCurrentFromClause().isFromElementAlias(getText())) { - if (resolveAsAlias()) { + if ( getWalker().getCurrentFromClause().isFromElementAlias( getText() ) ) { + FromElement fromElement = getWalker().getCurrentFromClause().getFromElement( getText() ); + if ( fromElement.getQueryableCollection() != null && fromElement.getQueryableCollection().getElementType().isComponentType() ) { + if ( getWalker().isInSelect() ) { + // This is a reference to an element collection + setFromElement( fromElement ); + super.setDataType( fromElement.getQueryableCollection().getElementType() ); + this.columns = resolveColumns( fromElement.getQueryableCollection() ); + initText( getColumns() ); + setFirstChild( null ); + // Don't resolve it + } + else { + resolveAsAlias(); + // Don't resolve it + } + } + else if ( resolveAsAlias() ) { setResolved(); // We represent a from-clause alias } } + else if ( + getColumns() != null + && ( getWalker().getAST() instanceof AbstractMapComponentNode || getWalker().getAST() instanceof IndexNode ) + && getWalker().getCurrentFromClause().isFromElementAlias( getOriginalText() ) + ) { + // We might have to revert our decision that this is naked element collection reference when we encounter it is embedded in a map function + setText( getOriginalText() ); + if ( resolveAsAlias() ) { + setResolved(); + } + } else if (parent != null && parent.getType() == SqlTokenTypes.DOT) { DotNode dot = (DotNode) parent; if (parent.getFirstChild() == this) { @@ -338,7 +387,12 @@ public class IdentNode extends FromReferenceNode implements SelectExpression { else { FromElement fe = getFromElement(); if (fe != null) { - setText(fe.renderScalarIdentifierSelect(i)); + if ( fe.getQueryableCollection() != null && fe.getQueryableCollection().getElementType().isComponentType() ) { + ColumnHelper.generateScalarColumns( this, getColumns(), i ); + } + else { + setText(fe.renderScalarIdentifierSelect(i)); + } } else { ColumnHelper.generateSingleScalarColumn(this, i); diff --git a/hibernate-core/src/test/java/org/hibernate/jpa/test/criteria/mapjoin/MapJoinTest2.java b/hibernate-core/src/test/java/org/hibernate/jpa/test/criteria/mapjoin/MapJoinTestWithEmbeddable.java similarity index 69% rename from hibernate-core/src/test/java/org/hibernate/jpa/test/criteria/mapjoin/MapJoinTest2.java rename to hibernate-core/src/test/java/org/hibernate/jpa/test/criteria/mapjoin/MapJoinTestWithEmbeddable.java index b405d21cd6..36ab3f51c0 100644 --- a/hibernate-core/src/test/java/org/hibernate/jpa/test/criteria/mapjoin/MapJoinTest2.java +++ b/hibernate-core/src/test/java/org/hibernate/jpa/test/criteria/mapjoin/MapJoinTestWithEmbeddable.java @@ -14,7 +14,6 @@ import javax.persistence.Column; import javax.persistence.ElementCollection; import javax.persistence.Embeddable; import javax.persistence.Entity; -import javax.persistence.EntityManager; import javax.persistence.EnumType; import javax.persistence.Enumerated; import javax.persistence.GeneratedValue; @@ -33,10 +32,12 @@ import org.hibernate.jpa.test.BaseEntityManagerFunctionalTestCase; import org.hibernate.testing.TestForIssue; import org.junit.Test; +import static org.hibernate.testing.transaction.TransactionUtil.doInJPA; + /** - * @author Steve Ebersole + * @author Christian Beikov */ -public class MapJoinTest2 extends BaseEntityManagerFunctionalTestCase { +public class MapJoinTestWithEmbeddable extends BaseEntityManagerFunctionalTestCase { @Override protected Class[] getAnnotatedClasses() { return new Class[] {Batch.class, Node.class, BatchNodeMetadata.class}; @@ -45,20 +46,35 @@ public class MapJoinTest2 extends BaseEntityManagerFunctionalTestCase { @Test @TestForIssue( jiraKey = "HHH-10455" ) public void testSelectingKeyOfMapJoin() { - EntityManager em = getOrCreateEntityManager(); + doInJPA( this::entityManagerFactory, entityManager -> { + CriteriaBuilder cb = entityManager.getCriteriaBuilder(); + CriteriaQuery query = cb.createQuery( Node.class ); + Root root = query.from( Batch.class ); - CriteriaBuilder cb = em.getCriteriaBuilder(); - CriteriaQuery query = cb.createQuery( Node.class ); - Root root = query.from( Batch.class ); + MapJoin nodes = (MapJoin) root.join( "batchNodeMetadata" ); - MapJoin nodes = (MapJoin) root.join( "batchNodeMetadata" ); + query.select( nodes.key() ); + query.where( cb.equal( root.get( "id" ), 1 ) ); - query.select( nodes.key() ); - query.where( cb.equal( root.get( "id" ), 1 ) ); + entityManager.createQuery( query ).getResultList(); + } ); + } - em.createQuery( query ).getResultList(); + @Test + @TestForIssue( jiraKey = "HHH-10229" ) + public void testSelectingValueOfMapJoin() { + doInJPA( this::entityManagerFactory, entityManager -> { + CriteriaBuilder cb = entityManager.getCriteriaBuilder(); + CriteriaQuery query = cb.createQuery( Node.class ); + Root root = query.from( Batch.class ); - em.close(); + MapJoin nodes = (MapJoin) root.join( "batchNodeMetadata" ); + + query.select( nodes ); + query.where( cb.equal( root.get( "id" ), 1 ) ); + + entityManager.createQuery( query ).getResultList(); + } ); } @Entity