Skip to content

Commit

Permalink
HHH-8864 count distinct tuples for postgres and h2
Browse files Browse the repository at this point in the history
Conflicts:
	hibernate-core/src/main/java/org/hibernate/dialect/PostgreSQL81Dialect.java
	hibernate-core/src/main/java/org/hibernate/dialect/function/StandardAnsiSqlAggregationFunctions.java
  • Loading branch information
brmeyer committed Jan 22, 2014
1 parent 3d7494d commit c404d84
Show file tree
Hide file tree
Showing 7 changed files with 78 additions and 16 deletions.
Expand Up @@ -2317,6 +2317,15 @@ public boolean supportsTupleDistinctCounts() {
// 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.
Expand Down
Expand Up @@ -381,8 +381,8 @@ public boolean supportsLobValueChangePropogation() {
}

@Override
public boolean supportsTupleDistinctCounts() {
return false;
public boolean requiresParensForTupleDistinctCounts() {
return true;
}

@Override
Expand Down
Expand Up @@ -336,8 +336,9 @@ public String getCurrentTimestampSelectString() {
return "select now()";
}

public boolean supportsTupleDistinctCounts() {
return false;

public boolean requiresParensForTupleDistinctCounts() {
return true;
}

public String toBooleanValueString(boolean bool) {
Expand Down
Expand Up @@ -30,6 +30,7 @@

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;
Expand All @@ -55,15 +56,16 @@ public CountFunction() {
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
Expand All @@ -72,6 +74,7 @@ private String renderCountDistinct(List arguments) {
.append( itr.next() );
sep = ", ";
}
if (dialect.requiresParensForTupleDistinctCounts()) buffer.append(")");
return buffer.append( ")" ).toString();
}
}
Expand Down
Expand Up @@ -255,7 +255,10 @@ else if ( propertyType.isCollectionType() ) {
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 );
Expand Down
Expand Up @@ -30,6 +30,7 @@
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;
Expand Down Expand Up @@ -168,18 +169,23 @@ private boolean resolveAsAlias() {
}
}

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 {
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()
(isInDistinctCount && ! dialect.requiresParensForTupleDistinctCounts())
|| isInNonDistinctCount
|| getWalker().getCurrentTopLevelClauseType() == HqlSqlTokenTypes.ORDER
|| getWalker().getCurrentTopLevelClauseType() == HqlSqlTokenTypes.GROUP;
if ( ! shouldSkipWrappingInParenthesis ) {
Expand Down
Expand Up @@ -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
Expand Down Expand Up @@ -93,6 +94,45 @@ public void testDistinctCountOfEntityWithCompositeId() {
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
Expand Down

0 comments on commit c404d84

Please sign in to comment.