Skip to content

Commit 7cfce06

Browse files
fredericDelaportefredericDelaporte
committed
Fix shortcomings of QueryBatcher initial implementation
 * AutoFlush is handled too late, causing stale data to be yielded * Cacheable result transformation is not fully done * Auto discovery of types fails * InitializeEntitiesAndCollections may be wrongly skipped or called * DoExecute should be renamed ExecuteBatched * BeginProcess should span the whole processing Follow up to #1718. See its reviews for more details on the above shortcomings.
1 parent 25777a3 commit 7cfce06

File tree

8 files changed

+509
-145
lines changed

8 files changed

+509
-145
lines changed

‎src/NHibernate.Test/Async/Futures/QueryBatchFixture.cs‎

Lines changed: 109 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -278,6 +278,115 @@ public async Task FutureForEagerMappedCollectionAsync()
278278
}
279279
}
280280

281+
[Test]
282+
public async Task AutoDiscoverWorksWithFutureAsync()
283+
{
284+
using (var s = OpenSession())
285+
using (var t = s.BeginTransaction())
286+
{
287+
var future =
288+
s
289+
.CreateSQLQuery("select count(*) as count from EntitySimpleChild where Name like :pattern")
290+
.AddScalar("count", NHibernateUtil.Int64)
291+
.SetString("pattern", "Chi%")
292+
.SetCacheable(true)
293+
.FutureValue<long>();
294+
295+
Assert.That(await (future.GetValueAsync()), Is.EqualTo(2L), "From DB");
296+
await (t.CommitAsync());
297+
}
298+
299+
using (var s = OpenSession())
300+
using (var t = s.BeginTransaction())
301+
{
302+
var future =
303+
s
304+
.CreateSQLQuery("select count(*) as count from EntitySimpleChild where Name like :pattern")
305+
.AddScalar("count", NHibernateUtil.Int64)
306+
.SetString("pattern", "Chi%")
307+
.SetCacheable(true)
308+
.FutureValue<long>();
309+
310+
Assert.That(await (future.GetValueAsync()), Is.EqualTo(2L), "From cache");
311+
await (t.CommitAsync());
312+
}
313+
}
314+
315+
[Test]
316+
public async Task AutoFlushCacheInvalidationWorksWithFutureAsync()
317+
{
318+
using (var s = OpenSession())
319+
using (var t = s.BeginTransaction())
320+
{
321+
var futureResults =
322+
(await (s
323+
.CreateQuery("from EntitySimpleChild")
324+
.SetCacheable(true)
325+
.Future<EntitySimpleChild>()
326+
.GetEnumerableAsync()))
327+
.ToList();
328+
329+
Assert.That(futureResults, Has.Count.EqualTo(2), "First call");
330+
331+
await (t.CommitAsync());
332+
}
333+
334+
using (var s = OpenSession())
335+
using (var t = s.BeginTransaction())
336+
{
337+
var deleted = await (s.Query<EntitySimpleChild>().FirstAsync());
338+
// We need to get rid of a referencing entity for the delete.
339+
deleted.Parent.Child1 = null;
340+
deleted.Parent.Child2 = null;
341+
await (s.DeleteAsync(deleted));
342+
343+
var future =
344+
s
345+
.CreateQuery("from EntitySimpleChild")
346+
.SetCacheable(true)
347+
.Future<EntitySimpleChild>();
348+
349+
Assert.That((await (future.GetEnumerableAsync())).ToList(), Has.Count.EqualTo(1), "After delete");
350+
await (t.CommitAsync());
351+
}
352+
}
353+
354+
[Test]
355+
public async Task UsingHqlToFutureWithCacheAndTransformerDoesntThrowAsync()
356+
{
357+
// Adapted from #383
358+
using (var session = OpenSession())
359+
using (var t = session.BeginTransaction())
360+
{
361+
//store values in cache
362+
await (session
363+
.CreateQuery("from EntitySimpleChild")
364+
.SetResultTransformer(Transformers.DistinctRootEntity)
365+
.SetCacheable(true)
366+
.SetCacheMode(CacheMode.Normal)
367+
.Future<EntitySimpleChild>()
368+
.GetEnumerableAsync());
369+
await (t.CommitAsync());
370+
}
371+
372+
using (var session = OpenSession())
373+
using (var t = session.BeginTransaction())
374+
{
375+
//get values from cache
376+
var results =
377+
(await (session
378+
.CreateQuery("from EntitySimpleChild")
379+
.SetResultTransformer(Transformers.DistinctRootEntity)
380+
.SetCacheable(true)
381+
.SetCacheMode(CacheMode.Normal)
382+
.Future<EntitySimpleChild>()
383+
.GetEnumerableAsync()))
384+
.ToList();
385+
386+
Assert.That(results.Count, Is.EqualTo(2));
387+
}
388+
}
389+
281390
#region Test Setup
282391

