Skip to content

Commit d23dff8

Browse files
gavinkinggavinking
committed
HHH-19266 fix multiple bugs and inconsistencies in ScrollableResults impls
1. fix setRowNumber(-n) 2. number rows from 1 instead of 0 3. fix position() Item 2 is a change to the documented behavior, but the documented behavior was already inconsistently implemented.
1 parent b485b74 commit d23dff8

File tree

6 files changed

+197
-39
lines changed

6 files changed

+197
-39
lines changed

‎hibernate-core/src/main/java/org/hibernate/ScrollMode.java

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@
1111
* to use underneath a {@link ScrollableResults}.
1212
*
1313
* @author Gavin King
14+
*
15+
* @see org.hibernate.query.SelectionQuery#scroll(ScrollMode)
1416
*/
1517
public enum ScrollMode {
1618
/**

‎hibernate-core/src/main/java/org/hibernate/ScrollableResults.java

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -8,15 +8,15 @@
88

99
/**
1010
* A result iterator that allows moving around within the results by
11-
* arbitrary increments. The {@link Query} / {@link ScrollableResults}
12-
* pattern is very similar to the JDBC {@link java.sql.PreparedStatement}/
11+
* arbitrary increments.
12+
*
13+
* @apiNote The {@link Query} / {@link ScrollableResults} pattern is
14+
* very similar to the JDBC {@link java.sql.PreparedStatement} /
1315
* {@link java.sql.ResultSet} pattern and so the semantics of methods
1416
* of this interface are similar to the similarly-named methods of
1517
* {@code ResultSet}.
16-
* <p>
17-
* Contrary to JDBC, columns of results are numbered from zero.
1818
*
19-
* @see Query#scroll()
19+
* @see org.hibernate.query.SelectionQuery#scroll()
2020
*
2121
* @author Gavin King
2222
*/
@@ -115,16 +115,18 @@ public interface ScrollableResults<R> extends AutoCloseable {
115115
/**
116116
* Get the current position in the results.
117117
* <p>
118-
* The first position is number 0 (unlike JDBC).
118+
* The first position is row number 1.
119119
*
120-
* @return The current position number, numbered from 0;
120+
* @return The current position number, numbered from 1;
121121
* -1 indicates that there is no current row
122122
*/
123123
int getRowNumber();
124124

125125
/**
126126
* Set the current position in the result set.
127127
* <p>
128+
* The first position is row number 1.
129+
* <p>
128130
* Can be numbered from the first result (positive number)
129131
* or backward from the last result (negative number).
130132
*

‎hibernate-core/src/main/java/org/hibernate/internal/FetchingScrollableResultsImpl.java

Lines changed: 16 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@
1616
import org.hibernate.sql.results.spi.RowReader;
1717

1818
/**
19-
* Implementation of ScrollableResults which can handle collection fetches.
19+
* Implementation of {@link org.hibernate.ScrollableResults} which can handle collection fetches.
2020
*
2121
* @author Steve Ebersole
2222
*/
@@ -44,7 +44,7 @@ public FetchingScrollableResultsImpl(
4444
persistenceContext
4545
);
4646

47-
this.maxPosition = jdbcValuesSourceProcessingState.getQueryOptions().getEffectiveLimit().getMaxRows();
47+
maxPosition = jdbcValuesSourceProcessingState.getQueryOptions().getEffectiveLimit().getMaxRows();
4848
beforeFirst = true;
4949
}
5050

@@ -67,6 +67,7 @@ else if ( maxPosition != null && maxPosition <= currentPosition ) {
6767
}
6868
else if ( beforeFirst ) {
6969
if ( !getRowProcessingState().next() ) {
70+
// no rows to read
7071
currentPosition = 0;
7172
beforeFirst = false;
7273
return false;
@@ -197,14 +198,7 @@ else if ( positions < 0 ) {
197198

198199
@Override
199200
public boolean position(int position) {
200-
final boolean underlyingScrollSuccessful = getRowProcessingState().position( position );
201-
if ( !underlyingScrollSuccessful ) {
202-
currentRow = null;
203-
return false;
204-
205-
}
206-
currentPosition = position - 1;
207-
return next();
201+
return setRowNumber( position );
208202
}
209203

210204
@Override
@@ -239,7 +233,7 @@ public boolean last() {
239233
public boolean first() {
240234
beforeFirst();
241235
final boolean more = next();
242-
afterScrollOperation();
236+
// afterScrollOperation();
243237
return more;
244238
}
245239

@@ -281,13 +275,20 @@ public boolean setRowNumber(int rowNumber) {
281275
if ( rowNumber == 1 ) {
282276
return first();
283277
}
284-
else if ( rowNumber == -1 ) {
278+
else if ( rowNumber == -1 || maxPosition != null && rowNumber == maxPosition ) {
285279
return last();
286280
}
287-
else if ( maxPosition != null && rowNumber == maxPosition ) {
288-
return last();
281+
else if ( rowNumber < 0 && maxPosition == null ) {
282+
while ( next() ) {
283+
// skip all the way to the end (inefficiently)
284+
}
285+
return scroll( rowNumber );
286+
}
287+
else {
288+
// rowNumber -1 is the same as maxPosition
289+
final int targetRowNumber = rowNumber < 0 ? maxPosition + rowNumber + 1 : rowNumber;
290+
return scroll( targetRowNumber - currentPosition );
289291
}
290-
return scroll( rowNumber - currentPosition );
291292
}
292293

293294
private boolean prepareCurrentRow() {

‎hibernate-core/src/main/java/org/hibernate/internal/ScrollableResultsImpl.java

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@
44
*/
55
package org.hibernate.internal;
66

7-
import org.hibernate.HibernateException;
87
import org.hibernate.engine.spi.PersistenceContext;
98
import org.hibernate.engine.spi.SharedSessionContractImplementor;
109
import org.hibernate.sql.results.internal.RowProcessingStateStandardImpl;
@@ -15,7 +14,7 @@
1514
import org.hibernate.sql.results.spi.RowReader;
1615

1716
/**
18-
* Standard ScrollableResults implementation.
17+
* Standard {@link org.hibernate.ScrollableResults} implementation.
1918
*
2019
* @author Gavin King
2120
*/
@@ -107,12 +106,12 @@ public boolean isLast() {
107106
}
108107

109108
@Override
110-
public int getRowNumber() throws HibernateException {
111-
return getRowProcessingState().getPosition();
109+
public int getRowNumber() {
110+
return getRowProcessingState().getPosition() + 1;
112111
}
113112

114113
@Override
115-
public boolean setRowNumber(int rowNumber) throws HibernateException {
114+
public boolean setRowNumber(int rowNumber) {
116115
return position( rowNumber );
117116
}
118117

‎hibernate-core/src/main/java/org/hibernate/query/SelectionQuery.java

Lines changed: 7 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -153,21 +153,18 @@ default List<R> getResultList() {
153153
}
154154

155155
/**
156-
* Returns scrollable access to the query results.
157-
* <p>
158-
* This form calls {@link #scroll(ScrollMode)} using {@link Dialect#defaultScrollMode()}
156+
* Returns scrollable access to the query results, using the
157+
* {@linkplain Dialect#defaultScrollMode() default scroll mode
158+
* of the SQL dialect.}
159159
*
160-
* @apiNote The exact behavior of this method depends somewhat
161-
* on the JDBC driver's {@link java.sql.ResultSet} scrolling support
160+
* @see #scroll(ScrollMode)
162161
*/
163162
ScrollableResults<R> scroll();
164163

165164
/**
166-
* Returns scrollable access to the query results. The capabilities of the
167-
* returned ScrollableResults depend on the specified ScrollMode.
168-
*
169-
* @apiNote The exact behavior of this method depends somewhat
170-
* on the JDBC driver's {@link java.sql.ResultSet} scrolling support
165+
* Returns scrollable access to the query results. The capabilities
166+
* of the returned {@link ScrollableResults} depend on the specified
167+
* {@link ScrollMode}.
171168
*/
172169
ScrollableResults<R> scroll(ScrollMode scrollMode);
173170

‎hibernate-core/src/test/java/org/hibernate/orm/test/hql/ScrollableCollectionFetchingTest.java

Lines changed: 159 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -340,6 +340,66 @@ public void testScrollingJoinFetchesReverse(SessionFactoryScope scope) {
340340
);
341341
}
342342

343+
@Test
344+
@RequiresDialectFeature(feature = DialectFeatureChecks.SupportsTemporaryTable.class)
345+
@SkipForDialect(dialectClass = HANADialect.class, matchSubTypes = true, reason = "HANA only supports forward-only cursors.")
346+
public void testScrollingJoinFetchesWithNext(SessionFactoryScope scope) {
347+
TestData data = new TestData();
348+
data.prepare( scope );
349+
350+
scope.inTransaction(
351+
session -> {
352+
try (ScrollableResults results = session
353+
.createQuery("from Animal a left join fetch a.offspring where a.description like :desc order by a.id" )
354+
.setParameter( "desc", "root%" )
355+
.scroll()) {
356+
357+
assertTrue( results.next() );
358+
Animal animal = (Animal) results.get();
359+
assertEquals( data.root1Id, animal.getId(), "next() did not return expected row" );
360+
assertEquals( 1, results.getRowNumber() );
361+
362+
assertTrue( results.next() );
363+
animal = (Animal) results.get();
364+
assertEquals( data.root2Id, animal.getId(), "next() did not return expected row" );
365+
assertEquals( 2, results.getRowNumber() );
366+
367+
assertFalse( results.next() );
368+
}
369+
}
370+
);
371+
}
372+
373+
@Test
374+
@RequiresDialectFeature(feature = DialectFeatureChecks.SupportsTemporaryTable.class)
375+
@SkipForDialect(dialectClass = HANADialect.class, matchSubTypes = true, reason = "HANA only supports forward-only cursors.")
376+
public void testScrollingNoJoinFetchesWithNext(SessionFactoryScope scope) {
377+
TestData data = new TestData();
378+
data.prepare( scope );
379+
380+
scope.inTransaction(
381+
session -> {
382+
try (ScrollableResults results = session
383+
.createQuery("from Animal a where a.description like :desc order by a.id" )
384+
.setParameter( "desc", "root%" )
385+
.scroll()) {
386+
387+
assertTrue( results.next() );
388+
Animal animal = (Animal) results.get();
389+
assertEquals( data.root1Id, animal.getId(), "next() did not return expected row" );
390+
assertEquals( 1, results.getRowNumber() );
391+
392+
assertTrue( results.next() );
393+
animal = (Animal) results.get();
394+
assertEquals( data.root2Id, animal.getId(), "next() did not return expected row" );
395+
assertEquals( 2, results.getRowNumber() );
396+
397+
assertFalse( results.next() );
398+
}
399+
}
400+
);
401+
}
402+
343403
@Test
344404
@RequiresDialectFeature(feature = DialectFeatureChecks.SupportsTemporaryTable.class)
345405
@SkipForDialect(dialectClass = HANADialect.class, matchSubTypes = true, reason = "HANA only supports forward-only cursors.")
@@ -350,30 +410,127 @@ public void testScrollingJoinFetchesPositioning(SessionFactoryScope scope) {
350410
scope.inTransaction(
351411
session -> {
352412
try (ScrollableResults results = session
353-
.createQuery(
354-
"from Animal a left join fetch a.offspring where a.description like :desc order by a.id" )
413+
.createQuery("from Animal a left join fetch a.offspring where a.description like :desc order by a.id" )
414+
.setParameter( "desc", "root%" )
415+
.scroll()) {
416+
417+
results.first();
418+
Animal animal = (Animal) results.get();
419+
assertEquals( data.root1Id, animal.getId(), "first() did not return expected row" );
420+
assertEquals( 1, results.getRowNumber() );
421+
422+
results.scroll( 1 );
423+
animal = (Animal) results.get();
424+
assertEquals( data.root2Id, animal.getId(), "scroll(1) did not return expected row" );
425+
assertEquals( 2, results.getRowNumber() );
426+
427+
results.scroll( -1 );
428+
animal = (Animal) results.get();
429+
assertEquals( data.root1Id, animal.getId(), "scroll(-1) did not return expected row" );
430+
assertEquals( 1, results.getRowNumber() );
431+
432+
results.next();
433+
animal = (Animal) results.get();
434+
assertEquals( data.root2Id, animal.getId(), "next() did not return expected row" );
435+
assertEquals( 2, results.getRowNumber() );
436+
437+
results.setRowNumber( 1 );
438+
animal = (Animal) results.get();
439+
assertEquals( data.root1Id, animal.getId(), "setRowNumber(1) did not return expected row" );
440+
assertEquals( 1, results.getRowNumber() );
441+
442+
results.setRowNumber( 2 );
443+
animal = (Animal) results.get();
444+
assertEquals( data.root2Id, animal.getId(), "setRowNumber(2) did not return expected row" );
445+
assertEquals( 2, results.getRowNumber() );
446+
447+
results.setRowNumber( -2 );
448+
animal = (Animal) results.get();
449+
assertEquals( data.root1Id, animal.getId(), "setRowNumber(-2) did not return expected row" );
450+
assertEquals( 1, results.getRowNumber() );
451+
452+
results.setRowNumber( -1 );
453+
animal = (Animal) results.get();
454+
assertEquals( data.root2Id, animal.getId(), "setRowNumber(-1) did not return expected row" );
455+
assertEquals( 2, results.getRowNumber() );
456+
457+
results.position( 1 );
458+
animal = (Animal) results.get();
459+
assertEquals( data.root1Id, animal.getId(), "position(1) did not return expected row" );
460+
assertEquals( 1, results.getRowNumber() );
461+
462+
results.position( 2 );
463+
animal = (Animal) results.get();
464+
assertEquals( data.root2Id, animal.getId(), "position(2) did not return expected row" );
465+
assertEquals( 2, results.getRowNumber() );
466+
}
467+
}
468+
);
469+
}
470+
471+
@Test
472+
@RequiresDialectFeature(feature = DialectFeatureChecks.SupportsTemporaryTable.class)
473+
@SkipForDialect(dialectClass = HANADialect.class, matchSubTypes = true, reason = "HANA only supports forward-only cursors.")
474+
public void testScrollingNoJoinFetchesPositioning(SessionFactoryScope scope) {
475+
TestData data = new TestData();
476+
data.prepare( scope );
477+
478+
scope.inTransaction(
479+
session -> {
480+
try (ScrollableResults results = session
481+
.createQuery("from Animal a where a.description like :desc order by a.id" )
355482
.setParameter( "desc", "root%" )
356483
.scroll()) {
357484

358485
results.first();
359486
Animal animal = (Animal) results.get();
360487
assertEquals( data.root1Id, animal.getId(), "first() did not return expected row" );
488+
assertEquals( 1, results.getRowNumber() );
361489

362490
results.scroll( 1 );
363491
animal = (Animal) results.get();
364492
assertEquals( data.root2Id, animal.getId(), "scroll(1) did not return expected row" );
493+
assertEquals( 2, results.getRowNumber() );
365494

366495
results.scroll( -1 );
367496
animal = (Animal) results.get();
368497
assertEquals( data.root1Id, animal.getId(), "scroll(-1) did not return expected row" );
498+
assertEquals( 1, results.getRowNumber() );
499+
500+
results.next();
501+
animal = (Animal) results.get();
502+
assertEquals( data.root2Id, animal.getId(), "next() did not return expected row" );
503+
assertEquals( 2, results.getRowNumber() );
369504

370505
results.setRowNumber( 1 );
371506
animal = (Animal) results.get();
372507
assertEquals( data.root1Id, animal.getId(), "setRowNumber(1) did not return expected row" );
508+
assertEquals( 1, results.getRowNumber() );
373509

374510
results.setRowNumber( 2 );
375511
animal = (Animal) results.get();
376512
assertEquals( data.root2Id, animal.getId(), "setRowNumber(2) did not return expected row" );
513+
assertEquals( 2, results.getRowNumber() );
514+
515+
results.setRowNumber( -2 );
516+
animal = (Animal) results.get();
517+
assertEquals( data.root1Id, animal.getId(), "setRowNumber(-2) did not return expected row" );
518+
assertEquals( 1, results.getRowNumber() );
519+
520+
results.setRowNumber( -1 );
521+
animal = (Animal) results.get();
522+
assertEquals( data.root2Id, animal.getId(), "setRowNumber(-1) did not return expected row" );
523+
assertEquals( 2, results.getRowNumber() );
524+
525+
results.position( 1 );
526+
animal = (Animal) results.get();
527+
assertEquals( data.root1Id, animal.getId(), "position(1) did not return expected row" );
528+
assertEquals( 1, results.getRowNumber() );
529+
530+
results.position( 2 );
531+
animal = (Animal) results.get();
532+
assertEquals( data.root2Id, animal.getId(), "position(2) did not return expected row" );
533+
assertEquals( 2, results.getRowNumber() );
377534
}
378535
}
379536
);

0 commit comments

Comments
 (0)