This is an automated email from the ASF dual-hosted git repository. asf-gitbox-commits pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/cayenne.git
commit 10396c6d888ccdf569d1430c639736375d4a3b33 Author: Andrus Adamchik <[email protected]> AuthorDate: Mon Jun 29 12:40:41 2026 -0400 CAY-2967 SQLTemplate/SQLSelect broken pagination --- RELEASE-NOTES.txt | 1 + .../org/apache/cayenne/access/jdbc/RSColumn.java | 45 +++++++++++-- .../cayenne/access/jdbc/SQLTemplateAction.java | 25 ++++--- .../jdbc/reader/DefaultRowReaderFactory.java | 78 +++++++++++++++++++--- .../cayenne/access/jdbc/reader/IndexRowReader.java | 69 +++++++++++++++++++ .../java/org/apache/cayenne/CayenneCompoundIT.java | 46 +++++++++++++ .../org/apache/cayenne/access/DataContextIT.java | 2 +- .../cayenne/access/DataContextSQLTemplateIT.java | 45 +++++++++++++ 8 files changed, 285 insertions(+), 26 deletions(-) diff --git a/RELEASE-NOTES.txt b/RELEASE-NOTES.txt index bc11eb7cb..40cb8de23 100644 --- a/RELEASE-NOTES.txt +++ b/RELEASE-NOTES.txt @@ -28,6 +28,7 @@ CAY-2961 PostgreSQL "text" column is reverse-engineered as CLOB CAY-2964 ClassCastException for non-generated meaningful PKs CAY-2965 MCP Cgen should not fail on an absent "<cgen>" tag CAY-2966 "comment" field is lost when upgrading from v10 to v12 +CAY-2967 SQLTemplate/SQLSelect broken pagination ---------------------------------- Release: 5.0-M2 diff --git a/cayenne/src/main/java/org/apache/cayenne/access/jdbc/RSColumn.java b/cayenne/src/main/java/org/apache/cayenne/access/jdbc/RSColumn.java index 6b2acf134..376f879b9 100644 --- a/cayenne/src/main/java/org/apache/cayenne/access/jdbc/RSColumn.java +++ b/cayenne/src/main/java/org/apache/cayenne/access/jdbc/RSColumn.java @@ -24,6 +24,7 @@ import org.apache.cayenne.access.types.ExtendedType; import org.apache.cayenne.access.types.ExtendedTypeMap; import org.apache.cayenne.dba.TypesMapping; import org.apache.cayenne.map.DbAttribute; +import org.apache.cayenne.map.DbEntity; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -104,6 +105,7 @@ public record RSColumn( private Map<String, String> typeOverrides; private boolean mergeColumnsWithRsMetadata; private boolean validateDuplicateColumnNames; + private DbEntity dbEntity; /** * Returns the array of {@link RSColumn}s describing the result row, with an {@link ExtendedType} @@ -111,21 +113,24 @@ public record RSColumn( */ public RSColumn[] build(ExtendedTypeMap typeMap) throws SQLException, IllegalStateException { - RSColumn[] columnsForRD; + RSColumn[] columns1; if (this.resultSetMetadata != null) { // do merge between explicitly-set columns and ResultSetMetadata // explicitly-set columns take precedence - columnsForRD = mergeResultSetAndPresetColumns(typeMap); + columns1 = mergeResultSetAndPresetColumns(typeMap); } else if (this.columns != null) { // use explicitly-set columns - columnsForRD = this.columns; + columns1 = this.columns; } else { throw new IllegalStateException( "Can't build row descriptor, both 'columns' and 'resultSetMetadata' are null"); } - return performTransformAndTypeOverride(columnsForRD, typeMap); + RSColumn[] columns2 = performTransformAndTypeOverride(columns1, typeMap); + + // match with DbAttributes after the final names are resolved + return resolveDbAttributes(columns2); } /** @@ -267,6 +272,29 @@ public record RSColumn( return result; } + private RSColumn[] resolveDbAttributes(RSColumn[] columnArray) { + if (dbEntity == null) { + return columnArray; + } + + RSColumn[] result = null; + for (int i = 0; i < columnArray.length; i++) { + RSColumn column = columnArray[i]; + if (column.attribute() != null) { + continue; + } + DbAttribute attribute = dbEntity.getAttribute(column.rsName()); + if (attribute != null) { + if (result == null) { + result = columnArray.clone(); + } + result[i] = new RSColumn( + column.rsName(), column.rsType(), column.dataRowName(), column.reader(), attribute); + } + } + return result != null ? result : columnArray; + } + /** * Sets an explicit set of columns. The builder may replace these with new instances to enforce the column * capitalization policy and column Java type overrides. @@ -315,5 +343,14 @@ public record RSColumn( this.mergeColumnsWithRsMetadata = true; return this; } + + /** + * Sets the root DbEntity used to resolve each column's {@link DbAttribute} by name. Optional - when not set, + * column attributes are left as provided. + */ + public RowBuilder dbEntity(DbEntity dbEntity) { + this.dbEntity = dbEntity; + return this; + } } } diff --git a/cayenne/src/main/java/org/apache/cayenne/access/jdbc/SQLTemplateAction.java b/cayenne/src/main/java/org/apache/cayenne/access/jdbc/SQLTemplateAction.java index b9d5de054..e9d50d11d 100644 --- a/cayenne/src/main/java/org/apache/cayenne/access/jdbc/SQLTemplateAction.java +++ b/cayenne/src/main/java/org/apache/cayenne/access/jdbc/SQLTemplateAction.java @@ -234,15 +234,18 @@ public class SQLTemplateAction implements SQLAction { } @SuppressWarnings({"unchecked", "rawtypes"}) - protected void processSelectResult(TranslatedSQL compiled, Connection connection, Statement statement, - ResultSet resultSet, OperationObserver callback, final long startTime) throws Exception { + protected void processSelectResult( + TranslatedSQL compiled, + Connection connection, Statement statement, + ResultSet resultSet, + OperationObserver callback, + long startTime) throws Exception { - boolean iteratedResult = callback.isIteratedResult(); - ExtendedTypeMap types = dataNode.getAdapter().getExtendedTypes(); - RSColumn.RowBuilder rowBuilder = rowBuilder(compiled, resultSet); recreateQueryMetadata(resultSet); - RowReader<?> rowReader = dataNode.getRowReaderFactory() - .rowReader(rowBuilder.build(types), queryMetadata, dataNode.getAdapter()); + boolean iteratedResult = callback.isIteratedResult(); + RSColumn[] columns = rowBuilder(compiled, resultSet).build(dataNode.getAdapter().getExtendedTypes()); + + RowReader<?> rowReader = dataNode.getRowReaderFactory().rowReader(columns, queryMetadata, dataNode.getAdapter()); ResultIterator<?> it = new RSIterator<>(statement, resultSet, rowReader); if (iteratedResult) { @@ -317,12 +320,14 @@ public class SQLTemplateAction implements SQLAction { /** * @since 3.0 */ - protected RSColumn.RowBuilder rowBuilder(TranslatedSQL compiled, ResultSet resultSet) - throws SQLException { + protected RSColumn.RowBuilder rowBuilder(TranslatedSQL compiled, ResultSet resultSet) throws SQLException { RSColumn.RowBuilder builder = RSColumn.rowBuilder() .resultSet(resultSet) .columns(createColumnDescriptors(compiled)) - .validateDuplicateColumnNames(); + .validateDuplicateColumnNames() + // resolve column DbAttributes so the row reader factory can tell e.g. PK columns apart, the same way + // it can for ObjectSelect; lets pagination read the PK regardless of column order + .dbEntity(dbEntity); if (query.getResultColumnsTypes() != null) { builder.mergeColumnsWithRsMetadata(); diff --git a/cayenne/src/main/java/org/apache/cayenne/access/jdbc/reader/DefaultRowReaderFactory.java b/cayenne/src/main/java/org/apache/cayenne/access/jdbc/reader/DefaultRowReaderFactory.java index 47bd4f07e..2db68028f 100644 --- a/cayenne/src/main/java/org/apache/cayenne/access/jdbc/reader/DefaultRowReaderFactory.java +++ b/cayenne/src/main/java/org/apache/cayenne/access/jdbc/reader/DefaultRowReaderFactory.java @@ -18,14 +18,20 @@ ****************************************************************/ package org.apache.cayenne.access.jdbc.reader; +import org.apache.cayenne.CayenneRuntimeException; import org.apache.cayenne.access.jdbc.RSColumn; import org.apache.cayenne.dba.DbAdapter; +import org.apache.cayenne.map.DbAttribute; +import org.apache.cayenne.map.DbEntity; +import org.apache.cayenne.map.ObjEntity; import org.apache.cayenne.query.EmbeddableResultSegment; import org.apache.cayenne.query.EntityResultSegment; import org.apache.cayenne.query.QueryMetadata; import org.apache.cayenne.query.ScalarResultSegment; +import java.util.HashSet; import java.util.List; +import java.util.Set; /** * @since 4.0 @@ -95,14 +101,15 @@ public class DefaultRowReaderFactory implements RowReaderFactory { protected RowReader<?> entitySegmentReader(RSColumn[] columns, QueryMetadata metadata, EntityResultSegment segment) { - // For a paginated query the result columns are trimmed to the root PK (see IdColumnExtractor). A single-column - // PK is read as a scalar (consumed by SimpleIdIncrementalFaultList); a compound PK falls through to the regular - // DataRow reader below, which over PK-only columns yields exactly the id map IncrementalFaultList expects. - if (metadata.getPageSize() > 0 - && segment.getClassDescriptor().getEntity().getDbEntity().getPrimaryKeys().size() == 1) { - int pk = segment.getColumnOffset(); - // jdbc column indexes start from 1 - return new ScalarRowReader<>(columns[pk].reader(), pk + 1, columns[pk].rsType()); + // For a paginated query only the root PK is read into the IncrementalFaultList - locate the PK columns within + // this segment by their DbAttribute (see idReader). + if (metadata.getPageSize() > 0) { + ObjEntity objEntity = segment.getClassDescriptor().getEntity(); + return idReader(columns, + segment.getColumnOffset(), + segment.getColumnOffset() + columns.length, + objEntity.getDbEntity(), + objEntity.getName()); } int startIndex = segment.getColumnOffset(); @@ -133,8 +140,57 @@ public class DefaultRowReaderFactory implements RowReaderFactory { } protected RowReader<?> noSegmentReader(RSColumn[] columns, QueryMetadata metadata) { - return metadata.getPageSize() > 0 && metadata.getDbEntity().getPrimaryKeys().size() == 1 - ? new ScalarRowReader<>(columns[0].reader(), 1, columns[0].rsType()) - : FullRowReader.of(columns, metadata); + if (metadata.getPageSize() > 0) { + ObjEntity objEntity = metadata.getObjEntity(); + return idReader(columns, 0, columns.length, metadata.getDbEntity(), + objEntity != null ? objEntity.getName() : null); + } + return FullRowReader.of(columns, metadata); } + + private RowReader<?> idReader(RSColumn[] columns, int from, int to, DbEntity dbEntity, String entityName) { + if (dbEntity == null) { + throw new CayenneRuntimeException("Null root DbEntity, can't index PK"); + } + + int pkLen = dbEntity.getPrimaryKeys().size(); + if (pkLen == 0) { + throw new CayenneRuntimeException("Root DbEntity has no PK defined: %s", dbEntity.getName()); + } + + RSColumn[] pkColumns = new RSColumn[pkLen]; + int[] pkIndexes = new int[pkLen]; + int found = 0; + + Set<DbAttribute> seen = new HashSet<>(); + for (int i = from; i < to && found < pkLen; i++) { + DbAttribute attribute = columns[i].attribute(); + if (attribute != null && attribute.isPrimaryKey() && seen.add(attribute)) { + pkColumns[found] = columns[i]; + pkIndexes[found] = i + 1; + found++; + } + } + + if (found != pkLen) { + // TODO: HACK: the result columns don't carry resolvable PK DbAttributes (these are most likely aliased + // EJBQL columns). Fall back to the legacy assumption that the PK is the first pkLen columns starting at + // 'from'. + if (from + pkLen > columns.length) { + throw new CayenneRuntimeException( + "Result set for paginated query is missing PK column(s) of entity '%s'; expected %s", + entityName, dbEntity.getPrimaryKeys().stream().map(DbAttribute::getName).toList()); + } + for (int i = 0; i < pkLen; i++) { + pkColumns[i] = columns[from + i]; + // jdbc column indexes start from 1 + pkIndexes[i] = from + i + 1; + } + } + + return pkLen == 1 + ? new ScalarRowReader<>(pkColumns[0].reader(), pkIndexes[0], pkColumns[0].rsType()) + : IndexRowReader.of(pkColumns, pkIndexes, entityName); + } + } diff --git a/cayenne/src/main/java/org/apache/cayenne/access/jdbc/reader/IndexRowReader.java b/cayenne/src/main/java/org/apache/cayenne/access/jdbc/reader/IndexRowReader.java new file mode 100644 index 000000000..add339f66 --- /dev/null +++ b/cayenne/src/main/java/org/apache/cayenne/access/jdbc/reader/IndexRowReader.java @@ -0,0 +1,69 @@ +/***************************************************************** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + ****************************************************************/ +package org.apache.cayenne.access.jdbc.reader; + +import org.apache.cayenne.CayenneRuntimeException; +import org.apache.cayenne.DataRow; +import org.apache.cayenne.access.jdbc.RSColumn; + +import java.sql.ResultSet; + +/** + * A {@link RowReader} that materializes a DataRow from a set of columns read at explicit, possibly non-contiguous, + * result-set positions. Unlike {@link OffsetRowReader}, each column carries its own position, so the columns need not + * form a contiguous run. + */ +class IndexRowReader implements RowReader<DataRow> { + + private final RSColumn[] columns; + private final int[] jdbcIndexes; + private final String entityName; + private final int mapCapacity; + + static RowReader<DataRow> of(RSColumn[] columns, int[] jdbcIndexes, String entityName) { + return new IndexRowReader(columns, jdbcIndexes, entityName); + } + + private IndexRowReader(RSColumn[] columns, int[] jdbcIndexes, String entityName) { + this.columns = columns; + this.jdbcIndexes = jdbcIndexes; + this.entityName = entityName; + this.mapCapacity = (int) Math.ceil(columns.length / 0.75); + } + + @Override + public DataRow readRow(ResultSet resultSet) { + DataRow dataRow = new DataRow(mapCapacity); + + try { + for (int i = 0; i < columns.length; i++) { + RSColumn column = columns[i]; + Object val = column.reader().materializeObject(resultSet, jdbcIndexes[i], column.rsType()); + dataRow.put(column.dataRowName(), val); + } + + dataRow.setEntityName(entityName); + return dataRow; + } catch (CayenneRuntimeException ex) { + throw ex; + } catch (Exception ex) { + throw new CayenneRuntimeException("Exception materializing column.", ex); + } + } +} diff --git a/cayenne/src/test/java/org/apache/cayenne/CayenneCompoundIT.java b/cayenne/src/test/java/org/apache/cayenne/CayenneCompoundIT.java index 421ab68f7..145d14a50 100644 --- a/cayenne/src/test/java/org/apache/cayenne/CayenneCompoundIT.java +++ b/cayenne/src/test/java/org/apache/cayenne/CayenneCompoundIT.java @@ -20,8 +20,11 @@ package org.apache.cayenne; import org.apache.cayenne.exp.property.PropertyFactory; +import org.apache.cayenne.query.CapsStrategy; import org.apache.cayenne.query.EJBQLQuery; import org.apache.cayenne.query.ObjectSelect; +import org.apache.cayenne.query.SQLSelect; +import org.apache.cayenne.query.SQLTemplate; import org.apache.cayenne.test.jdbc.TableHelper; import org.apache.cayenne.testdo.compound.CharPkTestEntity; import org.apache.cayenne.testdo.compound.CompoundPkTestEntity; @@ -179,6 +182,49 @@ public class CayenneCompoundIT { assertEquals(4, queriesCount); } + @Test + public void pageSizeCompoundPkSQLTemplate() throws Exception { + createCompoundPKs(20); + + int pageSize = 7; + + // the compound PK columns (KEY1, KEY2) are NOT the first selected columns and are read as a + // DataRow id map by the paginated id reader + SQLTemplate query = new SQLTemplate(CompoundPkTestEntity.class, + "SELECT NAME, KEY1, KEY2 FROM COMPOUND_PK_TEST ORDER BY NAME"); + query.setColumnNamesCapitalization(CapsStrategy.UPPER); + query.setPageSize(pageSize); + + List<?> result = env.context().performQuery(query); + assertEquals(20, result.size()); + + // resolve every page, including subsequent pages faulted in by compound id + for (int i = 0; i < result.size(); i++) { + CompoundPkTestEntity e = (CompoundPkTestEntity) result.get(i); + assertEquals(String.format("BBB%02d", i), e.getName()); + } + } + + @Test + public void pageSizeCompoundPkSQLSelect() throws Exception { + createCompoundPKs(20); + + int pageSize = 7; + + // SQLSelect builds a replacement SQLTemplate, so it exercises the same compound-PK paginated id reader + SQLSelect<CompoundPkTestEntity> query = SQLSelect + .query(CompoundPkTestEntity.class, "SELECT NAME, KEY1, KEY2 FROM COMPOUND_PK_TEST ORDER BY NAME") + .columnNameCaps(CapsStrategy.UPPER) + .pageSize(pageSize); + + List<CompoundPkTestEntity> result = query.select(env.context()); + assertEquals(20, result.size()); + + for (int i = 0; i < result.size(); i++) { + assertEquals(String.format("BBB%02d", i), result.get(i).getName()); + } + } + @Test public void ejbqlCountSelect() throws Exception { tCompoundIntPKTest.insert(1, 2, "test"); diff --git a/cayenne/src/test/java/org/apache/cayenne/access/DataContextIT.java b/cayenne/src/test/java/org/apache/cayenne/access/DataContextIT.java index 28eaccd7e..516118393 100644 --- a/cayenne/src/test/java/org/apache/cayenne/access/DataContextIT.java +++ b/cayenne/src/test/java/org/apache/cayenne/access/DataContextIT.java @@ -443,7 +443,7 @@ public class DataContextIT { } @Test - public void performPaginatedQuery1() throws Exception { + public void performPaginatedQuery_EJBQL() throws Exception { createArtistsDataSet(); EJBQLQuery query = new EJBQLQuery("select a FROM Artist a"); diff --git a/cayenne/src/test/java/org/apache/cayenne/access/DataContextSQLTemplateIT.java b/cayenne/src/test/java/org/apache/cayenne/access/DataContextSQLTemplateIT.java index 903b4eb07..942cb5a3d 100644 --- a/cayenne/src/test/java/org/apache/cayenne/access/DataContextSQLTemplateIT.java +++ b/cayenne/src/test/java/org/apache/cayenne/access/DataContextSQLTemplateIT.java @@ -27,6 +27,7 @@ import org.apache.cayenne.map.DataMap; import org.apache.cayenne.map.EntityResult; import org.apache.cayenne.map.SQLResult; import org.apache.cayenne.query.CapsStrategy; +import org.apache.cayenne.query.SQLSelect; import org.apache.cayenne.query.SQLTemplate; import org.apache.cayenne.test.jdbc.TableHelper; import org.apache.cayenne.testdo.testmap.Artist; @@ -496,6 +497,50 @@ public class DataContextSQLTemplateIT { assertEquals("artist" + (pageSize + 2), artist.getArtistName()); } + @Test + public void pageSizePkNotFirstColumn() throws Exception { + createFourArtists(); + + int pageSize = 3; + + // the PK is the SECOND result column - exercises the paginated id reader's column indexing + String template = "SELECT ARTIST_NAME, ARTIST_ID FROM ARTIST ORDER BY ARTIST_ID"; + SQLTemplate query = sqlTemplateCustomizer.createSQLTemplate(Artist.class, template); + + query.setPageSize(pageSize); + + List<?> objects = context.performQuery(query); + + assertEquals(4, objects.size()); + + // resolving the first page must materialize the right rows by their actual PK + assertTrue(objects.get(0) instanceof Artist); + assertEquals("artist2", ((Artist) objects.get(0)).getArtistName()); + + // resolving a subsequent page exercises id-to-object resolution from the stored ids + Artist artist = (Artist) objects.get(pageSize); + assertEquals("artist" + (pageSize + 2), artist.getArtistName()); + } + + @Test + public void pageSizeSQLSelectPkNotFirstColumn() throws Exception { + createFourArtists(); + + int pageSize = 3; + + // SQLSelect builds a replacement SQLTemplate, so it goes through the same paginated id reader + SQLSelect<Artist> query = SQLSelect + .query(Artist.class, "SELECT ARTIST_NAME, ARTIST_ID FROM ARTIST ORDER BY ARTIST_ID") + .columnNameCaps(CapsStrategy.UPPER) + .pageSize(pageSize); + + List<Artist> objects = query.select(context); + + assertEquals(4, objects.size()); + assertEquals("artist2", objects.get(0).getArtistName()); + assertEquals("artist" + (pageSize + 2), objects.get(pageSize).getArtistName()); + } + @Test public void iteratedQuery() throws Exception { createFourArtists();
