HHH-10125 - KEY() function in HQL causes inaccurate SQL when map key is an entity;

HHH-10132 - ENTRY() function in HQL causes invalid SQL when map key is an entity

(cherry picked from commit 3cdc447654)
This commit is contained in:
Steve Ebersole 2015-09-29 23:30:46 -05:00
parent 5cbefff7a0
commit 634707cad9
8 changed files with 373 additions and 45 deletions

View File

@ -24,6 +24,7 @@ import antlr.collections.AST;
* @author Steve Ebersole
*/
public abstract class AbstractMapComponentNode extends FromReferenceNode implements HqlSqlTokenTypes {
private FromElement mapFromElement;
private String[] columns;
public FromReferenceNode getMapReference() {
@ -72,6 +73,8 @@ public abstract class AbstractMapComponentNode extends FromReferenceNode impleme
throw nonMap();
}
mapFromElement = sourceFromElement;
setFromElement( sourceFromElement );
setDataType( resolveType( sourceFromElement.getQueryableCollection() ) );
this.columns = resolveColumns( sourceFromElement.getQueryableCollection() );
@ -79,6 +82,10 @@ public abstract class AbstractMapComponentNode extends FromReferenceNode impleme
setFirstChild( null );
}
public FromElement getMapFromElement() {
return mapFromElement;
}
private boolean isAliasRef(FromReferenceNode mapReference) {
return ALIAS_REF == mapReference.getType();
}
@ -107,4 +114,19 @@ public abstract class AbstractMapComponentNode extends FromReferenceNode impleme
public void resolveIndex(AST parent) throws SemanticException {
throw new UnsupportedOperationException( expressionDescription() + " expression cannot be the source for an index operation" );
}
protected MapKeyEntityFromElement findOrAddMapKeyEntityFromElement(QueryableCollection collectionPersister) {
if ( !collectionPersister.getIndexType().isEntityType() ) {
return null;
}
for ( FromElement destination : getFromElement().getDestinations() ) {
if ( destination instanceof MapKeyEntityFromElement ) {
return (MapKeyEntityFromElement) destination;
}
}
return MapKeyEntityFromElement.buildKeyJoin( getFromElement() );
}
}

View File