283392
protected override HbmMapping GetMappings()

‎src/NHibernate.Test/Futures/QueryBatchFixture.cs‎

Lines changed: 110 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -266,6 +266,116 @@ public void FutureForEagerMappedCollection()
266266
}
267267
}
268268

269+
[Test]
270+
public void AutoDiscoverWorksWithFuture()
271+
{
272+
using (var s = OpenSession())
273+
using (var t = s.BeginTransaction())
274+
{
275+
var future =
276+
s
277+
.CreateSQLQuery("select count(*) as count from EntitySimpleChild where Name like :pattern")
278+
.AddScalar("count", NHibernateUtil.Int64)
279+
.SetString("pattern", "Chi%")
280+
.SetCacheable(true)
281+
.FutureValue<long>();
282+
283+
Assert.That(future.Value, Is.EqualTo(2L), "From DB");
284+
t.Commit();
285+
}
286+
287+
using (var s = OpenSession())
288+
using (var t = s.BeginTransaction())
289+
{
290+
var future =
291+
s
292+
.CreateSQLQuery("select count(*) as count from EntitySimpleChild where Name like :pattern")
293+
.AddScalar("count", NHibernateUtil.Int64)
294+
.SetString("pattern", "Chi%")
295+
.SetCacheable(true)
296+
.FutureValue<long>();
297+
298+
Assert.That(future.Value, Is.EqualTo(2L), "From cache");
299+
t.Commit();
300+
}
301+
}
302+
303+
[Test]
304+
public void AutoFlushCacheInvalidationWorksWithFuture()
305+
{
306+
using (var s = OpenSession())
307+
using (var t = s.BeginTransaction())
308+
{
309+
var futureResults =
310+
s
311+
.CreateQuery("from EntitySimpleChild")
312+
.SetCacheable(true)
313+
.Future<EntitySimpleChild>()
314+
.GetEnumerable()
315+
.ToList();
316+
317+
Assert.That(futureResults, Has.Count.EqualTo(2), "First call");
318+
319+
t.Commit();
320+
}
321+
322+
using (var s = OpenSession())
323+
using (var t = s.BeginTransaction())
324+
{
325+
var deleted = s.Query<EntitySimpleChild>().First();
326+
// We need to get rid of a referencing entity for the delete.
327+
deleted.Parent.Child1 = null;
328+
deleted.Parent.Child2 = null;
329+
s.Delete(deleted);
330+
331+
var future =
332+
s
333+
.CreateQuery("from EntitySimpleChild")
334+
.SetCacheable(true)
335+
.Future<EntitySimpleChild>();
336+
337+
Assert.That(future.GetEnumerable().ToList(), Has.Count.EqualTo(1), "After delete");
338+
t.Commit();
339+
}
340+
}
341+
342+
[Test]
343+
public void UsingHqlToFutureWithCacheAndTransformerDoesntThrow()
344+
{
345+
// Adapted from #383
346+
using (var session = OpenSession())
347+
using (var t = session.BeginTransaction())
348+
{
349+
//store values in cache
350+
session
351+
.CreateQuery("from EntitySimpleChild")
352+
.SetResultTransformer(Transformers.DistinctRootEntity)
353+
.SetCacheable(true)
354+
.SetCacheMode(CacheMode.Normal)
355+
.Future<EntitySimpleChild>()
356+
.GetEnumerable();
357+
t.Commit();
358+
}
359+
360+
using (var session = OpenSession())
361+
using (var t = session.BeginTransaction())
362+
{
363+
//get values from cache
364+
var results =
365+
session
366+
.CreateQuery("from EntitySimpleChild")
367+
.SetResultTransformer(Transformers.DistinctRootEntity)
368+
.SetCacheable(true)
369+
.SetCacheMode(CacheMode.Normal)
370+
.Future<EntitySimpleChild>()
371+
.GetEnumerable()
372+
.ToList();
373+
374+
Assert.That(results.Count, Is.EqualTo(2));
375+
t.Commit();
376+
}
377+
}
378+
269379
#region Test Setup
270380

