diff --git a/hibernate-core/src/main/java/org/hibernate/dialect/SQLServerDialect.java b/hibernate-core/src/main/java/org/hibernate/dialect/SQLServerDialect.java index 6d83a47dcc..1a517a6dd7 100644 --- a/hibernate-core/src/main/java/org/hibernate/dialect/SQLServerDialect.java +++ b/hibernate-core/src/main/java/org/hibernate/dialect/SQLServerDialect.java @@ -38,7 +38,10 @@ import org.hibernate.type.StandardBasicTypes; */ public class SQLServerDialect extends AbstractTransactSQLDialect { private static final String SELECT = "select"; - private static final String SELECT_DISTINCT = "select distinct"; + private static final String FROM = "from"; + private static final String DISTINCT = "distinct"; + + public SQLServerDialect() { registerColumnType( Types.VARBINARY, "image" ); registerColumnType( Types.VARBINARY, 8000, "varbinary($l)" ); @@ -77,10 +80,8 @@ public class SQLServerDialect extends AbstractTransactSQLDialect { *
 	 * WITH query AS (SELECT ROW_NUMBER() OVER (ORDER BY orderby) as __hibernate_row_nr__, original_query_without_orderby)
 	 * SELECT * FROM query WHERE __hibernate_row_nr__ BEETWIN offset AND offset + last
-	 * --ORDER BY __hibernate_row_nr__
 	 * 
* - * I don't think that the last order by clause is mandatory * * @param querySqlString The SQL statement to base the limit query off of. * @param offset Offset of the first row to be returned by the query (zero-based) @@ -96,38 +97,79 @@ public class SQLServerDialect extends AbstractTransactSQLDialect { .toString(); } - StringBuilder sb = new StringBuilder( querySqlString.trim() ); + StringBuilder sb = new StringBuilder( querySqlString.trim().toLowerCase() ); - String querySqlLowered = querySqlString.trim().toLowerCase(); - int orderByIndex = querySqlLowered.toLowerCase().indexOf( "order by" ); - String orderby = orderByIndex > 0 ? querySqlString.substring( orderByIndex ) : "ORDER BY CURRENT_TIMESTAMP"; + int orderByIndex = sb.indexOf( "order by" ); + CharSequence orderby = orderByIndex > 0 ? sb.subSequence( orderByIndex, sb.length() ) : "ORDER BY CURRENT_TIMESTAMP"; // Delete the order by clause at the end of the query if ( orderByIndex > 0 ) { sb.delete( orderByIndex, orderByIndex + orderby.length() ); } - // Find the end of the select statement - int selectIndex = querySqlLowered.trim().indexOf(SELECT_DISTINCT); - if (selectIndex != -1) { - selectIndex += SELECT_DISTINCT.length(); - } else { - selectIndex = querySqlLowered.trim().indexOf(SELECT); - if (selectIndex != -1) { - selectIndex += SELECT.length(); - } - } - - // Isert after the select statement the row_number() function: - sb.insert(selectIndex, " ROW_NUMBER() OVER (" + orderby + ") as __hibernate_row_nr__,"); - + // HHH-5715 bug fix + replaceDistinctWithGroupBy( sb ); + + insertRowNumberFunction( sb, orderby ); + //Wrap the query within a with statement: - sb.insert(0, "WITH query AS (").append(") SELECT * FROM query "); - sb.append("WHERE __hibernate_row_nr__ BETWEEN ").append(offset + 1).append(" AND ").append(limit); + sb.insert( 0, "WITH query AS (").append(") SELECT * FROM query " ); + sb.append( "WHERE __hibernate_row_nr__ BETWEEN " ).append(offset + 1).append( " AND " ).append( limit ); return sb.toString(); } + + /** + * Utility method that checks if the given sql query is a select distinct one and if so replaces the distinct + * select with an equivelant simple select with a group by clause. See {@link SQLServerDialectTestCase#testReplaceDistinctWithGroupBy()} + * + * @param an sql query + */ + protected static void replaceDistinctWithGroupBy(StringBuilder sql) { + int distinctIndex = sql.indexOf( DISTINCT ); + if (distinctIndex > 0) { + sql.delete(distinctIndex, distinctIndex + DISTINCT.length() + 1); + sql.append(" group by").append(getSelectFieldsWithoutAliases(sql)); + } + } + /** + * This utility method searches the given sql query for the fields of the select statement and + * returns them without the aliases. See {@link SQLServerDialectTestCase#testGetSelectFieldsWithoutAliases()} + * + * @param an sql query + * @return the fields of the select statement without their alias + */ + protected static CharSequence getSelectFieldsWithoutAliases(StringBuilder sql) { + String select = sql.substring( sql.indexOf(SELECT) + SELECT.length(), sql.indexOf(FROM)); + + // Strip the as clauses + return stripAliases( select ); + } + + /** + * Utility method that strips the aliases. See {@link SQLServerDialectTestCase#testStripAliases()} + * + * @param a string to replace the as statements + * @return a string without the as statements + */ + protected static String stripAliases(String str) { + return str.replaceAll( "\\sas[^,]+(,?)", "$1" ); + } + + /** + * Right after the select statement of a given query we must place the row_number function + * + * @param sql the initial sql query without the order by clause + * @param orderby the order by clause of the query + */ + protected static void insertRowNumberFunction(StringBuilder sql, CharSequence orderby) { + // Find the end of the select statement + int selectEndIndex = sql.indexOf( SELECT ) + SELECT.length(); + + // Isert after the select statement the row_number() function: + sql.insert( selectEndIndex, " ROW_NUMBER() OVER (" + orderby + ") as __hibernate_row_nr__," ); + } /** * Use insert table(...) values(...) select SCOPE_IDENTITY() diff --git a/hibernate-core/src/test/java/org/hibernate/dialect/SQLServerDialectTestCase.java b/hibernate-core/src/test/java/org/hibernate/dialect/SQLServerDialectTestCase.java new file mode 100644 index 0000000000..a14857ec1a --- /dev/null +++ b/hibernate-core/src/test/java/org/hibernate/dialect/SQLServerDialectTestCase.java @@ -0,0 +1,42 @@ +package org.hibernate.dialect; + +import junit.framework.TestCase; + +/** + * Unit test of the behavior of the SQLServerDialect utility methods + * + * @author Valotasion Yoryos + * + */ +public class SQLServerDialectTestCase extends TestCase { + + public void testStripAliases() { + String input = "some_field1 as f1, some_fild2 as f2, _field3 as f3 "; + + assertEquals( "some_field1, some_fild2, _field3", SQLServerDialect.stripAliases(input) ); + } + + public void testGetSelectFieldsWithoutAliases() { + StringBuilder input = new StringBuilder( "select some_field1 as f12, some_fild2 as f879, _field3 as _f24674_3 from...." ); + String output = SQLServerDialect.getSelectFieldsWithoutAliases( input ).toString(); + + assertEquals( " some_field1, some_fild2, _field3", output ); + } + + + public void testReplaceDistinctWithGroupBy() { + StringBuilder input = new StringBuilder( "select distinct f1, f2 as ff, f3 from table where f1 = 5" ); + SQLServerDialect.replaceDistinctWithGroupBy( input ); + + assertEquals( "select f1, f2 as ff, f3 from table where f1 = 5 group by f1, f2, f3 ", input.toString() ); + } + + + public void testGetLimitString() { + String input = "select distinct f1 as f53245 from table849752 order by f234, f67 desc"; + + SQLServerDialect sqlDialect = new SQLServerDialect(); + + assertEquals( "with query as (select row_number() over (order by f234, f67 desc) as __hibernate_row_nr__, f1 as f53245 from table849752 group by f1) select * from query where __hibernate_row_nr__ between 11 and 15", sqlDialect.getLimitString(input, 10, 15).toLowerCase() ); + } +} diff --git a/hibernate-core/src/test/java/org/hibernate/test/pagination/DistinctSelectTest.java b/hibernate-core/src/test/java/org/hibernate/test/pagination/DistinctSelectTest.java new file mode 100644 index 0000000000..94d337206b --- /dev/null +++ b/hibernate-core/src/test/java/org/hibernate/test/pagination/DistinctSelectTest.java @@ -0,0 +1,64 @@ +package org.hibernate.test.pagination; + +import java.util.ArrayList; +import java.util.List; +import java.util.UUID; + +import org.hibernate.Session; +import org.hibernate.Transaction; +import org.hibernate.dialect.SQLServerDialect; +import org.hibernate.testing.junit.functional.FunctionalTestCase; + +/** + * HHH-5715 bug test case: Dublicated entries when using select distinct with join and pagination. The bug has to do + * with new {@link SQLServerDialect} that uses row_number function for pagination + * + * @author Valotasios Yoryos + * + */ +public class DistinctSelectTest extends FunctionalTestCase { + private static final int NUM_OF_USERS = 30; + + public DistinctSelectTest(String string) { + super(string); + } + + public String[] getMappings() { + return new String[] { "pagination/EntryTag.hbm.xml" }; + } + + public void feedDatabase() { + List tags = new ArrayList(); + + Session s = openSession(); + Transaction t = s.beginTransaction(); + + for (int i = 0; i < 5; i++) { + Tag tag = new Tag("Tag: " + UUID.randomUUID().toString()); + tags.add(tag); + s.save(tag); + } + + for (int i = 0; i < NUM_OF_USERS; i++) { + Entry e = new Entry("Entry: " + UUID.randomUUID().toString()); + e.getTags().addAll(tags); + s.save(e); + } + t.commit(); + s.close(); + } + + public void testDistinctSelectWithJoin() { + feedDatabase(); + + Session s = openSession(); + + List entries = s.createQuery("select distinct e from Entry e join e.tags t where t.surrogate != null order by e.name").setFirstResult(10).setMaxResults(5).list(); + + // System.out.println(entries); + Entry firstEntry = entries.remove(0); + assertFalse("The list of entries should not contain dublicated Entry objects as we've done a distinct select", entries.contains(firstEntry)); + + s.close(); + } +} diff --git a/hibernate-core/src/test/java/org/hibernate/test/pagination/Entry.java b/hibernate-core/src/test/java/org/hibernate/test/pagination/Entry.java new file mode 100644 index 0000000000..d16da5aafc --- /dev/null +++ b/hibernate-core/src/test/java/org/hibernate/test/pagination/Entry.java @@ -0,0 +1,68 @@ +package org.hibernate.test.pagination; + +import java.util.HashSet; +import java.util.Set; + +public class Entry { + private int id; + private String name; + private Set tags = new HashSet(); + + public Entry() { + + } + + public Entry(String name) { + this.name = name; + } + + public int getId() { + return id; + } + + public void setId(int id) { + this.id = id; + } + + public String getName() { + return name; + } + + public void setName(String name) { + this.name = name; + } + + public Set getTags() { + return tags; + } + + public void setTags(Set tags) { + this.tags = tags; + } + + @Override + public int hashCode() { + final int prime = 31; + int result = 1; + result = prime * result + ((name == null) ? 0 : name.hashCode()); + return result; + } + + @Override + public boolean equals(Object obj) { + if (this == obj) return true; + if (obj == null) return false; + if (getClass() != obj.getClass()) return false; + Entry other = (Entry) obj; + if (name == null) { + if (other.name != null) return false; + } + else if (!name.equals(other.name)) return false; + return true; + } + + @Override + public String toString() { + return getName(); + } +} diff --git a/hibernate-core/src/test/java/org/hibernate/test/pagination/EntryTag.hbm.xml b/hibernate-core/src/test/java/org/hibernate/test/pagination/EntryTag.hbm.xml new file mode 100755 index 0000000000..f7df31bbc7 --- /dev/null +++ b/hibernate-core/src/test/java/org/hibernate/test/pagination/EntryTag.hbm.xml @@ -0,0 +1,26 @@ + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/hibernate-core/src/test/java/org/hibernate/test/pagination/Tag.java b/hibernate-core/src/test/java/org/hibernate/test/pagination/Tag.java new file mode 100644 index 0000000000..d5db0d6be3 --- /dev/null +++ b/hibernate-core/src/test/java/org/hibernate/test/pagination/Tag.java @@ -0,0 +1,31 @@ +package org.hibernate.test.pagination; + +public class Tag { + private int id; + private String surrogate; + + public Tag() { + + } + + public Tag(String surrogate) { + this.surrogate = surrogate; + } + + public int getId() { + return id; + } + + public void setId(int id) { + this.id = id; + } + + public String getSurrogate() { + return surrogate; + } + + public void setSurrogate(String surrogate) { + this.surrogate = surrogate; + } + +} diff --git a/hibernate-core/src/test/resources/hibernate.properties b/hibernate-core/src/test/resources/hibernate.properties index 23a0730c6f..869bb3fec5 100644 --- a/hibernate-core/src/test/resources/hibernate.properties +++ b/hibernate-core/src/test/resources/hibernate.properties @@ -21,10 +21,11 @@ # 51 Franklin Street, Fifth Floor # Boston, MA 02110-1301 USA # -hibernate.dialect org.hibernate.dialect.H2Dialect -hibernate.connection.driver_class org.h2.Driver -hibernate.connection.url jdbc:h2:mem:db1;DB_CLOSE_DELAY=-1;MVCC=TRUE -hibernate.connection.username sa +hibernate.dialect org.hibernate.dialect.SQLServer2008Dialect +hibernate.connection.driver_class net.sourceforge.jtds.jdbc.Driver +hibernate.connection.url jdbc:jtds:sqlserver://localhost/test +hibernate.connection.username tester +hibernate.connection.password tester hibernate.connection.pool_size 5