Skip to content

Commit

Permalink
Fix for #85 - Invalid query when ORDER BY with DISTINCT
Browse files Browse the repository at this point in the history
  • Loading branch information
rbygrave committed May 18, 2014
1 parent b587aa8 commit b8743a5
Show file tree
Hide file tree
Showing 2 changed files with 81 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -305,6 +305,8 @@ private SqlLimitResponse buildSql(String selectClause, OrmQueryRequest<?> reques

StringBuilder sb = new StringBuilder(500);

String dbOrderBy = predicates.getDbOrderBy();

if (selectClause != null) {
sb.append(selectClause);

Expand All @@ -320,6 +322,10 @@ private SqlLimitResponse buildSql(String selectClause, OrmQueryRequest<?> reques
}

sb.append(select.getSelectSql());
if (query.isDistinct() && dbOrderBy != null) {
// add the orderby columns to the select clause (due to distinct)
sb.append(", ").append(convertDbOrderByForSelect(dbOrderBy));
}
}

sb.append(" from ");
Expand Down Expand Up @@ -375,7 +381,7 @@ private SqlLimitResponse buildSql(String selectClause, OrmQueryRequest<?> reques
sb.append(dbFilterMany);
}

String dbOrderBy = predicates.getDbOrderBy();

if (dbOrderBy != null) {
sb.append(" order by ").append(dbOrderBy);
}
Expand All @@ -386,12 +392,20 @@ private SqlLimitResponse buildSql(String selectClause, OrmQueryRequest<?> reques
return sqlLimiter.limit(r);

} else {

return new SqlLimitResponse(dbPlatform.completeSql(sb.toString(), query), false);
}

}

/**
* Convert the dbOrderBy clause to be safe for adding to select. This is done when 'distinct' is
* used.
*/
private String convertDbOrderByForSelect(String dbOrderBy) {
// just remove the ASC and DESC keywords
return dbOrderBy.replaceAll("(?i)\\b asc\\b|\\b desc\\b", "");
}

private boolean isEmpty(String s) {
return s == null || s.length() == 0;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
package com.avaje.tests.query.orderby;

import org.junit.Assert;
import org.junit.Test;

import com.avaje.ebean.BaseTestCase;
import com.avaje.ebean.Ebean;
import com.avaje.ebean.Query;
import com.avaje.tests.model.basic.Customer;

public class TestOrderByWithDistinctTake2 extends BaseTestCase {

@Test
public void testRegex() {

String test = "helloasc asc ASC desc DESC boodesc desc ASC";

test = test.replaceAll("(?i)\\b asc\\b|\\b desc\\b", "");
System.out.println(test);
Assert.assertEquals("helloasc boodesc", test);
}


@Test
public void test() {

Query<Customer> query = Ebean.find(Customer.class)
.select("id")
.where().ilike("contacts.firstName", "R%")
.order("name desc");

query.findList();

String generatedSql = query.getGeneratedSql();

// select distinct t0.id c0, t0.name
// from o_customer t0 join contact u1 on u1.customer_id = t0.id
// where lower(u1.first_name) like ?
// order by t0.name; --bind(r%)

Assert.assertTrue("t0.name added to the select clause", generatedSql.contains("select distinct t0.id c0, t0.name"));
Assert.assertTrue(generatedSql.contains("order by t0.name desc"));
Assert.assertTrue(generatedSql.contains("from o_customer t0 join contact u1 on u1.customer_id = t0.id"));
Assert.assertTrue(generatedSql.contains("where lower(u1.first_name) like ?"));
}

@Test
public void testWithAscAndDesc() {

Query<Customer> query = Ebean.find(Customer.class)
.select("id")
.where().ilike("contacts.firstName", "R%")
.order("name asc,id desc");

query.findList();

String generatedSql = query.getGeneratedSql();

Assert.assertTrue("t0.name added to the select clause", generatedSql.contains("select distinct t0.id c0, t0.name, t0.id"));
Assert.assertTrue(generatedSql.contains("order by t0.name, t0.id desc"));
Assert.assertTrue(generatedSql.contains("from o_customer t0 join contact u1 on u1.customer_id = t0.id"));
Assert.assertTrue(generatedSql.contains("where lower(u1.first_name) like ?"));
}

}

0 comments on commit b8743a5

Please sign in to comment.