@ -7,6 +7,7 @@
package org.hibernate.hql.internal.ast.tree;
import java.util.ArrayList;
import java.util.Collections;
import java.util.LinkedList;
import java.util.List;
import java.util.Set;
@ -64,7 +65,7 @@ public class FromElement extends HqlSqlWalkerNode implements DisplayableNode, Pa
private boolean initialized;
private FromElementType elementType;
private boolean useWhereFragment = true;
private List destinations = new LinkedList();
private List<FromElement> destinations;
private boolean manyToMany;
private String withClauseFragment;
private String withClauseJoinAlias;
@ -218,6 +219,14 @@ public class FromElement extends HqlSqlWalkerNode implements DisplayableNode, Pa
return elementType.renderPropertySelect( size, k, isAllPropertyFetch );
}
public String renderMapKeyPropertySelectFragment(int size, int k) {
return elementType.renderMapKeyPropertySelectFragment( size, k );
}
public String renderMapEntryPropertySelectFragment(int size, int k) {
return elementType.renderMapEntryPropertySelectFragment( size, k );
}
String renderCollectionSelectFragment(int size, int k) {
return elementType.renderCollectionSelectFragment( size, k );
}
@ -415,11 +424,19 @@ public class FromElement extends HqlSqlWalkerNode implements DisplayableNode, Pa
}
private void addDestination(FromElement fromElement) {
if ( destinations == null ) {
destinations = new LinkedList<FromElement>();
}
destinations.add( fromElement );
}
public List getDestinations() {
return destinations;
public List<FromElement> getDestinations() {
if ( destinations == null ) {
return Collections.emptyList();
}
else {
return destinations;
}
}
public FromElement getOrigin() {

View File

@ -14,10 +14,13 @@ import java.util.Set;
import org.hibernate.MappingException;
import org.hibernate.QueryException;
import org.hibernate.engine.internal.JoinSequence;
import org.hibernate.engine.spi.SessionFactoryImplementor;
import org.hibernate.hql.internal.CollectionProperties;
import org.hibernate.hql.internal.CollectionSubqueryFactory;
import org.hibernate.hql.internal.NameGenerator;
import org.hibernate.hql.internal.antlr.HqlSqlTokenTypes;
import org.hibernate.hql.internal.ast.HqlSqlWalker;
import org.hibernate.hql.internal.ast.util.SessionFactoryHelper;
import org.hibernate.internal.CoreLogging;
import org.hibernate.internal.CoreMessageLogger;
import org.hibernate.internal.log.DeprecationLogger;
@ -215,6 +218,46 @@ class FromElementType {
}
}
public String renderMapKeyPropertySelectFragment(int size, int k) {
if ( persister == null ) {
throw new IllegalStateException( "Unexpected state in call to renderMapKeyPropertySelectFragment" );
}
final String fragment = ( (Queryable) persister ).propertySelectFragment(
getTableAlias(),
getSuffix( size, k ),
false
);
return trimLeadingCommaAndSpaces( fragment );
// if ( queryableCollection == null
// || !Map.class.isAssignableFrom( queryableCollection.getCollectionType().getReturnedClass() ) ) {
// throw new IllegalStateException( "Illegal call to renderMapKeyPropertySelectFragment() when FromElement is not a Map" );
// }
//
// if ( !queryableCollection.getIndexType().isEntityType() ) {
// return null;
// }
//
// final HqlSqlWalker walker = fromElement.getWalker();
// final SessionFactoryHelper sfh = walker.getSessionFactoryHelper();
// final SessionFactoryImplementor sf = sfh.getFactory();
//
// final EntityType indexEntityType = (EntityType) queryableCollection.getIndexType();
// final EntityPersister indexEntityPersister = (EntityPersister) indexEntityType.getAssociatedJoinable( sf );
//
// final String fragment = ( (Queryable) indexEntityPersister ).propertySelectFragment(
// getTableAlias(),
// getSuffix( size, k ),
// false
// );
// return trimLeadingCommaAndSpaces( fragment );
}
public String renderMapEntryPropertySelectFragment(int size, int k) {
return null;
}
String renderCollectionSelectFragment(int size, int k) {
if ( queryableCollection == null ) {
return "";

View File

@ -95,13 +95,11 @@ public class MapEntryNode extends AbstractMapComponentNode implements Aggregated
AliasGenerator aliasGenerator = new LocalAliasGenerator( 0 );
appendSelectExpressions( collectionPersister.getIndexColumnNames(), selections, aliasGenerator );
Type keyType = collectionPersister.getIndexType();
if ( keyType.isAssociationType() ) {
EntityType entityType = (EntityType) keyType;
Queryable keyEntityPersister = (Queryable) sfi().getEntityPersister(
entityType.getAssociatedEntityName( sfi() )
);
if ( keyType.isEntityType() ) {
MapKeyEntityFromElement mapKeyEntityFromElement = findOrAddMapKeyEntityFromElement( collectionPersister );
Queryable keyEntityPersister = mapKeyEntityFromElement.getQueryable();
SelectFragment fragment = keyEntityPersister.propertySelectFragmentFragment(
collectionTableAlias(),
mapKeyEntityFromElement.getTableAlias(),
null,
false
);

View File

@ -0,0 +1,101 @@
/*
* Hibernate, Relational Persistence for Idiomatic Java
*
* License: GNU Lesser General Public License (LGPL), version 2.1 or later.
* See the lgpl.txt file in the root directory or <http://www.gnu.org/licenses/lgpl-2.1.html>.
*/
package org.hibernate.hql.internal.ast.tree;
import org.hibernate.engine.internal.JoinSequence;
import org.hibernate.engine.spi.SessionFactoryImplementor;
import org.hibernate.hql.internal.antlr.HqlSqlTokenTypes;
import org.hibernate.hql.internal.ast.HqlSqlWalker;
import org.hibernate.hql.internal.ast.util.SessionFactoryHelper;
import org.hibernate.persister.collection.QueryableCollection;
import org.hibernate.persister.entity.EntityPersister;
import org.hibernate.persister.entity.Joinable;
import org.hibernate.sql.JoinType;
import org.hibernate.type.EntityType;
import org.hibernate.type.Type;
/**
* @author Steve Ebersole
*/
public class MapKeyEntityFromElement extends FromElement {
private final boolean useThetaJoin;
public MapKeyEntityFromElement(boolean useThetaJoin) {
super();
this.useThetaJoin = useThetaJoin;
}
@Override
public boolean isImplied() {
return useThetaJoin;
}
@Override
public int getType() {
return useThetaJoin ? HqlSqlTokenTypes.FROM_FRAGMENT : HqlSqlTokenTypes.JOIN_FRAGMENT;
}
public static MapKeyEntityFromElement buildKeyJoin(FromElement collectionFromElement) {
final HqlSqlWalker walker = collectionFromElement.getWalker();
final SessionFactoryHelper sfh = walker.getSessionFactoryHelper();
final SessionFactoryImplementor sf = sfh.getFactory();
final QueryableCollection collectionPersister = collectionFromElement.getQueryableCollection();
final Type indexType = collectionPersister.getIndexType();
if ( indexType == null ) {
throw new IllegalArgumentException( "Given collection is not indexed" );
}
if ( !indexType.isEntityType() ) {
throw new IllegalArgumentException( "Given collection does not have an entity index" );
}
final EntityType indexEntityType = (EntityType) indexType;
final EntityPersister indexEntityPersister = (EntityPersister) indexEntityType.getAssociatedJoinable( sf );
final String rhsAlias = walker.getAliasGenerator().createName( indexEntityPersister.getEntityName() );
final boolean useThetaJoin = collectionFromElement.getJoinSequence().isThetaStyle();
MapKeyEntityFromElement join = new MapKeyEntityFromElement( useThetaJoin );
join.initialize( HqlSqlTokenTypes.JOIN_FRAGMENT, ( (Joinable) indexEntityPersister ).getTableName() );
join.initialize( collectionFromElement.getWalker() );
join.initializeEntity(
collectionFromElement.getFromClause(),
indexEntityPersister.getEntityName(),
indexEntityPersister,
indexEntityType,
"<map-key-join-" + collectionFromElement.getClassAlias() + ">",
rhsAlias
);
// String[] joinColumns = determineJoinColuns( collectionPersister, joinTableAlias );
// todo : assumes columns, no formulas
String[] joinColumns = collectionPersister.getIndexColumnNames( collectionFromElement.getCollectionTableAlias() );
JoinSequence joinSequence = sfh.createJoinSequence(
useThetaJoin,
indexEntityType,
rhsAlias,
// todo : ever a time when INNER is appropriate?
//JoinType.LEFT_OUTER_JOIN,
// needs to be an inner join because of how JoinSequence/JoinFragment work - ugh
JoinType.INNER_JOIN,
joinColumns
);
join.setJoinSequence( joinSequence );
join.setOrigin( collectionFromElement, collectionPersister.isManyToMany() );
join.setColumns( joinColumns );
join.setUseFromFragment( collectionFromElement.useFromFragment() );
join.setUseWhereFragment( collectionFromElement.useWhereFragment() );
walker.addQuerySpaces( indexEntityPersister.getQuerySpaces() );
return join;
}
}

View File

@ -15,6 +15,8 @@ import org.hibernate.type.Type;
* @author Steve Ebersole
*/
public class MapKeyNode extends AbstractMapComponentNode {
private MapKeyEntityFromElement mapKeyEntityFromElement;
@Override
protected String expressionDescription() {
return "key(*)";
@ -22,7 +24,12 @@ public class MapKeyNode extends AbstractMapComponentNode {
@Override
protected String[] resolveColumns(QueryableCollection collectionPersister) {
final FromElement fromElement = getFromElement();
this.mapKeyEntityFromElement = findOrAddMapKeyEntityFromElement( collectionPersister );
if ( mapKeyEntityFromElement != null ) {
setFromElement( mapKeyEntityFromElement );
}
final FromElement fromElement = getMapFromElement();
return fromElement.toColumns(
fromElement.getCollectionTableAlias(),
"index", // the JPA KEY "qualifier" is the same concept as the HQL INDEX function/property
@ -34,4 +41,8 @@ public class MapKeyNode extends AbstractMapComponentNode {
protected Type resolveType(QueryableCollection collectionPersister) {
return collectionPersister.getIndexType();
}
public MapKeyEntityFromElement getMapKeyEntityFromElement() {
return mapKeyEntityFromElement;
}
}

View File

@ -432,7 +432,7 @@ public class SelectClause extends SelectExpressionList {
if ( !selectExpressions[i].isScalar() ) {
FromElement fromElement = selectExpressions[i].getFromElement();
if ( fromElement != null ) {
renderNonScalarProperties( appender, fromElement, nonscalarSize, k );
renderNonScalarProperties( appender, selectExpressions[i], fromElement, nonscalarSize, k );
k++;
}
}
@ -446,13 +446,24 @@ public class SelectClause extends SelectExpressionList {
int j,
SelectExpression expr,
ASTAppender appender) {
String text = fromElement.renderIdentifierSelect( nonscalarSize, j );
if ( !fromElement.getFromClause().isSubQuery() ) {
if ( !scalarSelect && !getWalker().isShallowQuery() ) {
//TODO: is this a bit ugly?
// // todo : ugh this is all fugly code
// if ( expr instanceof MapKeyNode ) {
// // don't over-write node text
// }
// else if ( expr instanceof MapEntryNode ) {
// // don't over-write node text
// }
// else {
// String text = fromElement.renderIdentifierSelect( nonscalarSize, j );
// expr.setText( text );
// }
String text = fromElement.renderIdentifierSelect( nonscalarSize, j );
expr.setText( text );
}
else {
String text = fromElement.renderIdentifierSelect( nonscalarSize, j );
if (! alreadyRenderedIdentifiers.contains(text)) {
appender.append( SqlTokenTypes.SQL_TOKEN, text, false );
alreadyRenderedIdentifiers.add(text);
@ -461,21 +472,43 @@ public class SelectClause extends SelectExpressionList {
}
}
private void renderNonScalarProperties(ASTAppender appender, FromElement fromElement, int nonscalarSize, int k) {
String text = fromElement.renderPropertySelect( nonscalarSize, k );
appender.append( SqlTokenTypes.SQL_TOKEN, text, false );
if ( fromElement.getQueryableCollection() != null && fromElement.isFetch() ) {
text = fromElement.renderCollectionSelectFragment( nonscalarSize, k );
appender.append( SqlTokenTypes.SQL_TOKEN, text, false );
private void renderNonScalarProperties(
ASTAppender appender,
SelectExpression selectExpression,
FromElement fromElement,
int nonscalarSize,
int k) {
final String text;
if ( selectExpression instanceof MapKeyNode ) {
final MapKeyNode mapKeyNode = (MapKeyNode) selectExpression;
if ( mapKeyNode.getMapKeyEntityFromElement() != null ) {
text = mapKeyNode.getMapKeyEntityFromElement().renderMapKeyPropertySelectFragment( nonscalarSize, k );
}
else {
text = fromElement.renderPropertySelect( nonscalarSize, k );
}
}
else if ( selectExpression instanceof MapEntryNode ) {
text = fromElement.renderMapEntryPropertySelectFragment( nonscalarSize, k );
}
else {
text = fromElement.renderPropertySelect( nonscalarSize, k );
}
appender.append( SqlTokenTypes.SQL_TOKEN, text, false );
if ( fromElement.getQueryableCollection() != null && fromElement.isFetch() ) {
String subText1 = fromElement.renderCollectionSelectFragment( nonscalarSize, k );
appender.append( SqlTokenTypes.SQL_TOKEN, subText1, false );
}
// Look through the FromElement's children to find any collections of values that should be fetched...
ASTIterator iter = new ASTIterator( fromElement );
while ( iter.hasNext() ) {
FromElement child = (FromElement) iter.next();
ASTIterator itr = new ASTIterator( fromElement );
while ( itr.hasNext() ) {
FromElement child = (FromElement) itr.next();
if ( child.isCollectionOfValuesOrComponents() && child.isFetch() ) {
// Need a better way to define the suffixes here...
text = child.renderValueCollectionSelectFragment( nonscalarSize, nonscalarSize + k );
appender.append( SqlTokenTypes.SQL_TOKEN, text, false );
final String subText2 = child.renderValueCollectionSelectFragment( nonscalarSize, nonscalarSize + k );
appender.append( SqlTokenTypes.SQL_TOKEN, subText2, false );
}
}
}

View File

@ -7,30 +7,66 @@
package org.hibernate.test.hql;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import javax.persistence.Column;
import javax.persistence.ElementCollection;
import javax.persistence.Embeddable;
import javax.persistence.Entity;
import javax.persistence.EnumType;
import javax.persistence.Id;
import javax.persistence.JoinColumn;
import javax.persistence.MapKeyColumn;
import javax.persistence.MapKeyEnumerated;
import javax.persistence.JoinTable;
import javax.persistence.MapKeyJoinColumn;
import javax.persistence.OneToMany;
import javax.persistence.Table;
import org.hibernate.Session;
import org.hibernate.hql.internal.ast.ASTQueryTranslatorFactory;
import org.hibernate.testing.junit4.BaseNonConfigCoreFunctionalTestCase;
import org.junit.After;
import org.junit.Before;
import org.junit.Test;
import static org.hibernate.testing.junit4.ExtraAssertions.assertTyping;
import static org.junit.Assert.assertEquals;
/**
* Test originally written to help verify/diagnose HHH-10125
*
* @author Steve Ebersole
*/
public class MapFunctionExpressionsTest extends BaseNonConfigCoreFunctionalTestCase {
private final ASTQueryTranslatorFactory queryTranslatorFactory = new ASTQueryTranslatorFactory();
@Before
public void prepareTestData() {
Session s = openSession();
s.getTransaction().begin();
AddressType homeType = new AddressType( 1, "home" );
s.persist( homeType );
Address address = new Address( 1, "Main St.", "Somewhere, USA" );
s.persist( address );
Contact contact = new Contact( 1, "John" );
contact.addresses.put( homeType, address );
s.persist( contact );
s.getTransaction().commit();
s.close();
}
@After
public void cleanUpTestData() {
Session s = openSession();
s.getTransaction().begin();
s.delete( s.get( Contact.class, 1 ) );
s.delete( s.get( Address.class, 1 ) );
s.delete( s.get( AddressType.class, 1 ) );
s.getTransaction().commit();
s.close();
}
@Test
public void testMapKeyExpressionInWhere() {
@ -39,10 +75,17 @@ public class MapFunctionExpressionsTest extends BaseNonConfigCoreFunctionalTestC
Session s = openSession();
s.getTransaction().begin();
// JPA form
s.createQuery( "from Contact c join c.addresses a where key(a) = 'HOME'" ).list();
List contacts = s.createQuery( "select c from Contact c join c.addresses a where key(a) is not null" ).list();
assertEquals( 1, contacts.size() );
Contact contact = assertTyping( Contact.class, contacts.get( 0 ) );
// Hibernate additional form
s.createQuery( "from Contact c where key(c.addresses) = 'HOME'" ).list();
contacts = s.createQuery( "select c from Contact c where key(c.addresses) is not null" ).list();
assertEquals( 1, contacts.size() );
contact = assertTyping( Contact.class, contacts.get( 0 ) );
s.getTransaction().commit();
s.close();
}
@ -54,10 +97,17 @@ public class MapFunctionExpressionsTest extends BaseNonConfigCoreFunctionalTestC
Session s = openSession();
s.getTransaction().begin();
// JPA form
s.createQuery( "select key(a) from Contact c join c.addresses a" ).list();
List types = s.createQuery( "select key(a) from Contact c join c.addresses a" ).list();
assertEquals( 1, types.size() );
assertTyping( AddressType.class, types.get( 0 ) );
// Hibernate additional form
s.createQuery( "select key(c.addresses) from Contact c" ).list();
types = s.createQuery( "select key(c.addresses) from Contact c" ).list();
assertEquals( 1, types.size() );
assertTyping( AddressType.class, types.get( 0 ) );
s.getTransaction().commit();
s.close();
}
@ -66,21 +116,55 @@ public class MapFunctionExpressionsTest extends BaseNonConfigCoreFunctionalTestC
public void testMapValueExpressionInSelect() {
Session s = openSession();
s.getTransaction().begin();
s.createQuery( "select value(a) from Contact c join c.addresses a" ).list();
s.createQuery( "select value(c.addresses) from Contact c" ).list();
List addresses = s.createQuery( "select value(a) from Contact c join c.addresses a" ).list();
assertEquals( 1, addresses.size() );
assertTyping( Address.class, addresses.get( 0 ) );
addresses = s.createQuery( "select value(c.addresses) from Contact c" ).list();
assertEquals( 1, addresses.size() );
assertTyping( Address.class, addresses.get( 0 ) );
s.getTransaction().commit();
s.close();
}
@Test
public void testMapEntryExpressionInSelect() {
Session s = openSession();
s.getTransaction().begin();
List addresses = s.createQuery( "select entry(a) from Contact c join c.addresses a" ).list();
assertEquals( 1, addresses.size() );
assertTyping( Map.Entry.class, addresses.get( 0 ) );
addresses = s.createQuery( "select entry(c.addresses) from Contact c" ).list();
assertEquals( 1, addresses.size() );
assertTyping( Map.Entry.class, addresses.get( 0 ) );
s.getTransaction().commit();
s.close();
}
@Override
protected Class[] getAnnotatedClasses() {
return new Class[] { Address.class, Contact.class };
return new Class[] { Address.class, AddressType.class, Contact.class };
}
public static enum AddressType {
HOME,
WORK,
BUSINESS
@Entity(name = "AddressType")
@Table(name = "address_type")
public static class AddressType {
@Id
public Integer id;
String name;
public AddressType() {
}
public AddressType(Integer id, String name) {
this.id = id;
this.name = name;
}
}
@Entity(name = "Address")
@ -91,6 +175,15 @@ public class MapFunctionExpressionsTest extends BaseNonConfigCoreFunctionalTestC
public Integer id;
String street;
String city;
public Address() {
}
public Address(Integer id, String street, String city) {
this.id = id;
this.street = street;
this.city = city;
}
}
@Entity(name = "Contact")
@ -100,10 +193,20 @@ public class MapFunctionExpressionsTest extends BaseNonConfigCoreFunctionalTestC
public Integer id;
String name;
@OneToMany
@JoinColumn
@JoinTable(name = "contact_address")
@MapKeyJoinColumn(name="address_type_id", referencedColumnName="id")
// @JoinColumn
// @ElementCollection
@MapKeyEnumerated(EnumType.STRING)
@MapKeyColumn(name = "addr_type")
// @MapKeyEnumerated(EnumType.STRING)
// @MapKeyColumn(name = "addr_type")
Map<AddressType, Address> addresses = new HashMap<AddressType, Address>();
public Contact() {
}
public Contact(Integer id, String name) {
this.id = id;
this.name = name;
}
}
}