diff --git a/hibernate-core/src/main/java/org/hibernate/dialect/Dialect.java b/hibernate-core/src/main/java/org/hibernate/dialect/Dialect.java index a516bb76ac..19cfd1ade6 100644 --- a/hibernate-core/src/main/java/org/hibernate/dialect/Dialect.java +++ b/hibernate-core/src/main/java/org/hibernate/dialect/Dialect.java @@ -2317,6 +2317,15 @@ public abstract class Dialect implements ConversionContext { // oddly most database in fact seem to, so true is the default. return true; } + + /** + * If {@link #supportsTupleDistinctCounts()} is true, does the Dialect require the tuple to be wrapped with parens? + * + * @return boolean + */ + public boolean requiresParensForTupleDistinctCounts() { + return false; + } /** * Return the limit that the underlying database places on the number elements in an {@code IN} predicate. diff --git a/hibernate-core/src/main/java/org/hibernate/dialect/H2Dialect.java b/hibernate-core/src/main/java/org/hibernate/dialect/H2Dialect.java index 1b740327fc..de04a84883 100644 --- a/hibernate-core/src/main/java/org/hibernate/dialect/H2Dialect.java +++ b/hibernate-core/src/main/java/org/hibernate/dialect/H2Dialect.java @@ -381,8 +381,8 @@ public class H2Dialect extends Dialect { } @Override - public boolean supportsTupleDistinctCounts() { - return false; + public boolean requiresParensForTupleDistinctCounts() { + return true; } @Override diff --git a/hibernate-core/src/main/java/org/hibernate/dialect/PostgreSQL81Dialect.java b/hibernate-core/src/main/java/org/hibernate/dialect/PostgreSQL81Dialect.java index 75451ea801..d009a1ec89 100644 --- a/hibernate-core/src/main/java/org/hibernate/dialect/PostgreSQL81Dialect.java +++ b/hibernate-core/src/main/java/org/hibernate/dialect/PostgreSQL81Dialect.java @@ -336,8 +336,9 @@ public class PostgreSQL81Dialect extends Dialect { return "select now()"; } - public boolean supportsTupleDistinctCounts() { - return false; + + public boolean requiresParensForTupleDistinctCounts() { + return true; } public String toBooleanValueString(boolean bool) { diff --git a/hibernate-core/src/main/java/org/hibernate/dialect/function/StandardAnsiSqlAggregationFunctions.java b/hibernate-core/src/main/java/org/hibernate/dialect/function/StandardAnsiSqlAggregationFunctions.java index 34712c321f..aa7d8572fc 100644 --- a/hibernate-core/src/main/java/org/hibernate/dialect/function/StandardAnsiSqlAggregationFunctions.java +++ b/hibernate-core/src/main/java/org/hibernate/dialect/function/StandardAnsiSqlAggregationFunctions.java @@ -30,6 +30,7 @@ import java.util.Map; import org.hibernate.MappingException; import org.hibernate.QueryException; +import org.hibernate.dialect.Dialect; import org.hibernate.engine.spi.Mapping; import org.hibernate.engine.spi.SessionFactoryImplementor; import org.hibernate.type.StandardBasicTypes; @@ -55,15 +56,16 @@ public class StandardAnsiSqlAggregationFunctions { public String render(Type firstArgumentType, List arguments, SessionFactoryImplementor factory) { if ( arguments.size() > 1 ) { if ( "distinct".equalsIgnoreCase( arguments.get( 0 ).toString() ) ) { - return renderCountDistinct( arguments ); + return renderCountDistinct( arguments, factory.getDialect() ); } } return super.render( firstArgumentType, arguments, factory ); } - private String renderCountDistinct(List arguments) { + private String renderCountDistinct(List arguments, Dialect dialect) { StringBuilder buffer = new StringBuilder(); buffer.append( "count(distinct " ); + if (dialect.requiresParensForTupleDistinctCounts()) buffer.append("("); String sep = ""; Iterator itr = arguments.iterator(); itr.next(); // intentionally skip first @@ -72,6 +74,7 @@ public class StandardAnsiSqlAggregationFunctions { .append( itr.next() ); sep = ", "; } + if (dialect.requiresParensForTupleDistinctCounts()) buffer.append(")"); return buffer.append( ")" ).toString(); } } 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 f248e5c02a..34c40dfa60 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 @@ -255,7 +255,10 @@ public class DotNode extends FromReferenceNode implements DisplayableNode, Selec private void initText() { String[] cols = getColumns(); String text = StringHelper.join( ", ", cols ); - if ( cols.length > 1 && getWalker().isComparativeExpressionClause() ) { + boolean countDistinct = getWalker().isInCountDistinct() + && getWalker().getSessionFactoryHelper().getFactory().getDialect().requiresParensForTupleDistinctCounts(); + if ( cols.length > 1 && + ( getWalker().isComparativeExpressionClause() || countDistinct ) ) { text = "(" + text + ")"; } setText( text ); 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 119838102b..556de9a8b6 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 @@ -30,6 +30,7 @@ import antlr.SemanticException; import antlr.collections.AST; import org.hibernate.QueryException; +import org.hibernate.dialect.Dialect; import org.hibernate.dialect.function.SQLFunction; import org.hibernate.hql.internal.antlr.HqlSqlTokenTypes; import org.hibernate.hql.internal.antlr.SqlTokenTypes; @@ -168,10 +169,14 @@ public class IdentNode extends FromReferenceNode implements SelectExpression { } } - final boolean isInNonDistinctCount = getWalker().isInCount() && ! getWalker().isInCountDistinct(); + final Dialect dialect = getWalker().getSessionFactoryHelper().getFactory().getDialect(); + final boolean isInCount = getWalker().isInCount(); + final boolean isInDistinctCount = isInCount && getWalker().isInCountDistinct(); + final boolean isInNonDistinctCount = isInCount && ! getWalker().isInCountDistinct(); final boolean isCompositeValue = columnExpressions.length > 1; if ( isCompositeValue ) { - if ( isInNonDistinctCount && ! getWalker().getSessionFactoryHelper().getFactory().getDialect().supportsTupleCounts() ) { + if ( isInNonDistinctCount && ! dialect.supportsTupleCounts() ) { + // TODO: #supportsTupleCounts currently false for all Dialects -- could this be cleaned up? setText( columnExpressions[0] ); } else { @@ -179,7 +184,8 @@ public class IdentNode extends FromReferenceNode implements SelectExpression { // avoid wrapping in parenthesis (explicit tuple treatment) if possible due to varied support for // tuple syntax across databases.. final boolean shouldSkipWrappingInParenthesis = - getWalker().isInCount() + (isInDistinctCount && ! dialect.requiresParensForTupleDistinctCounts()) + || isInNonDistinctCount || getWalker().getCurrentTopLevelClauseType() == HqlSqlTokenTypes.ORDER || getWalker().getCurrentTopLevelClauseType() == HqlSqlTokenTypes.GROUP; if ( ! shouldSkipWrappingInParenthesis ) { diff --git a/hibernate-core/src/test/java/org/hibernate/test/cid/CompositeIdTest.java b/hibernate-core/src/test/java/org/hibernate/test/cid/CompositeIdTest.java index e8bf975463..2c5b10c417 100755 --- a/hibernate-core/src/test/java/org/hibernate/test/cid/CompositeIdTest.java +++ b/hibernate-core/src/test/java/org/hibernate/test/cid/CompositeIdTest.java @@ -23,24 +23,25 @@ */ package org.hibernate.test.cid; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; + import java.math.BigDecimal; import java.util.Calendar; import java.util.Collections; import java.util.Iterator; import java.util.List; -import org.junit.Test; - import org.hibernate.Hibernate; import org.hibernate.Session; import org.hibernate.Transaction; import org.hibernate.engine.query.spi.HQLQueryPlan; +import org.hibernate.exception.SQLGrammarException; import org.hibernate.hql.spi.QueryTranslator; import org.hibernate.testing.junit4.BaseCoreFunctionalTestCase; - -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertTrue; +import org.junit.Test; /** * @author Gavin King @@ -93,6 +94,45 @@ public class CompositeIdTest extends BaseCoreFunctionalTestCase { final String countExpressionFragment = generatedSql.substring( countExpressionListStart+6, countExpressionListEnd+1 ); assertTrue( countExpressionFragment.startsWith( "distinct" ) ); assertTrue( countExpressionFragment.contains( "," ) ); + + Session s = openSession(); + s.beginTransaction(); + Customer c = new Customer(); + c.setCustomerId( "1" ); + c.setAddress("123 somewhere"); + c.setName("Brett"); + Order o1 = new Order( c ); + o1.setOrderDate( Calendar.getInstance() ); + Order o2 = new Order( c ); + o2.setOrderDate( Calendar.getInstance() ); + s.persist( c ); + s.persist( o1 ); + s.persist( o2 ); + s.getTransaction().commit(); + s.clear(); + + s.beginTransaction(); + try { + long count = ( Long ) s.createQuery( "select count(distinct o) FROM Order o" ).uniqueResult(); + if ( ! getDialect().supportsTupleDistinctCounts() ) { + fail( "expected SQLGrammarException" ); + } + assertEquals( 2l, count ); + } + catch ( SQLGrammarException e ) { + if ( getDialect().supportsTupleDistinctCounts() ) { + throw e; + } + } + s.getTransaction().commit(); + s.close(); + + s = openSession(); + s.beginTransaction(); + s.createQuery("delete from Order").executeUpdate(); + s.createQuery("delete from Customer").executeUpdate(); + s.getTransaction().commit(); + s.close(); } @Test