github/ebean
4
0
Fork 0
mirror of https://github.com/ebean-orm/ebean.git synced 2026-10-23 21:15:52 +00:00
Code Issues Packages Projects Releases Wiki Activity

#1746 - Wrong SQL generated when using setDistinct and order by on aggregated column

Browse Source
This commit is contained in:
rob bygrave
2019-07-01 22:50:44 +12:00
parent 7f2e4f54ff
commit 52398317bc
5 changed files with 59 additions and 9 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/main/java/io/ebean/OrderBy.java
+20 -7
View File
@@ -80,7 +80,6 @@ public final class OrderBy<T> implements Serializable {
* Add a property with ascending order to this OrderBy.
*/
public Query<T> asc(String propertyName, String collation) {
list.add(new Property(propertyName, true, collation));
return query;
}
@@ -89,7 +88,6 @@ public final class OrderBy<T> implements Serializable {
* Add a property with descending order to this OrderBy.
*/
public Query<T> desc(String propertyName) {
list.add(new Property(propertyName, false));
return query;
}
@@ -98,7 +96,6 @@ public final class OrderBy<T> implements Serializable {
* Add a property with descending order to this OrderBy.
*/
public Query<T> desc(String propertyName, String collation) {
list.add(new Property(propertyName, false, collation));
return query;
}
@@ -108,7 +105,6 @@ public final class OrderBy<T> implements Serializable {
* Return true if the property is known to be contained in the order by clause.
*/
public boolean containsProperty(String propertyName) {
for (Property aList : list) {
if (propertyName.equals(aList.getProperty())) {
return true;
@@ -161,10 +157,9 @@ public final class OrderBy<T> implements Serializable {
* Return a copy of the OrderBy.
*/
public OrderBy<T> copy() {
OrderBy<T> copy = new OrderBy<>();
for (Property aList : list) {
copy.add(aList.copy());
for (Property property : list) {
copy.add(property.copy());
}
return copy;
}
@@ -241,6 +236,18 @@ public final class OrderBy<T> implements Serializable {
return this;
}
/**
* Return true if this order by can be used in select clause.
*/
public boolean supportsSelect() {
for (Property property : list) {
if (!property.supportsSelect()) {
return false;
}
}
return true;
}
/**
* A property and its ascending descending order.
*/
@@ -401,6 +408,12 @@ public final class OrderBy<T> implements Serializable {
this.ascending = ascending;
}
/**
* Support use in select clause if no collation or nulls ordering.
*/
boolean supportsSelect() {
return nulls == null && collation == null;
}
}
private void parse(String orderByClause) {
src/main/java/io/ebeaninternal/server/query/CQueryBuilder.java
+5 -1
View File
@@ -1,6 +1,7 @@
package io.ebeaninternal.server.query;
import io.ebean.CountDistinctOrder;
import io.ebean.OrderBy;
import io.ebean.Query;
import io.ebean.RawSql;
import io.ebean.RawSqlBuilder;
@@ -600,7 +601,10 @@ class CQueryBuilder {
}
if (distinct && dbOrderBy != null && !query.isSingleAttribute()) {
// add the orderBy columns to the select clause (due to distinct)
sb.append(", ").append(DbOrderByTrim.trim(dbOrderBy));
final OrderBy<?> orderBy = query.getOrderBy();
if (orderBy != null && orderBy.supportsSelect()) {
sb.append(", ").append(DbOrderByTrim.trim(dbOrderBy));
}
}
}
src/test/java/org/tests/aggregateformula/TestAggregateFormula.java
+24
View File
@@ -17,6 +17,30 @@ import static org.junit.Assert.assertNotNull;
public class TestAggregateFormula extends BaseTestCase {
@Test
public void minDistinctOrderByNulls() {
ResetBasicData.reset();
LoggedSqlCollector.start();
List<Contact> contacts = Ebean.find(Contact.class)
.setDistinct(true)
.select("lastName, min(customer)")
.orderBy("min(customer) asc nulls last")
.findList();
List<String> sql = LoggedSqlCollector.stop();
assertThat(sql.get(0)).contains("select distinct t0.last_name, min(t0.customer_id) from contact t0 group by t0.last_name order by min(t0.customer_id) nulls last");
assertThat(contacts).isNotEmpty();
Contact contact = contacts.get(0);
assertThat(contact.getLastName()).isNotNull();
assertThat(contact.getCustomer()).isNotNull();
assertThat(contact.getCustomer().getId()).isNotNull();
}
@Test
public void minOnManyToOne() {
src/test/java/org/tests/query/orderby/TestOrderByWithDistinct.java
+1 -1
View File
@@ -57,7 +57,7 @@ public class TestOrderByWithDistinct extends BaseTestCase {
if (isPostgres()) {
assertThat(sql).contains("select distinct on (t0.user_name, t0.userid) t0.userid,");
} else if (isH2()) {
assertThat(sql).contains("select distinct t0.userid, t0.user_name, t0.user_type_id,");
assertThat(sql).contains("select distinct t0.userid, t0.user_name, t0.user_type_id from muser t0");
}
}
src/test/java/org/tests/unitinternal/TestOrderByParse.java
+9
View File
@@ -24,6 +24,7 @@ public class TestOrderByParse extends BaseTestCase {
assertTrue(o1.getProperties().get(0).isAscending());
assertThat(o1.toStringFormat()).isEqualTo("case when status='N' then 1 when status='F' then 2 else 99 end");
assertThat(o1.getProperties().get(0).getProperty()).isEqualTo("case when status='N' then 1 when status='F' then 2 else 99 end");
assertTrue(o1.supportsSelect());
}
@Test
@@ -34,6 +35,7 @@ public class TestOrderByParse extends BaseTestCase {
assertEquals("id", o1.getProperties().get(0).getProperty());
assertTrue(o1.getProperties().get(0).isAscending());
assertEquals("id", o1.toStringFormat());
assertTrue(o1.supportsSelect());
o1 = new OrderBy<>("id asc");
assertEquals(1, o1.getProperties().size());
@@ -65,6 +67,7 @@ public class TestOrderByParse extends BaseTestCase {
assertEquals("id", o1.getProperties().get(0).getProperty());
assertTrue(!o1.getProperties().get(0).isAscending());
assertEquals("id desc nulls high", o1.toStringFormat());
assertFalse(o1.supportsSelect());
}
@Test
@@ -100,6 +103,7 @@ public class TestOrderByParse extends BaseTestCase {
assertEquals("name", o1.getProperties().get(1).getProperty());
assertTrue(o1.getProperties().get(1).isAscending());
assertEquals("id, name", o1.toStringFormat());
assertTrue(o1.supportsSelect());
o1 = new OrderBy<>(" id , name ");
assertEquals(2, o1.getProperties().size());
@@ -178,6 +182,7 @@ public class TestOrderByParse extends BaseTestCase {
assertEquals("id", o1.getProperties().get(0).getProperty());
assertTrue(o1.getProperties().get(0).isAscending());
assertEquals("id collate latin_1", o1.toStringFormat());
assertFalse(o1.supportsSelect());
o1 = new OrderBy<>();
o1.desc("id", "latin_1");
@@ -185,6 +190,7 @@ public class TestOrderByParse extends BaseTestCase {
assertEquals("id", o1.getProperties().get(0).getProperty());
assertTrue(!o1.getProperties().get(0).isAscending());
assertEquals("id collate latin_1 desc", o1.toStringFormat());
assertFalse(o1.supportsSelect());
o1 = new OrderBy<>();
o1.desc("id", "latin_1");
@@ -195,6 +201,7 @@ public class TestOrderByParse extends BaseTestCase {
assertTrue(!o1.getProperties().get(0).isAscending());
assertTrue(o1.getProperties().get(1).isAscending());
assertEquals("id collate latin_1 desc, date", o1.toStringFormat());
assertFalse(o1.supportsSelect());
o1 = new OrderBy<>();
o1.desc("id", "latin_1");
@@ -205,6 +212,7 @@ public class TestOrderByParse extends BaseTestCase {
assertTrue(!o1.getProperties().get(0).isAscending());
assertTrue(o1.getProperties().get(1).isAscending());
assertEquals("id collate latin_1 desc, name collate latin_2", o1.toStringFormat());
assertFalse(o1.supportsSelect());
// functional (DB2) syntax
o1 = new OrderBy<>();
@@ -213,6 +221,7 @@ public class TestOrderByParse extends BaseTestCase {
assertEquals("id", o1.getProperties().get(0).getProperty());
assertTrue(!o1.getProperties().get(0).isAscending());
assertEquals("COLLATION_KEY(id, 'latin_1') desc", o1.toStringFormat());
assertFalse(o1.supportsSelect());
}