Skip to content

Commit 10396c6

Browse files
andrusandrus
committed
CAY-2967 SQLTemplate/SQLSelect broken pagination
1 parent 190eb36 commit 10396c6

8 files changed

Lines changed: 285 additions & 26 deletions

File tree

‎RELEASE-NOTES.txt‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ CAY-2961 PostgreSQL "text" column is reverse-engineered as CLOB
2828
CAY-2964 ClassCastException for non-generated meaningful PKs
2929
CAY-2965 MCP Cgen should not fail on an absent "<cgen>" tag
3030
CAY-2966 "comment" field is lost when upgrading from v10 to v12
31+
CAY-2967 SQLTemplate/SQLSelect broken pagination
3132

3233
----------------------------------
3334
Release: 5.0-M2

‎cayenne/src/main/java/org/apache/cayenne/access/jdbc/RSColumn.java‎

Lines changed: 41 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
import org.apache.cayenne.access.types.ExtendedTypeMap;
2525
import org.apache.cayenne.dba.TypesMapping;
2626
import org.apache.cayenne.map.DbAttribute;
27+
import org.apache.cayenne.map.DbEntity;
2728
import org.slf4j.Logger;
2829
import org.slf4j.LoggerFactory;
2930

@@ -104,28 +105,32 @@ public static class RowBuilder {
104105
private Map<String, String> typeOverrides;
105106
private boolean mergeColumnsWithRsMetadata;
106107
private boolean validateDuplicateColumnNames;
108+
private DbEntity dbEntity;
107109

108110
/**
109111
* Returns the array of {@link RSColumn}s describing the result row, with an {@link ExtendedType}
110112
* resolved for each column.
111113
*/
112114
public RSColumn[] build(ExtendedTypeMap typeMap) throws SQLException, IllegalStateException {
113115

114-
RSColumn[] columnsForRD;
116+
RSColumn[] columns1;
115117

116118
if (this.resultSetMetadata != null) {
117119
// do merge between explicitly-set columns and ResultSetMetadata
118120
// explicitly-set columns take precedence
119-
columnsForRD = mergeResultSetAndPresetColumns(typeMap);
121+
columns1 = mergeResultSetAndPresetColumns(typeMap);
120122
} else if (this.columns != null) {
121123
// use explicitly-set columns
122-
columnsForRD = this.columns;
124+
columns1 = this.columns;
123125
} else {
124126
throw new IllegalStateException(
125127
"Can't build row descriptor, both 'columns' and 'resultSetMetadata' are null");
126128
}
127129

128-
return performTransformAndTypeOverride(columnsForRD, typeMap);
130+
RSColumn[] columns2 = performTransformAndTypeOverride(columns1, typeMap);
131+
132+
// match with DbAttributes after the final names are resolved
133+
return resolveDbAttributes(columns2);
129134
}
130135

131136
/**
@@ -267,6 +272,29 @@ private RSColumn[] performTransformAndTypeOverride(RSColumn[] columnArray, Exten
267272
return result;
268273
}
269274

275+
private RSColumn[] resolveDbAttributes(RSColumn[] columnArray) {
276+
if (dbEntity == null) {
277+
return columnArray;
278+
}
279+
280+
RSColumn[] result = null;
281+
for (int i = 0; i < columnArray.length; i++) {
282+
RSColumn column = columnArray[i];
283+
if (column.attribute() != null) {
284+
continue;
285+
}
286+
DbAttribute attribute = dbEntity.getAttribute(column.rsName());
287+
if (attribute != null) {
288+
if (result == null) {
289+
result = columnArray.clone();
290+
}
291+
result[i] = new RSColumn(
292+
column.rsName(), column.rsType(), column.dataRowName(), column.reader(), attribute);
293+
}
294+
}
295+
return result != null ? result : columnArray;
296+
}
297+
270298
/**
271299
* Sets an explicit set of columns. The builder may replace these with new instances to enforce the column
272300
* capitalization policy and column Java type overrides.
@@ -315,5 +343,14 @@ public RowBuilder mergeColumnsWithRsMetadata() {
315343
this.mergeColumnsWithRsMetadata = true;
316344
return this;
317345
}
346+
347+
/**
348+
* Sets the root DbEntity used to resolve each column's {@link DbAttribute} by name. Optional - when not set,
349+
* column attributes are left as provided.
350+
*/
351+
public RowBuilder dbEntity(DbEntity dbEntity) {
352+
this.dbEntity = dbEntity;
353+
return this;
354+
}
318355
}
319356
}