271381
protected override HbmMapping GetMappings()

‎src/NHibernate/Async/Multi/IQueryBatchItem.cs‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,13 @@ public partial interface IQueryBatchItem
2828
/// <param name="cancellationToken">A cancellation token that can be used to cancel the work</param>
2929
Task<IEnumerable<ISqlCommand>> GetCommandsAsync(CancellationToken cancellationToken);
3030

31+
/// <summary>
32+
/// Process the result sets generated by <see cref="GetCommands"/>. Advance the results set
33+
/// to the next query, or to its end if this is the last query.
34+
/// </summary>
35+
/// <returns>The number of rows processed.</returns>
36+
Task<int> ProcessResultsSetAsync(DbDataReader reader, CancellationToken cancellationToken);
37+
3138
/// <summary>
3239
/// Executed after all commands in batch are processed
3340
/// </summary>

‎src/NHibernate/Async/Multi/QueryBatch.cs‎

Lines changed: 46 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -29,39 +29,40 @@ public async Task ExecuteAsync(CancellationToken cancellationToken)
2929
cancellationToken.ThrowIfCancellationRequested();
3030
if (_queries.Count == 0)
3131
return;
32-
var sessionFlushMode = Session.FlushMode;
33-
if (FlushMode.HasValue)
34-
Session.FlushMode = FlushMode.Value;
35-
try
32+
using (Session.BeginProcess())
3633
{
37-
Init();
38-
39-
if (!Session.Factory.ConnectionProvider.Driver.SupportsMultipleQueries)
34+
var sessionFlushMode = Session.FlushMode;
35+
if (FlushMode.HasValue)
36+
Session.FlushMode = FlushMode.Value;
37+
try
4038
{
41-
foreach (var query in _queries)
39+
Init();
40+
41+
if (!Session.Factory.ConnectionProvider.Driver.SupportsMultipleQueries)
4242
{
43-
await (query.ExecuteNonBatchedAsync(cancellationToken)).ConfigureAwait(false);
43+
foreach (var query in _queries)
44+
{
45+
await (query.ExecuteNonBatchedAsync(cancellationToken)).ConfigureAwait(false);
46+
}
47+
48+
return;
4449
}
45-
return;
46-
}
4750

48-
using (Session.BeginProcess())
49-
{
50-
await (DoExecuteAsync(cancellationToken)).ConfigureAwait(false);
51+
await (ExecuteBatchedAsync(cancellationToken)).ConfigureAwait(false);
5152
}
52-
}
53-
finally
54-
{
55-
if (_autoReset)
53+
finally
5654
{
57-
_queries.Clear();
58-
_queriesByKey.Clear();
59-
}
60-
else
61-
_executed = true;
55+
if (_autoReset)
56+
{
57+
_queries.Clear();
58+
_queriesByKey.Clear();
59+
}
60+
else
61+
_executed = true;
6262

63-
if (FlushMode.HasValue)
64-
Session.FlushMode = sessionFlushMode;
63+
if (FlushMode.HasValue)
64+
Session.FlushMode = sessionFlushMode;
65+
}
6566
}
6667
}
6768

