diff --git a/hibernate-core/src/main/java/org/hibernate/hql/internal/ast/tree/DotNode.java b/hibernate-core/src/main/java/org/hibernate/hql/internal/ast/tree/DotNode.java index c83370e8cd..90055d8540 100644 --- a/hibernate-core/src/main/java/org/hibernate/hql/internal/ast/tree/DotNode.java +++ b/hibernate-core/src/main/java/org/hibernate/hql/internal/ast/tree/DotNode.java @@ -38,6 +38,7 @@ import org.hibernate.internal.CoreMessageLogger; import org.hibernate.internal.util.StringHelper; import org.hibernate.persister.collection.QueryableCollection; import org.hibernate.persister.entity.EntityPersister; +import org.hibernate.persister.entity.Queryable; import org.hibernate.sql.JoinFragment; import org.hibernate.sql.JoinType; import org.hibernate.type.CollectionType; @@ -282,19 +283,36 @@ public class DotNode extends FromReferenceNode implements DisplayableNode, Selec String propName = getPath(); FromClause currentFromClause = getWalker().getCurrentFromClause(); - if ( getWalker().getStatementType() != SqlTokenTypes.SELECT && indexed && classAlias == null ) { - // should indicate that we are processing an INSERT/UPDATE/DELETE - // query with a subquery implied via a collection property - // function. Here, we need to use the table name itself as the - // qualification alias. - // TODO : verify this works for all databases... - // TODO : is this also the case in non-"indexed" scenarios? - String alias = getLhs().getFromElement().getQueryable().getTableName(); - columns = getFromElement().toColumns( alias, propertyPath, false, true ); + // determine whether we should use the table name or table alias to qualify the column names... + // we need to use the table-name when: + // 1) the top-level statement is not a SELECT + // 2) the LHS FromElement is *the* FromElement from the top-level statement + // + // there is a caveat here.. if the update/delete statement are "multi-table" we should continue to use + // the alias also, even if the FromElement is the root one... + // + // in all other cases, we should use the table alias + final FromElement lhsFromElement = getLhs().getFromElement(); + if ( getWalker().getStatementType() != SqlTokenTypes.SELECT ) { + if ( isFromElementUpdateOrDeleteRoot( lhsFromElement ) ) { + // at this point we know we have the 2 conditions above, + // lets see if we have the mentioned "multi-table" caveat... + boolean useAlias = false; + if ( getWalker().getStatementType() != SqlTokenTypes.INSERT ) { + final Queryable persister = lhsFromElement.getQueryable(); + if ( persister.isMultiTable() ) { + useAlias = true; + } + } + if ( ! useAlias ) { + final String lhsTableName = lhsFromElement.getQueryable().getTableName(); + columns = getFromElement().toColumns( lhsTableName, propertyPath, false, true ); + } + } } - //We do not look for an existing join on the same path, because - //it makes sense to join twice on the same collection role + // We do not look for an existing join on the same path, because + // it makes sense to join twice on the same collection role FromElementFactory factory = new FromElementFactory( currentFromClause, getLhs().getFromElement(), diff --git a/hibernate-core/src/main/java/org/hibernate/hql/internal/ast/tree/FromReferenceNode.java b/hibernate-core/src/main/java/org/hibernate/hql/internal/ast/tree/FromReferenceNode.java index 8f1cdc663f..9658c4d65d 100644 --- a/hibernate-core/src/main/java/org/hibernate/hql/internal/ast/tree/FromReferenceNode.java +++ b/hibernate-core/src/main/java/org/hibernate/hql/internal/ast/tree/FromReferenceNode.java @@ -27,6 +27,7 @@ import antlr.SemanticException; import antlr.collections.AST; import org.jboss.logging.Logger; +import org.hibernate.hql.internal.antlr.HqlSqlTokenTypes; import org.hibernate.internal.CoreMessageLogger; /** @@ -130,4 +131,15 @@ public abstract class FromReferenceNode extends AbstractSelectExpression return null; } + @SuppressWarnings("SimplifiableIfStatement") + protected boolean isFromElementUpdateOrDeleteRoot(FromElement element) { + if ( element.getFromClause().getParentFromClause() != null ) { + // its not even a root... + return false; + } + + return getWalker().getStatementType() == HqlSqlTokenTypes.DELETE + || getWalker().getStatementType() == HqlSqlTokenTypes.UPDATE; + } + } 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 ff79488f61..119838102b 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 @@ -150,42 +150,54 @@ public class IdentNode extends FromReferenceNode implements SelectExpression { private boolean resolveAsAlias() { // This is not actually a constant, but a reference to FROM element. - FromElement element = getWalker().getCurrentFromClause().getFromElement( getText() ); - if ( element != null ) { - setType( SqlTokenTypes.ALIAS_REF ); - setFromElement( element ); - String[] columnExpressions = element.getIdentityColumns(); - final boolean isInNonDistinctCount = getWalker().isInCount() && ! getWalker().isInCountDistinct(); - final boolean isCompositeValue = columnExpressions.length > 1; - if ( isCompositeValue ) { - if ( isInNonDistinctCount && ! getWalker().getSessionFactoryHelper().getFactory().getDialect().supportsTupleCounts() ) { - setText( columnExpressions[0] ); - } - else { - String joinedFragment = StringHelper.join( ", ", columnExpressions ); - // avoid wrapping in parenthesis (explicit tuple treatment) if possible due to varied support for - // tuple syntax across databases.. - final boolean shouldSkipWrappingInParenthesis = - getWalker().isInCount() - || getWalker().getCurrentTopLevelClauseType() == HqlSqlTokenTypes.ORDER - || getWalker().getCurrentTopLevelClauseType() == HqlSqlTokenTypes.GROUP; - if ( ! shouldSkipWrappingInParenthesis ) { - joinedFragment = "(" + joinedFragment + ")"; - } - setText( joinedFragment ); - } - return true; - } - else if ( columnExpressions.length > 0 ) { - setText( columnExpressions[0] ); - return true; + final FromElement element = getWalker().getCurrentFromClause().getFromElement( getText() ); + if ( element == null ) { + return false; + } + + setType( SqlTokenTypes.ALIAS_REF ); + setFromElement( element ); + + String[] columnExpressions = element.getIdentityColumns(); + + // determine whether to apply qualification (table alias) to the column(s)... + if ( ! isFromElementUpdateOrDeleteRoot( element ) ) { + if ( StringHelper.isNotEmpty( element.getTableAlias() ) ) { + // apparently we also need to check that they are not already qualified. Ugh! + columnExpressions = StringHelper.qualifyIfNot( element.getTableAlias(), columnExpressions ); } } + + final boolean isInNonDistinctCount = getWalker().isInCount() && ! getWalker().isInCountDistinct(); + final boolean isCompositeValue = columnExpressions.length > 1; + if ( isCompositeValue ) { + if ( isInNonDistinctCount && ! getWalker().getSessionFactoryHelper().getFactory().getDialect().supportsTupleCounts() ) { + setText( columnExpressions[0] ); + } + else { + String joinedFragment = StringHelper.join( ", ", columnExpressions ); + // avoid wrapping in parenthesis (explicit tuple treatment) if possible due to varied support for + // tuple syntax across databases.. + final boolean shouldSkipWrappingInParenthesis = + getWalker().isInCount() + || getWalker().getCurrentTopLevelClauseType() == HqlSqlTokenTypes.ORDER + || getWalker().getCurrentTopLevelClauseType() == HqlSqlTokenTypes.GROUP; + if ( ! shouldSkipWrappingInParenthesis ) { + joinedFragment = "(" + joinedFragment + ")"; + } + setText( joinedFragment ); + } + return true; + } + else if ( columnExpressions.length > 0 ) { + setText( columnExpressions[0] ); + return true; + } + return false; } - private Type getNakedPropertyType(FromElement fromElement) - { + private Type getNakedPropertyType(FromElement fromElement) { if (fromElement == null) { return null; } diff --git a/hibernate-core/src/main/java/org/hibernate/internal/util/StringHelper.java b/hibernate-core/src/main/java/org/hibernate/internal/util/StringHelper.java index bf5107a635..f5711ef8d6 100644 --- a/hibernate-core/src/main/java/org/hibernate/internal/util/StringHelper.java +++ b/hibernate-core/src/main/java/org/hibernate/internal/util/StringHelper.java @@ -460,7 +460,9 @@ public final class StringHelper { } public static String[] qualify(String prefix, String[] names) { - if ( prefix == null ) return names; + if ( prefix == null ) { + return names; + } int len = names.length; String[] qualified = new String[len]; for ( int i = 0; i < len; i++ ) { @@ -468,6 +470,24 @@ public final class StringHelper { } return qualified; } + + public static String[] qualifyIfNot(String prefix, String[] names) { + if ( prefix == null ) { + return names; + } + int len = names.length; + String[] qualified = new String[len]; + for ( int i = 0; i < len; i++ ) { + if ( names[i].indexOf( '.' ) < 0 ) { + qualified[i] = qualify( prefix, names[i] ); + } + else { + qualified[i] = names[i]; + } + } + return qualified; + } + public static int firstIndexOfChar(String sqlString, BitSet keys, int startindex) { for ( int i = startindex, size = sqlString.length(); i < size; i++ ) { if ( keys.get( sqlString.charAt( i ) ) ) { diff --git a/hibernate-core/src/test/java/org/hibernate/test/annotations/query/QueryAndSQLTest.java b/hibernate-core/src/test/java/org/hibernate/test/annotations/query/QueryAndSQLTest.java index 4662baa6ed..7f358a4b12 100644 --- a/hibernate-core/src/test/java/org/hibernate/test/annotations/query/QueryAndSQLTest.java +++ b/hibernate-core/src/test/java/org/hibernate/test/annotations/query/QueryAndSQLTest.java @@ -452,21 +452,6 @@ public class QueryAndSQLTest extends BaseCoreFunctionalTestCase { tx.rollback(); s.close(); } - - @Test - @TestForIssue( jiraKey = "HHH-8318" ) - @FailureExpected( jiraKey = "HHH-8318" ) - public void testDeleteMemberOf() { - Session s = openSession(); - s.getTransaction().begin(); - s.createQuery( - "delete Attrvalue aval where aval.id in ( " - + "select val2.id from Employee e, Employeegroup eg, Attrset aset, Attrvalue val2 " - + "where eg.id = e.employeegroup.id " + "and aset.id = e.attrset.id " - + "and val2 member of aset.attrvalues)" ).executeUpdate(); - s.getTransaction().commit(); - s.close(); - } @Override protected Class[] getAnnotatedClasses() { diff --git a/hibernate-core/src/test/java/org/hibernate/test/hql/DeleteWhereMemberOfTest.java b/hibernate-core/src/test/java/org/hibernate/test/hql/DeleteWhereMemberOfTest.java new file mode 100644 index 0000000000..a2c9aeabb7 --- /dev/null +++ b/hibernate-core/src/test/java/org/hibernate/test/hql/DeleteWhereMemberOfTest.java @@ -0,0 +1,66 @@ +/* + * Hibernate, Relational Persistence for Idiomatic Java + * + * Copyright (c) 2013, Red Hat Inc. or third-party contributors as + * indicated by the @author tags or express copyright attribution + * statements applied by the authors. All third-party contributions are + * distributed under license by Red Hat Inc. + * + * This copyrighted material is made available to anyone wishing to use, modify, + * copy, or redistribute it subject to the terms and conditions of the GNU + * Lesser General Public License, as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY + * or FITNESS FOR A PARTICULAR PURPOSE. See the GNU Lesser General Public License + * for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this distribution; if not, write to: + * Free Software Foundation, Inc. + * 51 Franklin Street, Fifth Floor + * Boston, MA 02110-1301 USA + */ +package org.hibernate.test.hql; + +import org.hibernate.Session; + +import org.junit.Test; + +import org.hibernate.testing.TestForIssue; +import org.hibernate.testing.junit4.BaseCoreFunctionalTestCase; +import org.hibernate.test.annotations.query.Attrset; +import org.hibernate.test.annotations.query.Attrvalue; +import org.hibernate.test.annotations.query.Employee; +import org.hibernate.test.annotations.query.Employeegroup; + +/** + * @author Steve Ebersole + */ +public class DeleteWhereMemberOfTest extends BaseCoreFunctionalTestCase { + + @Override + protected Class[] getAnnotatedClasses() { + return new Class[] { + Attrset.class, + Attrvalue.class, + Employee.class, + Employeegroup.class + }; + } + + @Test + @TestForIssue( jiraKey = "HHH-8318" ) +// @FailureExpected( jiraKey = "HHH-8318" ) + public void testDeleteMemberOf() { + final String qry = "delete Attrvalue aval where aval.id in ( " + + "select val2.id from Employee e, Employeegroup eg, Attrset aset, Attrvalue val2 " + + "where eg.id = e.employeegroup.id " + "and aset.id = e.attrset.id " + + "and val2 member of aset.attrvalues)"; + Session s = openSession(); + s.getTransaction().begin(); + s.createQuery( qry ).executeUpdate(); + s.getTransaction().commit(); + s.close(); + } +}