‎cayenne/src/main/java/org/apache/cayenne/access/jdbc/SQLTemplateAction.java‎

Lines changed: 15 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -234,15 +234,18 @@ protected void execute(Connection connection, OperationObserver callback, Transl
234234
}
235235

236236
@SuppressWarnings({"unchecked", "rawtypes"})
237-
protected void processSelectResult(TranslatedSQL compiled, Connection connection, Statement statement,
238-
ResultSet resultSet, OperationObserver callback, final long startTime) throws Exception {
237+
protected void processSelectResult(
238+
TranslatedSQL compiled,
239+
Connection connection, Statement statement,
240+
ResultSet resultSet,
241+
OperationObserver callback,
242+
long startTime) throws Exception {
239243

240-
boolean iteratedResult = callback.isIteratedResult();
241-
ExtendedTypeMap types = dataNode.getAdapter().getExtendedTypes();
242-
RSColumn.RowBuilder rowBuilder = rowBuilder(compiled, resultSet);
243244
recreateQueryMetadata(resultSet);
244-
RowReader<?> rowReader = dataNode.getRowReaderFactory()
245-
.rowReader(rowBuilder.build(types), queryMetadata, dataNode.getAdapter());
245+
boolean iteratedResult = callback.isIteratedResult();
246+
RSColumn[] columns = rowBuilder(compiled, resultSet).build(dataNode.getAdapter().getExtendedTypes());
247+
248+
RowReader<?> rowReader = dataNode.getRowReaderFactory().rowReader(columns, queryMetadata, dataNode.getAdapter());
246249
ResultIterator<?> it = new RSIterator<>(statement, resultSet, rowReader);
247250

248251
if (iteratedResult) {
@@ -317,12 +320,14 @@ private RSColumn[] createColumnDescriptors(TranslatedSQL compiled) {
317320
/**
318321
* @since 3.0
319322
*/
320-
protected RSColumn.RowBuilder rowBuilder(TranslatedSQL compiled, ResultSet resultSet)
321-
throws SQLException {
323+
protected RSColumn.RowBuilder rowBuilder(TranslatedSQL compiled, ResultSet resultSet) throws SQLException {
322324
RSColumn.RowBuilder builder = RSColumn.rowBuilder()
323325
.resultSet(resultSet)
324326
.columns(createColumnDescriptors(compiled))
325-
.validateDuplicateColumnNames();
327+
.validateDuplicateColumnNames()
328+
// resolve column DbAttributes so the row reader factory can tell e.g. PK columns apart, the same way
329+
// it can for ObjectSelect; lets pagination read the PK regardless of column order
330+
.dbEntity(dbEntity);
326331

327332
if (query.getResultColumnsTypes() != null) {
328333
builder.mergeColumnsWithRsMetadata();

‎cayenne/src/main/java/org/apache/cayenne/access/jdbc/reader/DefaultRowReaderFactory.java‎

Lines changed: 67 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -18,14 +18,20 @@
1818
****************************************************************/
1919
package org.apache.cayenne.access.jdbc.reader;
2020

21+
import org.apache.cayenne.CayenneRuntimeException;
2122
import org.apache.cayenne.access.jdbc.RSColumn;
2223
import org.apache.cayenne.dba.DbAdapter;
24+
import org.apache.cayenne.map.DbAttribute;
25+
import org.apache.cayenne.map.DbEntity;
26+
import org.apache.cayenne.map.ObjEntity;
2327
import org.apache.cayenne.query.EmbeddableResultSegment;
2428
import org.apache.cayenne.query.EntityResultSegment;
2529
import org.apache.cayenne.query.QueryMetadata;
2630
import org.apache.cayenne.query.ScalarResultSegment;
2731

32+
import java.util.HashSet;
2833
import java.util.List;
34+
import java.util.Set;
2935

3036
/**
3137
* @since 4.0
@@ -95,14 +101,15 @@ protected RowReader<?> scalarSegmentReader(RSColumn[] columns, QueryMetadata met
95101

96102
protected RowReader<?> entitySegmentReader(RSColumn[] columns, QueryMetadata metadata, EntityResultSegment segment) {
97103

98-
// For a paginated query the result columns are trimmed to the root PK (see IdColumnExtractor). A single-column
99-
// PK is read as a scalar (consumed by SimpleIdIncrementalFaultList); a compound PK falls through to the regular
100-
// DataRow reader below, which over PK-only columns yields exactly the id map IncrementalFaultList expects.
101-
if (metadata.getPageSize() > 0
102-
&& segment.getClassDescriptor().getEntity().getDbEntity().getPrimaryKeys().size() == 1) {
103-
int pk = segment.getColumnOffset();
104-
// jdbc column indexes start from 1
105-
return new ScalarRowReader<>(columns[pk].reader(), pk + 1, columns[pk].rsType());
104+
// For a paginated query only the root PK is read into the IncrementalFaultList - locate the PK columns within
105+
// this segment by their DbAttribute (see idReader).
106+
if (metadata.getPageSize() > 0) {
107+
ObjEntity objEntity = segment.getClassDescriptor().getEntity();
108+
return idReader(columns,
109+
segment.getColumnOffset(),
110+
segment.getColumnOffset() + columns.length,
111+
objEntity.getDbEntity(),
112+
objEntity.getName());
106113
}
107114

108115
int startIndex = segment.getColumnOffset();
@@ -133,8 +140,57 @@ protected RowReader<?> entitySegmentReader(RSColumn[] columns, QueryMetadata met
133140
}
134141

135142
protected RowReader<?> noSegmentReader(RSColumn[] columns, QueryMetadata metadata) {
136-
return metadata.getPageSize() > 0 && metadata.getDbEntity().getPrimaryKeys().size() == 1
137-
? new ScalarRowReader<>(columns[0].reader(), 1, columns[0].rsType())
138-
: FullRowReader.of(columns, metadata);
143+
if (metadata.getPageSize() > 0) {
144+
ObjEntity objEntity = metadata.getObjEntity();
145+
return idReader(columns, 0, columns.length, metadata.getDbEntity(),
146+
objEntity != null ? objEntity.getName() : null);
147+
}
148+
return FullRowReader.of(columns, metadata);
139149
}
150+
151+
private RowReader<?> idReader(RSColumn[] columns, int from, int to, DbEntity dbEntity, String entityName) {
152+
if (dbEntity == null) {
153+
throw new CayenneRuntimeException("Null root DbEntity, can't index PK");
154+
}
155+
156+
int pkLen = dbEntity.getPrimaryKeys().size();
157+
if (pkLen == 0) {
158+
throw new CayenneRuntimeException("Root DbEntity has no PK defined: %s", dbEntity.getName());
159+
}
160+
161+
RSColumn[] pkColumns = new RSColumn[pkLen];
162+
int[] pkIndexes = new int[pkLen];
163+
int found = 0;
164+
165+
Set<DbAttribute> seen = new HashSet<>();
166+
for (int i = from; i < to && found < pkLen; i++) {
167+
DbAttribute attribute = columns[i].attribute();
168+
if (attribute != null && attribute.isPrimaryKey() && seen.add(attribute)) {
169+
pkColumns[found] = columns[i];
170+
pkIndexes[found] = i + 1;
171+
found++;
172+
}
173+
}
174+
175+
if (found != pkLen) {
176+
// TODO: HACK: the result columns don't carry resolvable PK DbAttributes (these are most likely aliased
177+
// EJBQL columns). Fall back to the legacy assumption that the PK is the first pkLen columns starting at
178+
// 'from'.
179+
if (from + pkLen > columns.length) {
180+
throw new CayenneRuntimeException(
181+
"Result set for paginated query is missing PK column(s) of entity '%s'; expected %s",
182+
entityName, dbEntity.getPrimaryKeys().stream().map(DbAttribute::getName).toList());
183+
}
184+
for (int i = 0; i < pkLen; i++) {
185+
pkColumns[i] = columns[from + i];
186+
// jdbc column indexes start from 1
187+
pkIndexes[i] = from + i + 1;
188+
}
189+
}
190+
191+
return pkLen == 1
192+
? new ScalarRowReader<>(pkColumns[0].reader(), pkIndexes[0], pkColumns[0].rsType())
193+
: IndexRowReader.of(pkColumns, pkIndexes, entityName);
194+
}
195+
140196
}

‎cayenne/src/main/java/org/apache/cayenne/access/jdbc/reader/IndexRowReader.java‎

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
/*****************************************************************
2+
* Licensed to the Apache Software Foundation (ASF) under one
3+
* or more contributor license agreements. See the NOTICE file
4+
* distributed with this work for additional information
5+
* regarding copyright ownership. The ASF licenses this file
6+
* to you under the Apache License, Version 2.0 (the
7+
* "License"); you may not use this file except in compliance
8+
* with the License. You may obtain a copy of the License at
9+
*
10+
* https://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing,
13+
* software distributed under the License is distributed on an
14+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
* KIND, either express or implied. See the License for the
16+
* specific language governing permissions and limitations
17+
* under the License.
18+
****************************************************************/
19+
package org.apache.cayenne.access.jdbc.reader;
20+
21+
import org.apache.cayenne.CayenneRuntimeException;
22+
import org.apache.cayenne.DataRow;
23+
import org.apache.cayenne.access.jdbc.RSColumn;
24+
25+
import java.sql.ResultSet;
26+
27+
/**
28+
* A {@link RowReader} that materializes a DataRow from a set of columns read at explicit, possibly non-contiguous,
29+
* result-set positions. Unlike {@link OffsetRowReader}, each column carries its own position, so the columns need not
30+
* form a contiguous run.
31+
*/
32+
class IndexRowReader implements RowReader<DataRow> {
33+
34+
private final RSColumn[] columns;
35+
private final int[] jdbcIndexes;
36+
private final String entityName;
37+
private final int mapCapacity;
38+
39+
static RowReader<DataRow> of(RSColumn[] columns, int[] jdbcIndexes, String entityName) {
40+
return new IndexRowReader(columns, jdbcIndexes, entityName);
41+
}
42+
43+
private IndexRowReader(RSColumn[] columns, int[] jdbcIndexes, String entityName) {
44+
this.columns = columns;
45+
this.jdbcIndexes = jdbcIndexes;
46+
this.entityName = entityName;
47+
this.mapCapacity = (int) Math.ceil(columns.length / 0.75);
48+
}
49+
50+
@Override
51+
public DataRow readRow(ResultSet resultSet) {
52+
DataRow dataRow = new DataRow(mapCapacity);
53+
54+
try {
55+
for (int i = 0; i < columns.length; i++) {
56+
RSColumn column = columns[i];
57+
Object val = column.reader().materializeObject(resultSet, jdbcIndexes[i], column.rsType());
58+
dataRow.put(column.dataRowName(), val);
59+
}
60+
61+
dataRow.setEntityName(entityName);
62+
return dataRow;
63+
} catch (CayenneRuntimeException ex) {
64+
throw ex;
65+
} catch (Exception ex) {
66+
throw new CayenneRuntimeException("Exception materializing column.", ex);
67+
}
68+
}
69+
}

‎cayenne/src/test/java/org/apache/cayenne/CayenneCompoundIT.java‎

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,11 @@
2020
package org.apache.cayenne;
2121

2222
import org.apache.cayenne.exp.property.PropertyFactory;
23+
import org.apache.cayenne.query.CapsStrategy;
2324
import org.apache.cayenne.query.EJBQLQuery;
2425
import org.apache.cayenne.query.ObjectSelect;
26+
import org.apache.cayenne.query.SQLSelect;
27+
import org.apache.cayenne.query.SQLTemplate;
2528
import org.apache.cayenne.test.jdbc.TableHelper;
2629
import org.apache.cayenne.testdo.compound.CharPkTestEntity;
2730
import org.apache.cayenne.testdo.compound.CompoundPkTestEntity;
@@ -179,6 +182,49 @@ public void paginatedObjectSelect() throws Exception {
179182
assertEquals(4, queriesCount);
180183
}
181184

185+
@Test
186+
public void pageSizeCompoundPkSQLTemplate() throws Exception {
187+
createCompoundPKs(20);
188+
189+
int pageSize = 7;
190+
191+
// the compound PK columns (KEY1, KEY2) are NOT the first selected columns and are read as a
192+
// DataRow id map by the paginated id reader
193+
SQLTemplate query = new SQLTemplate(CompoundPkTestEntity.class,
194+
"SELECT NAME, KEY1, KEY2 FROM COMPOUND_PK_TEST ORDER BY NAME");
195+
query.setColumnNamesCapitalization(CapsStrategy.UPPER);
196+
query.setPageSize(pageSize);
197+
198+
List<?> result = env.context().performQuery(query);
199+
assertEquals(20, result.size());
200+
201+
// resolve every page, including subsequent pages faulted in by compound id
202+
for (int i = 0; i < result.size(); i++) {
203+
CompoundPkTestEntity e = (CompoundPkTestEntity) result.get(i);
204+
assertEquals(String.format("BBB%02d", i), e.getName());
205+
}
206+
}
207+
208+
@Test
209+
public void pageSizeCompoundPkSQLSelect() throws Exception {
210+
createCompoundPKs(20);
211+
212+
int pageSize = 7;
213+
214+
// SQLSelect builds a replacement SQLTemplate, so it exercises the same compound-PK paginated id reader
215+
SQLSelect<CompoundPkTestEntity> query = SQLSelect
216+
.query(CompoundPkTestEntity.class, "SELECT NAME, KEY1, KEY2 FROM COMPOUND_PK_TEST ORDER BY NAME")
217+
.columnNameCaps(CapsStrategy.UPPER)
218+
.pageSize(pageSize);
219+
220+
List<CompoundPkTestEntity> result = query.select(env.context());
221+
assertEquals(20, result.size());
222+
223+
for (int i = 0; i < result.size(); i++) {
224+
assertEquals(String.format("BBB%02d", i), result.get(i).getName());
225+
}
226+
}
227+
182228
@Test
183229
public void ejbqlCountSelect() throws Exception {
184230
tCompoundIntPKTest.insert(1, 2, "test");

‎cayenne/src/test/java/org/apache/cayenne/access/DataContextIT.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -443,7 +443,7 @@ public void performPaginatedQuery() throws Exception {
443443
}
444444

445445
@Test
446-
public void performPaginatedQuery1() throws Exception {
446+
public void performPaginatedQuery_EJBQL() throws Exception {
447447
createArtistsDataSet();
448448

449449
EJBQLQuery query = new EJBQLQuery("select a FROM Artist a");

0 commit comments

Comments
 (0)