HHH-16 - Explicit joins on unrelated classes

HHH-7321 - HQL: Combining a CROSS JOIN with a LEFT JOIN which requires a  WITH clause triggers an exception.
This commit is contained in:
Steve Ebersole 2016-02-08 15:52:42 -06:00
parent 5eb4f1c834
commit f4b5f2c649
11 changed files with 641 additions and 53 deletions

View File

@ -38,6 +38,7 @@ tokens
FROM_FRAGMENT; // A fragment of SQL that represents a table reference in a FROM clause.
IMPLIED_FROM; // An implied FROM element.
JOIN_FRAGMENT; // A JOIN fragment.
ENTITY_JOIN; // An "ad-hoc" join to an entity
SELECT_CLAUSE;
LEFT_OUTER;
RIGHT_OUTER;
@ -260,6 +261,8 @@ tokens
protected void processMapComponentReference(AST node) throws SemanticException { }
protected void validateMapPropertyExpression(AST node) throws SemanticException { }
protected void finishFromClause (AST fromClause) throws SemanticException { }
}
// The main statement rule.
@ -456,6 +459,7 @@ fromClause {
prepareFromClauseInputTree(#fromClause_in);
}
: #(f:FROM { pushFromClause(#fromClause,f); handleClauseStart( FROM ); } fromElementList ) {
finishFromClause( #f );
handleClauseEnd();
}
;

View File

@ -294,6 +294,7 @@ fromTable
// Write the table node (from fragment) and all the join fragments associated with it.
: #( a:FROM_FRAGMENT { out(a); } (tableJoin [ a ])* { fromFragmentSeparator(a); } )
| #( b:JOIN_FRAGMENT { out(b); } (tableJoin [ b ])* { fromFragmentSeparator(b); } )
| #( e:ENTITY_JOIN { out(e); } )
;
tableJoin [ AST parent ]

View File

@ -35,6 +35,7 @@ public final class QuerySplitter {
BEFORE_CLASS_TOKENS.add( "update" );
//beforeClassTokens.add("new"); DEFINITELY DON'T HAVE THIS!!
BEFORE_CLASS_TOKENS.add( "," );
BEFORE_CLASS_TOKENS.add( "join" );
NOT_AFTER_CLASS_TOKENS.add( "in" );
//notAfterClassTokens.add(",");
NOT_AFTER_CLASS_TOKENS.add( "from" );

View File

@ -18,7 +18,6 @@ import java.util.List;
import java.util.Map;
import java.util.Set;
import org.hibernate.HibernateException;
import org.hibernate.QueryException;
import org.hibernate.engine.internal.JoinSequence;
import org.hibernate.engine.internal.ParameterBinder;
@ -34,6 +33,7 @@ import org.hibernate.hql.internal.ast.tree.CollectionFunction;
import org.hibernate.hql.internal.ast.tree.ConstructorNode;
import org.hibernate.hql.internal.ast.tree.DeleteStatement;
import org.hibernate.hql.internal.ast.tree.DotNode;
import org.hibernate.hql.internal.ast.tree.EntityJoinFromElement;
import org.hibernate.hql.internal.ast.tree.FromClause;
import org.hibernate.hql.internal.ast.tree.FromElement;
import org.hibernate.hql.internal.ast.tree.FromElementFactory;
@ -74,10 +74,10 @@ import org.hibernate.param.ParameterSpecification;
import org.hibernate.param.PositionalParameterSpecification;
import org.hibernate.param.VersionTypeSeedParameterSpecification;
import org.hibernate.persister.collection.QueryableCollection;
import org.hibernate.persister.entity.EntityPersister;
import org.hibernate.persister.entity.Queryable;
import org.hibernate.sql.JoinType;
import org.hibernate.type.AssociationType;
import org.hibernate.type.ComponentType;
import org.hibernate.type.CompositeType;
import org.hibernate.type.DbTimestampType;
import org.hibernate.type.Type;
@ -363,50 +363,121 @@ public class HqlSqlWalker extends HqlSqlBaseWalker implements ErrorReporter, Par
if ( fetch && isSubQuery() ) {
throw new QueryException( "fetch not allowed in subquery from-elements" );
}
// The path AST should be a DotNode, and it should have been evaluated already.
if ( path.getType() != SqlTokenTypes.DOT ) {
throw new SemanticException( "Path expected for join!" );
}
DotNode dot = (DotNode) path;
JoinType hibernateJoinType = JoinProcessor.toHibernateJoinType( joinType );
dot.setJoinType( hibernateJoinType ); // Tell the dot node about the join type.
dot.setFetch( fetch );
// Generate an explicit join for the root dot node. The implied joins will be collected and passed up
// to the root dot node.
dot.resolve( true, false, alias == null ? null : alias.getText() );
final FromElement fromElement;
if ( dot.getDataType() != null && dot.getDataType().isComponentType() ) {
if ( dot.getDataType().isAnyType() ) {
throw new SemanticException( "An AnyType attribute cannot be join fetched" );
// ^^ because the discriminator (aka, the "meta columns") must be known to the SQL in
// a non-parameterized way.
}
FromElementFactory factory = new FromElementFactory(
getCurrentFromClause(),
dot.getLhs().getFromElement(),
dot.getPropertyPath(),
alias == null ? null : alias.getText(),
null,
false
// the incoming "path" can be either:
// 1) an implicit join path (join p.address.city)
// 2) an entity-join (join com.acme.User)
//
// so make the proper interpretation here...
final EntityPersister entityJoinReferencedPersister = resolveEntityJoinReferencedPersister( path );
if ( entityJoinReferencedPersister != null ) {
// `path` referenced an entity
final EntityJoinFromElement join = createEntityJoin(
entityJoinReferencedPersister,
alias,
joinType,
propertyFetch,
with
);
fromElement = factory.createComponentJoin( (CompositeType) dot.getDataType() );
( (FromReferenceNode) path ).setFromElement( join );
}
else {
fromElement = dot.getImpliedJoin();
fromElement.setAllPropertyFetch( propertyFetch != null );
if ( path.getType() != SqlTokenTypes.DOT ) {
throw new SemanticException( "Path expected for join!" );
}
if ( with != null ) {
if ( fetch ) {
throw new SemanticException( "with-clause not allowed on fetched associations; use filters" );
DotNode dot = (DotNode) path;
JoinType hibernateJoinType = JoinProcessor.toHibernateJoinType( joinType );
dot.setJoinType( hibernateJoinType ); // Tell the dot node about the join type.
dot.setFetch( fetch );
// Generate an explicit join for the root dot node. The implied joins will be collected and passed up
// to the root dot node.
dot.resolve( true, false, alias == null ? null : alias.getText() );
final FromElement fromElement;
if ( dot.getDataType() != null && dot.getDataType().isComponentType() ) {
if ( dot.getDataType().isAnyType() ) {
throw new SemanticException( "An AnyType attribute cannot be join fetched" );
// ^^ because the discriminator (aka, the "meta columns") must be known to the SQL in
// a non-parameterized way.
}
handleWithFragment( fromElement, with );
FromElementFactory factory = new FromElementFactory(
getCurrentFromClause(),
dot.getLhs().getFromElement(),
dot.getPropertyPath(),
alias == null ? null : alias.getText(),
null,
false
);
fromElement = factory.createComponentJoin( (CompositeType) dot.getDataType() );
}
else {
fromElement = dot.getImpliedJoin();
fromElement.setAllPropertyFetch( propertyFetch != null );
if ( with != null ) {
if ( fetch ) {
throw new SemanticException( "with-clause not allowed on fetched associations; use filters" );
}
handleWithFragment( fromElement, with );
}
}
if ( LOG.isDebugEnabled() ) {
LOG.debug(
"createFromJoinElement() : "
+ getASTPrinter().showAsString( fromElement, "-- join tree --" )
);
}
}
}
if ( LOG.isDebugEnabled() ) {
LOG.debug( "createFromJoinElement() : " + getASTPrinter().showAsString( fromElement, "-- join tree --" ) );
private EntityPersister resolveEntityJoinReferencedPersister(AST path) {
if ( path.getType() == IDENT ) {
final IdentNode pathIdentNode = (IdentNode) path;
String name = path.getText();
if ( name == null ) {
name = pathIdentNode.getOriginalText();
}
return sessionFactoryHelper.findEntityPersisterByName( name );
}
else if ( path.getType() == DOT ) {
final String pathText = ASTUtil.getPathText( path );
return sessionFactoryHelper.findEntityPersisterByName( pathText );
}
return null;
}
@Override
protected void finishFromClause(AST fromClause) throws SemanticException {
( (FromClause) fromClause ).finishInit();
}
private EntityJoinFromElement createEntityJoin(
EntityPersister entityPersister,
AST aliasNode,
int joinType,
AST propertyFetch,
AST with) throws SemanticException {
final String alias = aliasNode == null ? null : aliasNode.getText();
LOG.debugf( "Creating entity-join FromElement [%s -> %s]", alias, entityPersister.getEntityName() );
EntityJoinFromElement join = new EntityJoinFromElement(
this,
getCurrentFromClause(),
entityPersister,
JoinProcessor.toHibernateJoinType( joinType ),
propertyFetch != null,
alias
);
if ( with != null ) {
handleWithFragment( join, with );
}
return join;
}
private void handleWithFragment(FromElement fromElement, AST hqlWithNode) throws SemanticException {
@ -429,16 +500,16 @@ public class HqlSqlWalker extends HqlSqlBaseWalker implements ErrorReporter, Par
if ( withClauseJoinAlias == null ) {
withClauseJoinAlias = fromElement.getCollectionTableAlias();
}
else {
FromElement referencedFromElement = visitor.getReferencedFromElement();
if ( referencedFromElement != fromElement ) {
LOG.warnf(
"with-clause expressions do not reference the from-clause element to which the " +
"with-clause was associated. The query may not work as expected [%s]",
queryTranslatorImpl.getQueryString()
);
}
}
// else {
// FromElement referencedFromElement = visitor.getReferencedFromElement();
// if ( referencedFromElement != fromElement ) {
// LOG.warnf(
// "with-clause expressions do not reference the from-clause element to which the " +
// "with-clause was associated. The query may not work as expected [%s]",
// queryTranslatorImpl.getQueryString()
// );
// }
// }
SqlGenerator sql = new SqlGenerator( getSessionFactoryHelper().getFactory() );
sql.whereExpr( hqlSqlWithNode.getFirstChild() );
@ -483,9 +554,9 @@ public class HqlSqlWalker extends HqlSqlBaseWalker implements ErrorReporter, Par
DotNode dotNode = (DotNode) node;
FromElement fromElement = dotNode.getFromElement();
if ( referencedFromElement != null ) {
if ( fromElement != referencedFromElement ) {
throw new HibernateException( "with-clause referenced two different from-clause elements" );
}
// if ( fromElement != referencedFromElement ) {
// throw new HibernateException( "with-clause referenced two different from-clause elements" );
// }
}
else {
referencedFromElement = fromElement;

View File

@ -356,7 +356,10 @@ public class SqlGenerator extends SqlGeneratorBase implements ErrorReporter {
return;
}
if ( right.getRealOrigin() == left ||
if ( right.getType() == ENTITY_JOIN ) {
out( " " );
}
else if ( right.getRealOrigin() == left ||
( right.getRealOrigin() != null && right.getRealOrigin() == left.getRealOrigin() ) ) {
// right represents a joins originating from left; or
// both right and left reprersent joins originating from the same FromElement

View File

@ -0,0 +1,236 @@
/*
* 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 java.util.Collections;
import java.util.Map;
import org.hibernate.AssertionFailure;
import org.hibernate.MappingException;
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.internal.util.StringHelper;
import org.hibernate.persister.entity.EntityPersister;
import org.hibernate.persister.entity.Joinable;
import org.hibernate.persister.entity.Queryable;
import org.hibernate.sql.JoinFragment;
import org.hibernate.sql.JoinType;
import org.hibernate.type.EntityType;
/**
* @author Steve Ebersole
*/
public class EntityJoinFromElement extends FromElement {
public EntityJoinFromElement(
HqlSqlWalker walker,
FromClause fromClause,
EntityPersister entityPersister,
JoinType joinType,
boolean fetchProperties,
String alias) {
initialize( walker );
final String tableName = ( (Joinable) entityPersister ).getTableName();
final String tableAlias = fromClause.getAliasGenerator().createName( entityPersister.getEntityName() );
final EntityType entityType = (EntityType) ( (Queryable) entityPersister ).getType();
initializeEntity(
fromClause,
entityPersister.getEntityName(),
entityPersister,
entityType,
alias,
tableAlias
);
EntityJoinJoinSequenceImpl joinSequence = new EntityJoinJoinSequenceImpl(
getSessionFactoryHelper().getFactory(),
entityType,
tableName,
tableAlias,
joinType
);
setJoinSequence( joinSequence );
setAllPropertyFetch( fetchProperties );
// Add to the query spaces.
fromClause.getWalker().addQuerySpaces( entityPersister.getQuerySpaces() );
setType( HqlSqlTokenTypes.ENTITY_JOIN );
// setType( HqlSqlTokenTypes.FROM_FRAGMENT );
setText( tableName );
}
@Override
public void setText(String s) {
super.setText( s );
}
private static class EntityJoinJoinSequenceImpl extends JoinSequence {
private final SessionFactoryImplementor factory;
private final String entityTableText;
private final String entityTableAlias;
private final EntityType entityType;
private final JoinType joinType;
public EntityJoinJoinSequenceImpl(
SessionFactoryImplementor factory,
EntityType entityType,
String entityTableText,
String entityTableAlias,
JoinType joinType) {
super( factory );
this.factory = factory;
this.entityType = entityType;
this.entityTableText = entityTableText;
this.entityTableAlias = entityTableAlias;
this.joinType = joinType;
setUseThetaStyle( false );
}
/**
* Ugh!
*/
@Override
public JoinFragment toJoinFragment(
Map enabledFilters,
boolean includeAllSubclassJoins,
String withClauseFragment,
String withClauseJoinAlias) throws MappingException {
final String joinString;
switch ( joinType ) {
case INNER_JOIN:
joinString = " inner join ";
break;
case LEFT_OUTER_JOIN:
joinString = " left outer join ";
break;
case RIGHT_OUTER_JOIN:
joinString = " right outer join ";
break;
case FULL_JOIN:
joinString = " full outer join ";
break;
default:
throw new AssertionFailure( "undefined join type" );
}
final StringBuilder buffer = new StringBuilder();
buffer.append( joinString )
.append( entityTableText )
.append( ' ' )
.append( entityTableAlias )
.append( " on " );
final String filters = entityType.getOnCondition(
entityTableAlias,
factory,
enabledFilters,
Collections.<String>emptySet()
);
buffer.append( filters );
if ( withClauseFragment != null ) {
if ( StringHelper.isNotEmpty( filters ) ) {
buffer.append( " and " );
}
buffer.append( withClauseFragment );
}
return new EntityJoinJoinFragment( buffer.toString() );
}
}
private static class EntityJoinJoinFragment extends JoinFragment {
private final String fragmentString;
public EntityJoinJoinFragment(String fragmentString) {
this.fragmentString = fragmentString;
}
@Override
public void addJoin(
String tableName,
String alias,
String[] fkColumns,
String[] pkColumns,
JoinType joinType) {
}
@Override
public void addJoin(
String tableName,
String alias,
String[] fkColumns,
String[] pkColumns,
JoinType joinType,
String on) {
}
@Override
public void addCrossJoin(String tableName, String alias) {
}
@Override
public void addJoins(String fromFragment, String whereFragment) {
}
@Override
public String toFromFragmentString() {
return fragmentString;
}
@Override
public String toWhereFragmentString() {
return null;
}
@Override
public void addCondition(String alias, String[] fkColumns, String[] pkColumns) {
}
@Override
public boolean addCondition(String condition) {
return false;
}
@Override
public JoinFragment copy() {
return null;
}
}
// ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
// n/a
@Override
protected void initializeComponentJoin(FromElementType elementType) {
}
@Override
public void initializeCollection(FromClause fromClause, String classAlias, String tableAlias) {
}
@Override
public String getCollectionSuffix() {
return null;
}
@Override
public void setCollectionSuffix(String suffix) {
}
}

View File

@ -6,6 +6,7 @@
*/
package org.hibernate.hql.internal.ast.tree;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.HashSet;
import java.util.LinkedList;
@ -60,6 +61,8 @@ public class FromClause extends HqlSqlWalkerNode implements HqlSqlTokenTypes, Di
*/
private List impliedElements = new LinkedList();
private List<EntityJoinFromElement> entityJoinFromElements;
/**
* Adds a new from element to the from node.
*
@ -88,6 +91,25 @@ public class FromClause extends HqlSqlWalkerNode implements HqlSqlTokenTypes, Di
if ( tableAlias != null ) {
fromElementByTableAlias.put( tableAlias, element );
}
if ( element instanceof EntityJoinFromElement ) {
if ( entityJoinFromElements == null ) {
entityJoinFromElements = new ArrayList<EntityJoinFromElement>();
}
entityJoinFromElements.add( (EntityJoinFromElement) element );
}
}
public void finishInit() {
if ( entityJoinFromElements == null ) {
return;
}
for ( EntityJoinFromElement entityJoinFromElement : entityJoinFromElements ) {
ASTUtil.appendChild( this, entityJoinFromElement );
}
entityJoinFromElements.clear();
}
void addDuplicateAlias(String alias, FromElement element) {

View File

@ -126,7 +126,7 @@ public class FromElement extends HqlSqlWalkerNode implements DisplayableNode, Pa
initialized = true;
}
private void doInitialize(
protected void doInitialize(
FromClause fromClause,
String tableAlias,
String className,
@ -597,7 +597,9 @@ public class FromElement extends HqlSqlWalkerNode implements DisplayableNode, Pa
}
public boolean isFromOrJoinFragment() {
return getType() == SqlTokenTypes.FROM_FRAGMENT || getType() == SqlTokenTypes.JOIN_FRAGMENT;
return getType() == SqlTokenTypes.FROM_FRAGMENT
|| getType() == SqlTokenTypes.JOIN_FRAGMENT
|| getType() == SqlTokenTypes.ENTITY_JOIN;
}
public boolean isAllPropertyFetch() {

View File

@ -305,6 +305,15 @@ public final class ASTUtil {
}
}
public static void appendChild(AST parent, AST child) {
if ( parent.getFirstChild() == null ) {
parent.setFirstChild( child );
}
else {
getLastChild( parent ).setNextSibling( child );
}
}
private static ASTArray createAstArray(
ASTFactory factory,
int size,

View File

@ -136,7 +136,7 @@ public class SessionFactoryHelper {
*
* @throws MappingException
*/
private EntityPersister findEntityPersisterByName(String name) throws MappingException {
public EntityPersister findEntityPersisterByName(String name) throws MappingException {
// First, try to get the persister using the given name directly.
try {
return sfi.getEntityPersister( name );

View File

@ -0,0 +1,239 @@
/*
* 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.test.hql;
import java.util.List;
import javax.persistence.Entity;
import javax.persistence.Id;
import javax.persistence.JoinColumn;
import javax.persistence.ManyToOne;
import javax.persistence.Table;
import org.hibernate.Session;
import org.hibernate.annotations.NaturalId;
import org.hibernate.testing.junit4.BaseNonConfigCoreFunctionalTestCase;
import org.junit.Test;
import static org.hamcrest.core.Is.is;
import static org.junit.Assert.assertThat;
/**
* @author Steve Ebersole
*/
public class EntityJoinTest extends BaseNonConfigCoreFunctionalTestCase {
@Override
protected Class[] getAnnotatedClasses() {
return new Class[] { FinancialRecord.class, User.class, Customer.class };
}
@Test
public void testEntityJoins() {
createTestData();
try {
// testInnerEntityJoins();
testOuterEntityJoins();
}
finally {
deleteTestData();
}
}
private void testInnerEntityJoins() {
Session session = openSession();
session.beginTransaction();
try {
List result = session.createQuery(
"select r.id, c.name, u.id, u.username " +
"from FinancialRecord r " +
" inner join r.customer c " +
" inner join User u on r.lastUpdateBy = u.username"
).list();
assertThat( result.size(), is( 1 ) );
// NOTE that this leads to not really valid SQL, although some databases might support it /
// result = session.createQuery(
// "select r.id, r.customer.name, u.id, u.username " +
// "from FinancialRecord r " +
// " inner join User u on r.lastUpdateBy = u.username"
// ).list();
// assertThat( result.size(), is( 1 ) );
}
finally {
session.getTransaction().commit();
session.close();
}
}
private void testOuterEntityJoins() {
Session session = openSession();
session.beginTransaction();
try {
List result = session.createQuery(
"select r.id, c.name, u.id, u.username " +
"from FinancialRecord r " +
" inner join r.customer c " +
" left join User u on r.lastUpdateBy = u.username"
).list();
assertThat( result.size(), is( 1 ) );
// NOTE that this leads to not really valid SQL, although some databases might support it /
// result = session.createQuery(
// "select r.id, r.customer.name, u.id, u.username " +
// "from FinancialRecord r " +
// " left join User u on r.lastUpdateBy = u.username"
// ).list();
// assertThat( result.size(), is( 1 ) );
}
finally {
session.getTransaction().commit();
session.close();
}
}
private void createTestData() {
Session session = openSession();
session.getTransaction().begin();
session.save( new User( 1, "steve") );
session.save( new User( 2, "jane") );
final Customer customer = new Customer( 1, "Acme" );
session.save( customer );
session.save( new FinancialRecord( 1, customer, "steve" ) );
session.getTransaction().commit();
session.close();
}
private void deleteTestData() {
Session session = openSession();
session.getTransaction().begin();
session.createQuery( "delete FinancialRecord" ).executeUpdate();
session.createQuery( "delete Customer" ).executeUpdate();
session.createQuery( "delete User" ).executeUpdate();
session.getTransaction().commit();
session.close();
}
@Entity(name = "Customer")
@Table(name = "customer")
public static class Customer {
private Integer id;
private String name;
public Customer() {
}
public Customer(Integer id, String name) {
this.id = id;
this.name = name;
}
@Id
public Integer getId() {
return id;
}
public void setId(Integer id) {
this.id = id;
}
public String getName() {
return name;
}
public void setName(String name) {
this.name = name;
}
}
@Entity(name = "FinancialRecord")
@Table(name = "financial_record")
public static class FinancialRecord {
private Integer id;
private Customer customer;
private String lastUpdateBy;
public FinancialRecord() {
}
public FinancialRecord(Integer id, Customer customer, String lastUpdateBy) {
this.id = id;
this.customer = customer;
this.lastUpdateBy = lastUpdateBy;
}
@Id
public Integer getId() {
return id;
}
public void setId(Integer id) {
this.id = id;
}
@ManyToOne
@JoinColumn
public Customer getCustomer() {
return customer;
}
public void setCustomer(Customer customer) {
this.customer = customer;
}
public String getLastUpdateBy() {
return lastUpdateBy;
}
public void setLastUpdateBy(String lastUpdateBy) {
this.lastUpdateBy = lastUpdateBy;
}
}
@Entity(name = "User")
@Table(name = "`user`")
public static class User {
private Integer id;
private String username;
public User() {
}
public User(Integer id, String username) {
this.id = id;
this.username = username;
}
@Id
public Integer getId() {
return id;
}
public void setId(Integer id) {
this.id = id;
}
@NaturalId
public String getUsername() {
return username;
}
public void setUsername(String username) {
this.username = username;
}
}
}