Skip to content

Commit 401d18f

Browse files
authored
Add streamingRequiresTransaction to correctly apply fetch size. Add tests. (#76)
1 parent d197ea1 commit 401d18f

10 files changed

Lines changed: 337 additions & 24 deletions

File tree

docs/queries.md

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1029,8 +1029,8 @@ Inline records expand in GROUP BY the same way. This is particularly useful in c
10291029
```kotlin
10301030
data class CityOrderCount(val city: City, val count: Long)
10311031

1032-
val page = orm.query(Order::class)
1033-
.select(Order_.city, "COUNT(*)")
1032+
val orders = orm.entity(Order::class)
1033+
val slice = orders.select(CityOrderCount::class) { "${City::class}, COUNT(*)" }
10341034
.groupBy(Order_.city)
10351035
.slice(Order_.city.key(), 20)
10361036
```
@@ -1121,7 +1121,7 @@ Optional<User> user = orm.entity(User.class)
11211121
## Tips
11221122

11231123
1. **Use the metamodel** -- `User_.email` catches typos at compile time; see [Metamodel](metamodel.md)
1124-
2. **Kotlin: choose your style** -- quick queries (`orm.find`, `orm.findAll`) for simple cases, repository builder for complex operations
1124+
2. **Kotlin: choose your style** -- quick queries (`orm.find`, `orm.findAll`) for simple cases, query builder for complex operations
11251125
3. **Java: DSL or Templates** -- DSL for type-safe conditions, SQL Templates for complex SQL like CTEs, window functions, or database-specific features
11261126
4. **Entity graphs load in one query** -- related entities marked with `@FK` are JOINed automatically, no N+1 problems
11271127
5. **Close Java streams** -- always use try-with-resources with `Stream` results

storm-core/src/main/java/st/orm/core/template/SqlDialect.java

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -493,4 +493,19 @@ default boolean streamOnlyFetchSize() {
493493
return false;
494494
}
495495

496+
/**
497+
* Returns whether streaming result sets require an active transaction for the fetch size hint to take effect.
498+
*
499+
* <p>Some JDBC drivers silently ignore {@link PreparedStatement#setFetchSize(int)} when the connection is in
500+
* auto-commit mode. When this method returns {@code true} and a streaming method ({@code getResultStream()}) is
501+
* called without an active transaction, a warning is logged to alert the caller that the entire result set will
502+
* be buffered in memory instead of being fetched in batches.</p>
503+
*
504+
* @return {@code true} if an active transaction is required for cursor-based streaming.
505+
* @since 1.10
506+
*/
507+
default boolean streamingRequiresTransaction() {
508+
return false;
509+
}
510+
496511
}

storm-core/src/main/java/st/orm/core/template/impl/PreparedQueryImpl.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,8 +48,9 @@ public PreparedQueryImpl(@Nonnull RefFactory refFactory,
4848
boolean managed,
4949
int defaultFetchSize,
5050
boolean streamOnlyFetchSize,
51+
boolean streamingRequiresTransaction,
5152
@Nonnull Function<Throwable, PersistenceException> exceptionTransformer) {
52-
super(refFactory, ignore -> statement, bindVarsHandle, affectedType, versionAware, managed, false, defaultFetchSize, streamOnlyFetchSize, exceptionTransformer);
53+
super(refFactory, ignore -> statement, bindVarsHandle, affectedType, versionAware, managed, false, defaultFetchSize, streamOnlyFetchSize, streamingRequiresTransaction, exceptionTransformer);
5354
this.refFactory = refFactory;
5455
this.statement = statement;
5556
this.bindVarsHandle = bindVarsHandle;

storm-core/src/main/java/st/orm/core/template/impl/PreparedStatementTemplateImpl.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -507,7 +507,7 @@ public Query create(@Nonnull TemplateString template) {
507507
} catch (SQLException e) {
508508
throw new PersistenceException(e);
509509
}
510-
}, bindVariables == null ? null : bindVariables.getHandle(), sql.affectedType().orElse(null), sql.versionAware(), false, false, dialect.defaultFetchSize(), dialect.streamOnlyFetchSize(), getExceptionTransformer(sql));
510+
}, bindVariables == null ? null : bindVariables.getHandle(), sql.affectedType().orElse(null), sql.versionAware(), false, false, dialect.defaultFetchSize(), dialect.streamOnlyFetchSize(), dialect.streamingRequiresTransaction(), getExceptionTransformer(sql));
511511
} catch (SqlTemplateException e) {
512512
throw new PersistenceException(e);
513513
}