@@ -90,7 +91,7 @@ private async Task<IList<TResult>> GetResultsAsync<TResult>(IQueryBatchItem quer
9091
cancellationToken.ThrowIfCancellationRequested();
9192
if (!_executed)
9293
await (ExecuteAsync(cancellationToken)).ConfigureAwait(false);
93-
return ((IQueryBatchItem<TResult>)query).GetResults();
94+
return ((IQueryBatchItem<TResult>) query).GetResults();
9495
}
9596

9697
private async Task CombineQueriesAsync(IResultSetsCommand resultSetsCommand, CancellationToken cancellationToken)
@@ -103,25 +104,29 @@ private async Task CombineQueriesAsync(IResultSetsCommand resultSetsCommand, Can
103104
}
104105
}
105106

106-
protected async Task DoExecuteAsync(CancellationToken cancellationToken)
107+
protected async Task ExecuteBatchedAsync(CancellationToken cancellationToken)
107108
{
108109
cancellationToken.ThrowIfCancellationRequested();
109-
var resultSetsCommand = Session.Factory.ConnectionProvider.Driver.GetResultSetsCommand(Session);
110-
await (CombineQueriesAsync(resultSetsCommand, cancellationToken)).ConfigureAwait(false);
111-
112110
var querySpaces = new HashSet<string>(_queries.SelectMany(t => t.GetQuerySpaces()));
113-
if (resultSetsCommand.HasQueries)
111+
if (querySpaces.Count > 0)
114112
{
115113
await (Session.AutoFlushIfRequiredAsync(querySpaces, cancellationToken)).ConfigureAwait(false);
116114
}
117115

116+
var resultSetsCommand = Session.Factory.ConnectionProvider.Driver.GetResultSetsCommand(Session);
117+
// CombineQueries queries the second level cache, which may contain stale data in regard to
118+
// the session changes. For having them invalidated, auto-flush must have been handled before
119+
// calling CombineQueries.
120+
await (CombineQueriesAsync(resultSetsCommand, cancellationToken)).ConfigureAwait(false);
121+
118122
bool statsEnabled = Session.Factory.Statistics.IsStatisticsEnabled;
119123
Stopwatch stopWatch = null;
120124
if (statsEnabled)
121125
{
122126
stopWatch = new Stopwatch();
123127
stopWatch.Start();
124128
}
129+
125130
if (Log.IsDebugEnabled())
126131
{
127132
Log.Debug("Multi query with {0} queries: {1}", _queries.Count, resultSetsCommand.Sql);
@@ -136,11 +141,7 @@ protected async Task DoExecuteAsync(CancellationToken cancellationToken)
136141
{
137142
foreach (var multiSource in _queries)
138143
{
139-
foreach (var resultSetHandler in multiSource.GetResultSetHandler())
140-
{
141-
rowCount += resultSetHandler(reader);
142-
await (reader.NextResultAsync(cancellationToken)).ConfigureAwait(false);
143-
}
144+
rowCount += await (multiSource.ProcessResultsSetAsync(reader, cancellationToken)).ConfigureAwait(false);
144145
}
145146
}
146147
}
@@ -154,13 +155,20 @@ protected async Task DoExecuteAsync(CancellationToken cancellationToken)
154155
catch (Exception sqle)
155156
{
156157
Log.Error(sqle, "Failed to execute multi query: [{0}]", resultSetsCommand.Sql);
157-
throw ADOExceptionHelper.Convert(Session.Factory.SQLExceptionConverter, sqle, "Failed to execute multi query", resultSetsCommand.Sql);
158+
throw ADOExceptionHelper.Convert(
159+
Session.Factory.SQLExceptionConverter,
160+
sqle,
161+
"Failed to execute multi query",
162+
resultSetsCommand.Sql);
158163
}
159164

160165
if (statsEnabled)
161166
{
162167
stopWatch.Stop();
163-
Session.Factory.StatisticsImplementor.QueryExecuted($"{_queries.Count} queries", rowCount, stopWatch.Elapsed);
168+
Session.Factory.StatisticsImplementor.QueryExecuted(
169+
$"{_queries.Count} queries",
170+
rowCount,
171+
stopWatch.Elapsed);
164172
}
165173
}
166174
}

0 commit comments

Comments
 (0)