HHH-8318 - Problem determining qualifier to use for column names from HQL query parser in certain circumstances

This commit is contained in:
Steve Ebersole 2013-06-20 22:05:38 -05:00
parent 2c1605904d
commit 4ab3caa789
6 changed files with 171 additions and 58 deletions

View File

@ -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(),

View File

@ -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;
}
}

View File

@ -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;
}

View File

@ -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 ) ) ) {

View File

@ -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() {

View File

@ -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();
}
}