storm-core/src/main/java/st/orm/core/template/impl/QueryImpl.java

Lines changed: 59 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -71,14 +71,15 @@ class QueryImpl implements Query {
7171
private final boolean managed;
7272
private final int defaultFetchSize;
7373
private final boolean streamOnlyFetchSize;
74+
private final boolean streamingRequiresTransaction;
7475
private final Function<Throwable, PersistenceException> exceptionTransformer;
7576

7677
QueryImpl(@Nonnull RefFactory refFactory,
7778
@Nonnull Function<Boolean, PreparedStatement> statement,
7879
@Nullable BindVarsHandle bindVarsHandle,
7980
boolean versionAware,
8081
@Nonnull Function<Throwable, PersistenceException> exceptionTransformer) {
81-
this(refFactory, statement, bindVarsHandle, null, versionAware, false, false, 0, false, exceptionTransformer);
82+
this(refFactory, statement, bindVarsHandle, null, versionAware, false, false, 0, false, false, exceptionTransformer);
8283
}
8384

8485
QueryImpl(@Nonnull RefFactory refFactory,
@@ -87,7 +88,7 @@ class QueryImpl implements Query {
8788
boolean versionAware,
8889
boolean unsafe,
8990
@Nonnull Function<Throwable, PersistenceException> exceptionTransformer) {
90-
this(refFactory, statement, bindVarsHandle, null, versionAware, false, unsafe, 0, false, exceptionTransformer);
91+
this(refFactory, statement, bindVarsHandle, null, versionAware, false, unsafe, 0, false, false, exceptionTransformer);
9192
}
9293

9394
QueryImpl(@Nonnull RefFactory refFactory,
@@ -97,7 +98,7 @@ class QueryImpl implements Query {
9798
boolean versionAware,
9899
boolean unsafe,
99100
@Nonnull Function<Throwable, PersistenceException> exceptionTransformer) {
100-
this(refFactory, statement, bindVarsHandle, affectedType, versionAware, false, unsafe, 0, false, exceptionTransformer);
101+
this(refFactory, statement, bindVarsHandle, affectedType, versionAware, false, unsafe, 0, false, false, exceptionTransformer);
101102
}
102103

103104
QueryImpl(@Nonnull RefFactory refFactory,
@@ -108,7 +109,7 @@ class QueryImpl implements Query {
108109
boolean managed,
109110
boolean unsafe,
110111
@Nonnull Function<Throwable, PersistenceException> exceptionTransformer) {
111-
this(refFactory, statement, bindVarsHandle, affectedType, versionAware, managed, unsafe, 0, false, exceptionTransformer);
112+
this(refFactory, statement, bindVarsHandle, affectedType, versionAware, managed, unsafe, 0, false, false, exceptionTransformer);
112113
}
113114

114115
QueryImpl(@Nonnull RefFactory refFactory,
@@ -120,6 +121,7 @@ class QueryImpl implements Query {
120121
boolean unsafe,
121122
int defaultFetchSize,
122123
boolean streamOnlyFetchSize,
124+
boolean streamingRequiresTransaction,
123125
@Nonnull Function<Throwable, PersistenceException> exceptionTransformer) {
124126
this.refFactory = refFactory;
125127
this.statement = statement;
@@ -130,6 +132,7 @@ class QueryImpl implements Query {
130132
this.unsafe = unsafe;
131133
this.defaultFetchSize = defaultFetchSize;
132134
this.streamOnlyFetchSize = streamOnlyFetchSize;
135+
this.streamingRequiresTransaction = streamingRequiresTransaction;
133136
this.exceptionTransformer = exceptionTransformer;
134137
}
135138

@@ -147,7 +150,7 @@ class QueryImpl implements Query {
147150
*/
148151
@Override
149152
public PreparedQuery prepare() {
150-
return MonitoredResource.wrap(new PreparedQueryImpl(refFactory, statement.apply(unsafe), bindVarsHandle, affectedType, versionAware, managed, defaultFetchSize, streamOnlyFetchSize, exceptionTransformer));
153+
return MonitoredResource.wrap(new PreparedQueryImpl(refFactory, statement.apply(unsafe), bindVarsHandle, affectedType, versionAware, managed, defaultFetchSize, streamOnlyFetchSize, streamingRequiresTransaction, exceptionTransformer));
151154
}
152155

153156
/**
@@ -158,7 +161,7 @@ public PreparedQuery prepare() {
158161
*/
159162
@Override
160163
public Query managed() {
161-
return new QueryImpl(refFactory, statement, bindVarsHandle, affectedType, versionAware, true, unsafe, defaultFetchSize, streamOnlyFetchSize, exceptionTransformer);
164+
return new QueryImpl(refFactory, statement, bindVarsHandle, affectedType, versionAware, true, unsafe, defaultFetchSize, streamOnlyFetchSize, streamingRequiresTransaction, exceptionTransformer);
162165
}
163166

164167
/**
@@ -169,11 +172,11 @@ public Query managed() {
169172
*/
170173
@Override
171174
public Query unsafe() {
172-
return new QueryImpl(refFactory, statement, bindVarsHandle, affectedType, versionAware, managed, true, defaultFetchSize, streamOnlyFetchSize, exceptionTransformer);
175+
return new QueryImpl(refFactory, statement, bindVarsHandle, affectedType, versionAware, managed, true, defaultFetchSize, streamOnlyFetchSize, streamingRequiresTransaction, exceptionTransformer);
173176
}
174177

175178
private QueryImpl withoutFetchSize() {
176-
return new QueryImpl(refFactory, statement, bindVarsHandle, affectedType, versionAware, managed, unsafe, 0, false, exceptionTransformer);
179+
return new QueryImpl(refFactory, statement, bindVarsHandle, affectedType, versionAware, managed, unsafe, 0, false, false, exceptionTransformer);
177180
}
178181

179182
private PreparedStatement getStatement() {
@@ -186,6 +189,38 @@ private void applyFetchSize(@Nonnull PreparedStatement statement) throws SQLExce
186189
}
187190
}
188191

192+
/**
193+
* Configures the connection for cursor-based streaming when the dialect requires an active transaction.
194+
*
195+
* <p>If the connection is in auto-commit mode and the dialect indicates that streaming requires a transaction,
196+
* auto-commit is disabled to enable cursor-based result batching. The returned {@code Runnable} restores the
197+
* connection to its original state when the stream is closed.</p>
198+
*
199+
* @param statement the prepared statement whose connection to configure.
200+
* @return a cleanup action that restores auto-commit, or {@code null} if no configuration was needed.
201+
*/
202+
private @Nullable Runnable configureStreamingTransaction(@Nonnull PreparedStatement statement) {
203+
if (streamingRequiresTransaction && defaultFetchSize != 0) {
204+
try {
205+
var connection = statement.getConnection();
206+
if (connection.getAutoCommit()) {
207+
connection.setAutoCommit(false);
208+
return () -> {
209+
try {
210+
connection.commit();
211+
connection.setAutoCommit(true);
212+
} catch (SQLException e) {
213+
throw new PersistenceException(e);
214+
}
215+
};
216+
}
217+
} catch (SQLException ignore) {
218+
// Unable to determine or change auto-commit state; proceed without cursor-based streaming.
219+
}
220+
}
221+
return null;
222+
}
223+
189224
protected boolean closeStatement() {
190225
return true;
191226
}
@@ -216,6 +251,7 @@ public Stream<Object[]> getResultStream() {
216251
boolean close = true;
217252
try {
218253
applyFetchSize(statement);
254+
Runnable streamingCleanup = configureStreamingTransaction(statement);
219255
ResultSet resultSet = statement.executeQuery();
220256
try {
221257
int columnCount = resultSet.getMetaData().getColumnCount();
@@ -229,7 +265,7 @@ public Stream<Object[]> getResultStream() {
229265
}
230266
})
231267
.takeWhile(Objects::nonNull)
232-
.onClose(() -> close(resultSet, statement)));
268+
.onClose(() -> close(resultSet, statement, streamingCleanup)));
233269
} finally {
234270
if (close) {
235271
resultSet.close();
@@ -271,6 +307,7 @@ public <T> Stream<T> getResultStream(@Nonnull Class<T> type) {
271307
try {
272308
try {
273309
applyFetchSize(statement);
310+
Runnable streamingCleanup = configureStreamingTransaction(statement);
274311
ResultSet resultSet = statement.executeQuery();
275312
int columnCount = resultSet.getMetaData().getColumnCount();
276313
var mapper = getObjectMapper(columnCount, type, refFactory)
@@ -285,7 +322,7 @@ public <T> Stream<T> getResultStream(@Nonnull Class<T> type) {
285322
}
286323
})
287324
.takeWhile(Objects::nonNull)
288-
.onClose(() -> close(resultSet, statement)));
325+
.onClose(() -> close(resultSet, statement, streamingCleanup)));
289326
} finally {
290327
if (close && closeStatement()) {
291328
statement.close();
@@ -377,9 +414,20 @@ public long getResultCount() {
377414
}
378415

379416
protected void close(@Nonnull ResultSet resultSet, @Nonnull PreparedStatement statement) {
417+
close(resultSet, statement, null);
418+
}
419+
420+
protected void close(@Nonnull ResultSet resultSet, @Nonnull PreparedStatement statement,
421+
@Nullable Runnable streamingCleanup) {
380422
try {
381423
try {
382-
resultSet.close();
424+
try {
425+
resultSet.close();
426+
} finally {
427+
if (streamingCleanup != null) {
428+
streamingCleanup.run();
429+
}
430+
}
383431
} finally {
384432
if (closeStatement()) {
385433
statement.close();

storm-core/src/test/java/st/orm/core/template/impl/FetchSizeTest.java

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,14 @@ private QueryImpl createQueryWithFetchSize(Connection connection,
6464
String sql,
6565
int defaultFetchSize,
6666
boolean streamOnlyFetchSize) {
67+
return createQueryWithFetchSize(connection, sql, defaultFetchSize, streamOnlyFetchSize, false);
68+
}
69+
70+
private QueryImpl createQueryWithFetchSize(Connection connection,
71+
String sql,
72+
int defaultFetchSize,
73+
boolean streamOnlyFetchSize,
74+
boolean streamingRequiresTransaction) {
6775
return new QueryImpl(
6876
DETACHED_REF_FACTORY,
6977
unsafe -> {
@@ -80,6 +88,7 @@ private QueryImpl createQueryWithFetchSize(Connection connection,
8088
false,
8189
defaultFetchSize,
8290
streamOnlyFetchSize,
91+
streamingRequiresTransaction,
8392
e -> new PersistenceException(e)
8493
);
8594
}
@@ -242,4 +251,47 @@ public void testPreparePassesFetchSizeToPreparedQuery() throws Exception {
242251
assertEquals(6, count);
243252
}
244253
}
254+
255+
@Test
256+
public void testStreamingRequiresTransaction_disablesAutoCommitDuringStream() throws Exception {
257+
try (Connection connection = dataSource.getConnection()) {
258+
assertTrue(connection.getAutoCommit());
259+
QueryImpl query = createQueryWithFetchSize(connection, "SELECT id FROM city WHERE id = 1", 100, false, true);
260+
try (Stream<Object[]> stream = query.getResultStream()) {
261+
Object[] row = stream.findFirst().orElseThrow();
262+
assertNotNull(row[0]);
263+
}
264+
// Auto-commit should be restored after the stream is closed.
265+
assertTrue(connection.getAutoCommit(), "Auto-commit should be restored after stream close");
266+
}
267+
}
268+
269+
@Test
270+
public void testStreamingRequiresTransaction_noChangeWhenAlreadyInTransaction() throws Exception {
271+
try (Connection connection = dataSource.getConnection()) {
272+
connection.setAutoCommit(false);
273+
QueryImpl query = createQueryWithFetchSize(connection, "SELECT id FROM city WHERE id = 1", 100, false, true);
274+
try (Stream<Object[]> stream = query.getResultStream()) {
275+
Object[] row = stream.findFirst().orElseThrow();
276+
assertNotNull(row[0]);
277+
}
278+
// Auto-commit should remain false (unchanged).
279+
assertTrue(!connection.getAutoCommit(), "Auto-commit should remain false when already in a transaction");
280+
connection.rollback();
281+
}
282+
}
283+
284+
@Test
285+
public void testStreamingRequiresTransaction_noChangeWhenFlagDisabled() throws Exception {
286+
try (Connection connection = dataSource.getConnection()) {
287+
assertTrue(connection.getAutoCommit());
288+
QueryImpl query = createQueryWithFetchSize(connection, "SELECT id FROM city WHERE id = 1", 100, false, false);
289+
try (Stream<Object[]> stream = query.getResultStream()) {
290+
Object[] row = stream.findFirst().orElseThrow();
291+
assertNotNull(row[0]);
292+
}
293+
// Auto-commit should remain true (unchanged).
294+
assertTrue(connection.getAutoCommit(), "Auto-commit should remain true when flag is disabled");
295+
}
296+
}
245297
}

0 commit comments

Comments
 (0)