Skip to content

Commit 63fa6a5

Browse files
author
Igor Polevoy
committed
#558 Paginator doesn't account for "distinct" keyword
1 parent d1a7c20 commit 63fa6a5

File tree

3 files changed

+140
-10
lines changed

3 files changed

+140
-10
lines changed

activejdbc/src/main/java/org/javalite/activejdbc/Paginator.java

Lines changed: 116 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ public class Paginator<T extends Model> implements Serializable {
4646
private final MetaModel metaModel;
4747
private int currentPage;
4848
private final boolean fullQuery;
49-
private final String countQuery;
49+
private final String countQueryFull;
5050
private boolean suppressCounts;
5151
private Long count = 0L;
5252

@@ -55,7 +55,14 @@ public class Paginator<T extends Model> implements Serializable {
5555
* Convenience constructor. Calls {@link #Paginator(Class, int, String, Object...)} and passes true for <code>suppressCounts</code>.
5656
*/
5757
public Paginator(Class<? extends T> modelClass, int pageSize, String query, Object... params) {
58-
this(modelClass, pageSize, false, query, params);
58+
this(modelClass, pageSize, false, query, null, params);
59+
}
60+
61+
/**
62+
* Convenience constructor. Calls {@link #Paginator(Class, int, String, Object...)} and passes null for <code>countQuery</code>.
63+
*/
64+
public Paginator(Class<? extends T> modelClass, int pageSize, boolean suppressCounts, String query, Object... params) {
65+
this(modelClass, pageSize, suppressCounts, query, null, params);
5966
}
6067

6168
/**
@@ -87,7 +94,7 @@ public Paginator(Class<? extends T> modelClass, int pageSize, String query, Obje
8794
* query should not contain limit, offset or order by clauses of any kind, Paginator will do this automatically.
8895
* This parameter can have two forms, a sub-query or a full query.
8996
*/
90-
public Paginator(Class<? extends T> modelClass, int pageSize, boolean suppressCounts, String query, Object... params) {
97+
public Paginator(Class<? extends T> modelClass, int pageSize, boolean suppressCounts, String query, String countQuery, Object... params) {
9198

9299
this.suppressCounts = suppressCounts;
93100
try {
@@ -102,21 +109,27 @@ public Paginator(Class<? extends T> modelClass, int pageSize, boolean suppressCo
102109
String tableName = Registry.instance().getTableName(modelClass);
103110
this.metaModel = metaModelFor(tableName);
104111

112+
105113
this.fullQuery = DB.SELECT_PATTERN.matcher(query).find();
106114
if (fullQuery) {
107115
Matcher m = FROM_PATTERN.matcher(query);
108116
if (!m.find()) {
109117
throw new IllegalArgumentException("SELECT query without FROM");
110118
}
111-
this.countQuery = metaModel.getDialect().selectCount(query.substring(m.end()));
119+
String from = query.substring(m.end());
120+
if (countQuery != null) {
121+
this.countQueryFull = "SELECT " + countQuery + " FROM " + from;
122+
} else {
123+
this.countQueryFull = metaModel.getDialect().selectCount(from);
124+
}
112125
} else if (query.equals("*")) {
113126
if (params.length == 0) {
114-
this.countQuery = metaModel.getDialect().selectCount(tableName);
127+
this.countQueryFull = metaModel.getDialect().selectCount(tableName);
115128
} else {
116129
throw new IllegalArgumentException("cannot provide parameters with query: '*'");
117130
}
118131
} else {
119-
this.countQuery = metaModel.getDialect().selectCount(tableName, query);
132+
this.countQueryFull = metaModel.getDialect().selectCount(tableName, query);
120133
}
121134
}
122135

@@ -223,10 +236,10 @@ private LazyList<T> findAll() {
223236
public Long getCount() {
224237
if (count == 0L || !suppressCounts) {
225238
if (metaModel.cached()) {
226-
count = (Long) QueryCache.instance().getItem(metaModel.getTableName(), countQuery, params);
239+
count = (Long) QueryCache.instance().getItem(metaModel.getTableName(), countQueryFull, params);
227240
if (count == null || count == 0) {
228241
count = doCount();
229-
QueryCache.instance().addItem(metaModel.getTableName(), countQuery, params, count);
242+
QueryCache.instance().addItem(metaModel.getTableName(), countQueryFull, params, count);
230243
}
231244
} else {
232245
count = doCount();
@@ -239,10 +252,104 @@ public Long getCount() {
239252
}
240253

241254
private Long doCount() {
242-
return Convert.toLong(new DB(metaModel.getDbName()).firstCell(countQuery, params));
255+
return Convert.toLong(new DB(metaModel.getDbName()).firstCell(countQueryFull, params));
243256
}
244257

245258
public int getPageSize() {
246259
return pageSize;
247260
}
261+
262+
/**
263+
* Use to create a paginator instance, and provide arguments as needed.
264+
* @return self.
265+
*/
266+
public static PaginatorBuilder instance(){
267+
return new PaginatorBuilder();
268+
}
269+
270+
271+
/**
272+
* Provides a builder pattern to create new instances of paginator.
273+
*/
274+
static class PaginatorBuilder<T extends Model>{
275+
private Class<? extends T> modelClass;
276+
private int pageSize;
277+
private boolean suppressCounts = false;
278+
private String query;
279+
private String countQuery = null;
280+
private Object[] params;
281+
282+
/**
283+
* Model class mapped to a table.>
284+
*
285+
* @param modelClass Model class mapped to a table.>
286+
* @return self
287+
*/
288+
public PaginatorBuilder modelClass(Class<T> modelClass){
289+
this.modelClass = modelClass;
290+
return this;
291+
}
292+
293+
/**
294+
* Page size - number of items in a page
295+
*
296+
* @param pageSize Page size - number of items in a page
297+
*/
298+
public PaginatorBuilder pageSize(int pageSize){
299+
this.pageSize = pageSize;
300+
return this;
301+
}
302+
303+
/**
304+
* Suppress calling "select count(*)... " on a table each time. If set to true,
305+
* it will call count only once. If set to false, it will call count each time
306+
* {@link #getCount()} is called from {@link #hasNext()} as well.
307+
*
308+
* @param suppressCounts suppress counts every time.
309+
*/
310+
public PaginatorBuilder suppressCounts(boolean suppressCounts){
311+
this.suppressCounts= suppressCounts;
312+
return this;
313+
}
314+
315+
316+
/**
317+
* @param query Query that will be applied every time a new page is requested; this
318+
* query should not contain limit, offset or order by clauses of any kind, Paginator will do this automatically.
319+
* This parameter can have two forms, a sub-query or a full query.
320+
*/
321+
public PaginatorBuilder query(String query) {
322+
this.query = query;
323+
return this;
324+
}
325+
326+
/**
327+
* Part of the query that is responsible for count. Example: <code>COUNT(DISTINCT(u.id)</code>.
328+
* Only use this method if you need something more complex than <code>COUNT(*)</code>, since
329+
* that is the value that us used by default.
330+
*
331+
* @param countQuery Part of the query that is responsible for "count. Example: <code>count(*)</code>" or <code>COUNT(DISTINCT(u.id)</code>.
332+
*/
333+
public PaginatorBuilder countQuery(String countQuery) {
334+
this.countQuery = countQuery;
335+
return this;
336+
}
337+
338+
/**
339+
* Array of parameters in case a query is parametrized
340+
* @param params Array of parameters in case a query is parametrized
341+
*/
342+
public PaginatorBuilder params(Object ... params){
343+
this.params = params;
344+
return this;
345+
}
346+
347+
/**
348+
* Terminal method to create an instance of Paginator.
349+
* @return new Paginator properly configured.
350+
*/
351+
public Paginator<T> create(){
352+
return new Paginator<T>(modelClass, pageSize, suppressCounts, query, countQuery, params);
353+
}
354+
}
248355
}

activejdbc/src/test/java/org/javalite/activejdbc/PaginatorTest.java

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,9 @@
1818
package org.javalite.activejdbc;
1919

2020
import org.javalite.activejdbc.test.ActiveJDBCTest;
21+
import org.javalite.activejdbc.test_models.Address;
2122
import org.javalite.activejdbc.test_models.Item;
23+
import org.javalite.activejdbc.test_models.User;
2224
import org.junit.Test;
2325

2426
import java.io.*;
@@ -156,4 +158,26 @@ public void should_not_ignore_changes_in_count_after_started(){
156158
}
157159
a(p.getCount()).shouldBeEqual(1004);
158160
}
161+
162+
@Test
163+
public void should_Fix_558(){ // https://github.com/javalite/activejdbc/issues/558
164+
165+
User u = User.createIt("email", "john@doe.com", "first_name", "John", "last_name", "Doe");
166+
167+
u.add(Address.create("address1", "123 Pine St.", "address2", "apt 1", "city", "Springfield", "state", "IL", "zip", "60004"));
168+
u.add(Address.create("address1", "456 Pine St.", "address2", "apt 3", "city", "Springfield", "state", "IL", "zip", "60004"));
169+
170+
Paginator<User> paginator2 = Paginator.instance()
171+
.modelClass(User.class)
172+
.query("select distinct u.* FROM users u left join addresses a on u.id=a.user_id where a.address1 like ?")
173+
.pageSize(5)
174+
.params("%Pine%")
175+
.countQuery("COUNT(DISTINCT u.id)")
176+
.create();
177+
178+
a(paginator2.getCount()).shouldBeEqual(1);
179+
180+
}
181+
182+
159183
}

activejdbc/src/test/java/org/javalite/activejdbc/test/ActiveJDBCTest.java

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -219,7 +219,6 @@ private void executeStatements(List<String> statements) {
219219
try {
220220
st = Base.connection().createStatement();
221221
st.executeUpdate(statement);
222-
System.out.println("STATEMENT: " + statement);
223222
} catch (SQLException e) {
224223
throw new RuntimeException(statement, e);
225224
} finally {

0 commit comments

Comments
 (0)