From 7cfce069d412c66377ada3d5e0760802e79b8fa6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20Delaporte?= <12201973+fredericDelaporte@users.noreply.github.com> Date: Tue, 10 Jul 2018 16:52:15 +0200 Subject: [PATCH 1/3] 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. --- .../Async/Futures/QueryBatchFixture.cs | 109 ++++++++++++++ .../Futures/QueryBatchFixture.cs | 110 ++++++++++++++ src/NHibernate/Async/Multi/IQueryBatchItem.cs | 7 + src/NHibernate/Async/Multi/QueryBatch.cs | 84 ++++++----- .../Async/Multi/QueryBatchItemBase.cs | 112 +++++++++++++- src/NHibernate/Multi/IQueryBatchItem.cs | 8 +- src/NHibernate/Multi/QueryBatch.cs | 84 ++++++----- src/NHibernate/Multi/QueryBatchItemBase.cs | 140 ++++++++++-------- 8 files changed, 509 insertions(+), 145 deletions(-) diff --git a/src/NHibernate.Test/Async/Futures/QueryBatchFixture.cs b/src/NHibernate.Test/Async/Futures/QueryBatchFixture.cs index c7ece785054..7ba634c44f4 100644 --- a/src/NHibernate.Test/Async/Futures/QueryBatchFixture.cs +++ b/src/NHibernate.Test/Async/Futures/QueryBatchFixture.cs @@ -278,6 +278,115 @@ public async Task FutureForEagerMappedCollectionAsync() } } + [Test] + public async Task AutoDiscoverWorksWithFutureAsync() + { + using (var s = OpenSession()) + using (var t = s.BeginTransaction()) + { + var future = + s + .CreateSQLQuery("select count(*) as count from EntitySimpleChild where Name like :pattern") + .AddScalar("count", NHibernateUtil.Int64) + .SetString("pattern", "Chi%") + .SetCacheable(true) + .FutureValue(); + + Assert.That(await (future.GetValueAsync()), Is.EqualTo(2L), "From DB"); + await (t.CommitAsync()); + } + + using (var s = OpenSession()) + using (var t = s.BeginTransaction()) + { + var future = + s + .CreateSQLQuery("select count(*) as count from EntitySimpleChild where Name like :pattern") + .AddScalar("count", NHibernateUtil.Int64) + .SetString("pattern", "Chi%") + .SetCacheable(true) + .FutureValue(); + + Assert.That(await (future.GetValueAsync()), Is.EqualTo(2L), "From cache"); + await (t.CommitAsync()); + } + } + + [Test] + public async Task AutoFlushCacheInvalidationWorksWithFutureAsync() + { + using (var s = OpenSession()) + using (var t = s.BeginTransaction()) + { + var futureResults = + (await (s + .CreateQuery("from EntitySimpleChild") + .SetCacheable(true) + .Future() + .GetEnumerableAsync())) + .ToList(); + + Assert.That(futureResults, Has.Count.EqualTo(2), "First call"); + + await (t.CommitAsync()); + } + + using (var s = OpenSession()) + using (var t = s.BeginTransaction()) + { + var deleted = await (s.Query().FirstAsync()); + // We need to get rid of a referencing entity for the delete. + deleted.Parent.Child1 = null; + deleted.Parent.Child2 = null; + await (s.DeleteAsync(deleted)); + + var future = + s + .CreateQuery("from EntitySimpleChild") + .SetCacheable(true) + .Future(); + + Assert.That((await (future.GetEnumerableAsync())).ToList(), Has.Count.EqualTo(1), "After delete"); + await (t.CommitAsync()); + } + } + + [Test] + public async Task UsingHqlToFutureWithCacheAndTransformerDoesntThrowAsync() + { + // Adapted from #383 + using (var session = OpenSession()) + using (var t = session.BeginTransaction()) + { + //store values in cache + await (session + .CreateQuery("from EntitySimpleChild") + .SetResultTransformer(Transformers.DistinctRootEntity) + .SetCacheable(true) + .SetCacheMode(CacheMode.Normal) + .Future() + .GetEnumerableAsync()); + await (t.CommitAsync()); + } + + using (var session = OpenSession()) + using (var t = session.BeginTransaction()) + { + //get values from cache + var results = + (await (session + .CreateQuery("from EntitySimpleChild") + .SetResultTransformer(Transformers.DistinctRootEntity) + .SetCacheable(true) + .SetCacheMode(CacheMode.Normal) + .Future() + .GetEnumerableAsync())) + .ToList(); + + Assert.That(results.Count, Is.EqualTo(2)); + } + } + #region Test Setup protected override HbmMapping GetMappings() diff --git a/src/NHibernate.Test/Futures/QueryBatchFixture.cs b/src/NHibernate.Test/Futures/QueryBatchFixture.cs index 2e6989ec0d9..d5d46833d6f 100644 --- a/src/NHibernate.Test/Futures/QueryBatchFixture.cs +++ b/src/NHibernate.Test/Futures/QueryBatchFixture.cs @@ -266,6 +266,116 @@ public void FutureForEagerMappedCollection() } } + [Test] + public void AutoDiscoverWorksWithFuture() + { + using (var s = OpenSession()) + using (var t = s.BeginTransaction()) + { + var future = + s + .CreateSQLQuery("select count(*) as count from EntitySimpleChild where Name like :pattern") + .AddScalar("count", NHibernateUtil.Int64) + .SetString("pattern", "Chi%") + .SetCacheable(true) + .FutureValue(); + + Assert.That(future.Value, Is.EqualTo(2L), "From DB"); + t.Commit(); + } + + using (var s = OpenSession()) + using (var t = s.BeginTransaction()) + { + var future = + s + .CreateSQLQuery("select count(*) as count from EntitySimpleChild where Name like :pattern") + .AddScalar("count", NHibernateUtil.Int64) + .SetString("pattern", "Chi%") + .SetCacheable(true) + .FutureValue(); + + Assert.That(future.Value, Is.EqualTo(2L), "From cache"); + t.Commit(); + } + } + + [Test] + public void AutoFlushCacheInvalidationWorksWithFuture() + { + using (var s = OpenSession()) + using (var t = s.BeginTransaction()) + { + var futureResults = + s + .CreateQuery("from EntitySimpleChild") + .SetCacheable(true) + .Future() + .GetEnumerable() + .ToList(); + + Assert.That(futureResults, Has.Count.EqualTo(2), "First call"); + + t.Commit(); + } + + using (var s = OpenSession()) + using (var t = s.BeginTransaction()) + { + var deleted = s.Query().First(); + // We need to get rid of a referencing entity for the delete. + deleted.Parent.Child1 = null; + deleted.Parent.Child2 = null; + s.Delete(deleted); + + var future = + s + .CreateQuery("from EntitySimpleChild") + .SetCacheable(true) + .Future(); + + Assert.That(future.GetEnumerable().ToList(), Has.Count.EqualTo(1), "After delete"); + t.Commit(); + } + } + + [Test] + public void UsingHqlToFutureWithCacheAndTransformerDoesntThrow() + { + // Adapted from #383 + using (var session = OpenSession()) + using (var t = session.BeginTransaction()) + { + //store values in cache + session + .CreateQuery("from EntitySimpleChild") + .SetResultTransformer(Transformers.DistinctRootEntity) + .SetCacheable(true) + .SetCacheMode(CacheMode.Normal) + .Future() + .GetEnumerable(); + t.Commit(); + } + + using (var session = OpenSession()) + using (var t = session.BeginTransaction()) + { + //get values from cache + var results = + session + .CreateQuery("from EntitySimpleChild") + .SetResultTransformer(Transformers.DistinctRootEntity) + .SetCacheable(true) + .SetCacheMode(CacheMode.Normal) + .Future() + .GetEnumerable() + .ToList(); + + Assert.That(results.Count, Is.EqualTo(2)); + t.Commit(); + } + } + #region Test Setup protected override HbmMapping GetMappings() diff --git a/src/NHibernate/Async/Multi/IQueryBatchItem.cs b/src/NHibernate/Async/Multi/IQueryBatchItem.cs index 876f9618cdd..20b8b8f78a7 100644 --- a/src/NHibernate/Async/Multi/IQueryBatchItem.cs +++ b/src/NHibernate/Async/Multi/IQueryBatchItem.cs @@ -28,6 +28,13 @@ public partial interface IQueryBatchItem /// A cancellation token that can be used to cancel the work Task> GetCommandsAsync(CancellationToken cancellationToken); + /// + /// Process the result sets generated by . Advance the results set + /// to the next query, or to its end if this is the last query. + /// + /// The number of rows processed. + Task ProcessResultsSetAsync(DbDataReader reader, CancellationToken cancellationToken); + /// /// Executed after all commands in batch are processed /// diff --git a/src/NHibernate/Async/Multi/QueryBatch.cs b/src/NHibernate/Async/Multi/QueryBatch.cs index 55ddc679111..3fb8783227e 100644 --- a/src/NHibernate/Async/Multi/QueryBatch.cs +++ b/src/NHibernate/Async/Multi/QueryBatch.cs @@ -29,39 +29,40 @@ public async Task ExecuteAsync(CancellationToken cancellationToken) cancellationToken.ThrowIfCancellationRequested(); if (_queries.Count == 0) return; - var sessionFlushMode = Session.FlushMode; - if (FlushMode.HasValue) - Session.FlushMode = FlushMode.Value; - try + using (Session.BeginProcess()) { - Init(); - - if (!Session.Factory.ConnectionProvider.Driver.SupportsMultipleQueries) + var sessionFlushMode = Session.FlushMode; + if (FlushMode.HasValue) + Session.FlushMode = FlushMode.Value; + try { - foreach (var query in _queries) + Init(); + + if (!Session.Factory.ConnectionProvider.Driver.SupportsMultipleQueries) { - await (query.ExecuteNonBatchedAsync(cancellationToken)).ConfigureAwait(false); + foreach (var query in _queries) + { + await (query.ExecuteNonBatchedAsync(cancellationToken)).ConfigureAwait(false); + } + + return; } - return; - } - using (Session.BeginProcess()) - { - await (DoExecuteAsync(cancellationToken)).ConfigureAwait(false); + await (ExecuteBatchedAsync(cancellationToken)).ConfigureAwait(false); } - } - finally - { - if (_autoReset) + finally { - _queries.Clear(); - _queriesByKey.Clear(); - } - else - _executed = true; + if (_autoReset) + { + _queries.Clear(); + _queriesByKey.Clear(); + } + else + _executed = true; - if (FlushMode.HasValue) - Session.FlushMode = sessionFlushMode; + if (FlushMode.HasValue) + Session.FlushMode = sessionFlushMode; + } } } @@ -90,7 +91,7 @@ private async Task> GetResultsAsync(IQueryBatchItem quer cancellationToken.ThrowIfCancellationRequested(); if (!_executed) await (ExecuteAsync(cancellationToken)).ConfigureAwait(false); - return ((IQueryBatchItem)query).GetResults(); + return ((IQueryBatchItem) query).GetResults(); } private async Task CombineQueriesAsync(IResultSetsCommand resultSetsCommand, CancellationToken cancellationToken) @@ -103,18 +104,21 @@ private async Task CombineQueriesAsync(IResultSetsCommand resultSetsCommand, Can } } - protected async Task DoExecuteAsync(CancellationToken cancellationToken) + protected async Task ExecuteBatchedAsync(CancellationToken cancellationToken) { cancellationToken.ThrowIfCancellationRequested(); - var resultSetsCommand = Session.Factory.ConnectionProvider.Driver.GetResultSetsCommand(Session); - await (CombineQueriesAsync(resultSetsCommand, cancellationToken)).ConfigureAwait(false); - var querySpaces = new HashSet(_queries.SelectMany(t => t.GetQuerySpaces())); - if (resultSetsCommand.HasQueries) + if (querySpaces.Count > 0) { await (Session.AutoFlushIfRequiredAsync(querySpaces, cancellationToken)).ConfigureAwait(false); } + var resultSetsCommand = Session.Factory.ConnectionProvider.Driver.GetResultSetsCommand(Session); + // CombineQueries queries the second level cache, which may contain stale data in regard to + // the session changes. For having them invalidated, auto-flush must have been handled before + // calling CombineQueries. + await (CombineQueriesAsync(resultSetsCommand, cancellationToken)).ConfigureAwait(false); + bool statsEnabled = Session.Factory.Statistics.IsStatisticsEnabled; Stopwatch stopWatch = null; if (statsEnabled) @@ -122,6 +126,7 @@ protected async Task DoExecuteAsync(CancellationToken cancellationToken) stopWatch = new Stopwatch(); stopWatch.Start(); } + if (Log.IsDebugEnabled()) { Log.Debug("Multi query with {0} queries: {1}", _queries.Count, resultSetsCommand.Sql); @@ -136,11 +141,7 @@ protected async Task DoExecuteAsync(CancellationToken cancellationToken) { foreach (var multiSource in _queries) { - foreach (var resultSetHandler in multiSource.GetResultSetHandler()) - { - rowCount += resultSetHandler(reader); - await (reader.NextResultAsync(cancellationToken)).ConfigureAwait(false); - } + rowCount += await (multiSource.ProcessResultsSetAsync(reader, cancellationToken)).ConfigureAwait(false); } } } @@ -154,13 +155,20 @@ protected async Task DoExecuteAsync(CancellationToken cancellationToken) catch (Exception sqle) { Log.Error(sqle, "Failed to execute multi query: [{0}]", resultSetsCommand.Sql); - throw ADOExceptionHelper.Convert(Session.Factory.SQLExceptionConverter, sqle, "Failed to execute multi query", resultSetsCommand.Sql); + throw ADOExceptionHelper.Convert( + Session.Factory.SQLExceptionConverter, + sqle, + "Failed to execute multi query", + resultSetsCommand.Sql); } if (statsEnabled) { stopWatch.Stop(); - Session.Factory.StatisticsImplementor.QueryExecuted($"{_queries.Count} queries", rowCount, stopWatch.Elapsed); + Session.Factory.StatisticsImplementor.QueryExecuted( + $"{_queries.Count} queries", + rowCount, + stopWatch.Elapsed); } } } diff --git a/src/NHibernate/Async/Multi/QueryBatchItemBase.cs b/src/NHibernate/Async/Multi/QueryBatchItemBase.cs index ff2d884ce18..c13522c2f38 100644 --- a/src/NHibernate/Async/Multi/QueryBatchItemBase.cs +++ b/src/NHibernate/Async/Multi/QueryBatchItemBase.cs @@ -41,6 +41,7 @@ public async Task> GetCommandsAsync(CancellationToken c if (qi.Loader.IsCacheable(qi.Parameters)) { + qi.IsCacheable = true; // Check if the results are available in the cache qi.Cache = Session.Factory.GetQueryCache(qi.Parameters.CacheRegion); qi.CacheKey = qi.Loader.GenerateQueryKey(Session, qi.Parameters); @@ -49,8 +50,8 @@ public async Task> GetCommandsAsync(CancellationToken c if (resultsFromCache != null) { // Cached results available, skip the command for them and stores them. - qi.Cache = null; _loaderResults[index] = resultsFromCache; + qi.IsResultFromCache = true; continue; } } @@ -59,6 +60,86 @@ public async Task> GetCommandsAsync(CancellationToken c return yields; } + public async Task ProcessResultsSetAsync(DbDataReader reader, CancellationToken cancellationToken) + { + cancellationToken.ThrowIfCancellationRequested(); + var dialect = Session.Factory.Dialect; + var hydratedObjects = new List[_queryInfos.Count]; + + var rowCount = 0; + for (var i = 0; i < _queryInfos.Count; i++) + { + var queryInfo = _queryInfos[i]; + var loader = queryInfo.Loader; + var queryParameters = queryInfo.Parameters; + + //Skip processing for items already loaded from cache + if (queryInfo.IsResultFromCache) + { + continue; + } + + var entitySpan = loader.EntityPersisters.Length; + hydratedObjects[i] = entitySpan == 0 ? null : new List(entitySpan); + var keys = new EntityKey[entitySpan]; + + var selection = queryParameters.RowSelection; + var createSubselects = loader.IsSubselectLoadingEnabled; + + _subselectResultKeys[i] = createSubselects ? new List() : null; + var maxRows = Loader.Loader.HasMaxRows(selection) ? selection.MaxRows : int.MaxValue; + var advanceSelection = !dialect.SupportsLimitOffset || !loader.UseLimit(selection, dialect); + + if (advanceSelection) + { + await (Loader.Loader.AdvanceAsync(reader, selection, cancellationToken)).ConfigureAwait(false); + } + + var forcedResultTransformer = queryInfo.CacheKey?.ResultTransformer; + if (queryParameters.HasAutoDiscoverScalarTypes) + { + loader.AutoDiscoverTypes(reader, queryParameters, forcedResultTransformer); + } + + var lockModeArray = loader.GetLockModes(queryParameters.LockModes); + var optionalObjectKey = Loader.Loader.GetOptionalObjectKey(queryParameters, Session); + var tmpResults = new List(); + + for (var count = 0; count < maxRows && await (reader.ReadAsync(cancellationToken)).ConfigureAwait(false); count++) + { + rowCount++; + + var o = + await (loader.GetRowFromResultSetAsync( + reader, + Session, + queryParameters, + lockModeArray, + optionalObjectKey, + hydratedObjects[i], + keys, + true, + forcedResultTransformer +, cancellationToken )).ConfigureAwait(false); + if (loader.IsSubselectLoadingEnabled) + { + _subselectResultKeys[i].Add(keys); + keys = new EntityKey[entitySpan]; //can't reuse in this case + } + + tmpResults.Add(o); + } + + _loaderResults[i] = tmpResults; + + await (reader.NextResultAsync(cancellationToken)).ConfigureAwait(false); + } + + await (InitializeEntitiesAndCollectionsAsync(reader, hydratedObjects, cancellationToken)).ConfigureAwait(false); + + return rowCount; + } + public async Task ProcessResultsAsync(CancellationToken cancellationToken) { cancellationToken.ThrowIfCancellationRequested(); @@ -71,9 +152,21 @@ public async Task ProcessResultsAsync(CancellationToken cancellationToken) } // Handle cache if cacheable. - if (queryInfo.Cache != null) + if (queryInfo.IsCacheable) { - await (queryInfo.Loader.PutResultInQueryCacheAsync(Session, queryInfo.Parameters, queryInfo.Cache, queryInfo.CacheKey, _loaderResults[i], cancellationToken)).ConfigureAwait(false); + if (!queryInfo.IsResultFromCache) + { + await (queryInfo.Loader.PutResultInQueryCacheAsync( + Session, + queryInfo.Parameters, + queryInfo.Cache, + queryInfo.CacheKey, + _loaderResults[i], cancellationToken)).ConfigureAwait(false); + } + + _loaderResults[i] = + queryInfo.Loader.TransformCacheableResults( + queryInfo.Parameters, queryInfo.CacheKey.ResultTransformer, _loaderResults[i]); } } AfterLoadCallback?.Invoke(GetResults()); @@ -87,5 +180,18 @@ public async Task ExecuteNonBatchedAsync(CancellationToken cancellationToken) } protected abstract Task> GetResultsNonBatchedAsync(CancellationToken cancellationToken); + + private async Task InitializeEntitiesAndCollectionsAsync(DbDataReader reader, List[] hydratedObjects, CancellationToken cancellationToken) + { + cancellationToken.ThrowIfCancellationRequested(); + for (var i = 0; i < _queryInfos.Count; i++) + { + var queryInfo = _queryInfos[i]; + if (queryInfo.IsResultFromCache) + continue; + await (queryInfo.Loader.InitializeEntitiesAndCollectionsAsync( + hydratedObjects[i], reader, Session, Session.PersistenceContext.DefaultReadOnly, cancellationToken)).ConfigureAwait(false); + } + } } } diff --git a/src/NHibernate/Multi/IQueryBatchItem.cs b/src/NHibernate/Multi/IQueryBatchItem.cs index e892e862b17..cef51c35e0e 100644 --- a/src/NHibernate/Multi/IQueryBatchItem.cs +++ b/src/NHibernate/Multi/IQueryBatchItem.cs @@ -42,11 +42,11 @@ public partial interface IQueryBatchItem IEnumerable GetCommands(); /// - /// Returns delegates for processing result sets generated by . - /// Delegate should return number of rows loaded by command. + /// Process the result sets generated by . Advance the results set + /// to the next query, or to its end if this is the last query. /// - /// - IEnumerable> GetResultSetHandler(); + /// The number of rows processed. + int ProcessResultsSet(DbDataReader reader); /// /// Executed after all commands in batch are processed diff --git a/src/NHibernate/Multi/QueryBatch.cs b/src/NHibernate/Multi/QueryBatch.cs index 7e92c6419bf..66f50a32ab5 100644 --- a/src/NHibernate/Multi/QueryBatch.cs +++ b/src/NHibernate/Multi/QueryBatch.cs @@ -37,39 +37,40 @@ public void Execute() { if (_queries.Count == 0) return; - var sessionFlushMode = Session.FlushMode; - if (FlushMode.HasValue) - Session.FlushMode = FlushMode.Value; - try + using (Session.BeginProcess()) { - Init(); - - if (!Session.Factory.ConnectionProvider.Driver.SupportsMultipleQueries) + var sessionFlushMode = Session.FlushMode; + if (FlushMode.HasValue) + Session.FlushMode = FlushMode.Value; + try { - foreach (var query in _queries) + Init(); + + if (!Session.Factory.ConnectionProvider.Driver.SupportsMultipleQueries) { - query.ExecuteNonBatched(); + foreach (var query in _queries) + { + query.ExecuteNonBatched(); + } + + return; } - return; - } - using (Session.BeginProcess()) - { - DoExecute(); + ExecuteBatched(); } - } - finally - { - if (_autoReset) + finally { - _queries.Clear(); - _queriesByKey.Clear(); - } - else - _executed = true; + if (_autoReset) + { + _queries.Clear(); + _queriesByKey.Clear(); + } + else + _executed = true; - if (FlushMode.HasValue) - Session.FlushMode = sessionFlushMode; + if (FlushMode.HasValue) + Session.FlushMode = sessionFlushMode; + } } } @@ -109,7 +110,7 @@ private IList GetResults(IQueryBatchItem query) { if (!_executed) Execute(); - return ((IQueryBatchItem)query).GetResults(); + return ((IQueryBatchItem) query).GetResults(); } private void Init() @@ -129,17 +130,20 @@ private void CombineQueries(IResultSetsCommand resultSetsCommand) } } - protected void DoExecute() + protected void ExecuteBatched() { - var resultSetsCommand = Session.Factory.ConnectionProvider.Driver.GetResultSetsCommand(Session); - CombineQueries(resultSetsCommand); - var querySpaces = new HashSet(_queries.SelectMany(t => t.GetQuerySpaces())); - if (resultSetsCommand.HasQueries) + if (querySpaces.Count > 0) { Session.AutoFlushIfRequired(querySpaces); } + var resultSetsCommand = Session.Factory.ConnectionProvider.Driver.GetResultSetsCommand(Session); + // CombineQueries queries the second level cache, which may contain stale data in regard to + // the session changes. For having them invalidated, auto-flush must have been handled before + // calling CombineQueries. + CombineQueries(resultSetsCommand); + bool statsEnabled = Session.Factory.Statistics.IsStatisticsEnabled; Stopwatch stopWatch = null; if (statsEnabled) @@ -147,6 +151,7 @@ protected void DoExecute() stopWatch = new Stopwatch(); stopWatch.Start(); } + if (Log.IsDebugEnabled()) { Log.Debug("Multi query with {0} queries: {1}", _queries.Count, resultSetsCommand.Sql); @@ -161,11 +166,7 @@ protected void DoExecute() { foreach (var multiSource in _queries) { - foreach (var resultSetHandler in multiSource.GetResultSetHandler()) - { - rowCount += resultSetHandler(reader); - reader.NextResult(); - } + rowCount += multiSource.ProcessResultsSet(reader); } } } @@ -178,13 +179,20 @@ protected void DoExecute() catch (Exception sqle) { Log.Error(sqle, "Failed to execute multi query: [{0}]", resultSetsCommand.Sql); - throw ADOExceptionHelper.Convert(Session.Factory.SQLExceptionConverter, sqle, "Failed to execute multi query", resultSetsCommand.Sql); + throw ADOExceptionHelper.Convert( + Session.Factory.SQLExceptionConverter, + sqle, + "Failed to execute multi query", + resultSetsCommand.Sql); } if (statsEnabled) { stopWatch.Stop(); - Session.Factory.StatisticsImplementor.QueryExecuted($"{_queries.Count} queries", rowCount, stopWatch.Elapsed); + Session.Factory.StatisticsImplementor.QueryExecuted( + $"{_queries.Count} queries", + rowCount, + stopWatch.Elapsed); } } } diff --git a/src/NHibernate/Multi/QueryBatchItemBase.cs b/src/NHibernate/Multi/QueryBatchItemBase.cs index 4fc5535b230..dfde5d3638f 100644 --- a/src/NHibernate/Multi/QueryBatchItemBase.cs +++ b/src/NHibernate/Multi/QueryBatchItemBase.cs @@ -28,9 +28,11 @@ protected class QueryLoadInfo public QueryParameters Parameters; //Cache related properties: + public bool IsCacheable; public ISet QuerySpaces; public IQueryCache Cache; public QueryKey CacheKey; + public bool IsResultFromCache; } protected abstract List GetQueryLoadInfo(); @@ -61,6 +63,7 @@ public IEnumerable GetCommands() if (qi.Loader.IsCacheable(qi.Parameters)) { + qi.IsCacheable = true; // Check if the results are available in the cache qi.Cache = Session.Factory.GetQueryCache(qi.Parameters.CacheRegion); qi.CacheKey = qi.Loader.GenerateQueryKey(Session, qi.Parameters); @@ -69,8 +72,8 @@ public IEnumerable GetCommands() if (resultsFromCache != null) { // Cached results available, skip the command for them and stores them. - qi.Cache = null; _loaderResults[index] = resultsFromCache; + qi.IsResultFromCache = true; continue; } } @@ -79,85 +82,83 @@ public IEnumerable GetCommands() } } - public IEnumerable> GetResultSetHandler() + public int ProcessResultsSet(DbDataReader reader) { var dialect = Session.Factory.Dialect; - List[] hydratedObjects = new List[_queryInfos.Count]; + var hydratedObjects = new List[_queryInfos.Count]; + var rowCount = 0; for (var i = 0; i < _queryInfos.Count; i++) { - Loader.Loader loader = _queryInfos[i].Loader; - var queryParameters = _queryInfos[i].Parameters; + var queryInfo = _queryInfos[i]; + var loader = queryInfo.Loader; + var queryParameters = queryInfo.Parameters; //Skip processing for items already loaded from cache - if (_queryInfos[i].CacheKey?.ResultTransformer != null && _loaderResults[i] != null) + if (queryInfo.IsResultFromCache) { - _loaderResults[i] = loader.TransformCacheableResults(queryParameters, _queryInfos[i].CacheKey.ResultTransformer, _loaderResults[i]); continue; } - int entitySpan = loader.EntityPersisters.Length; + var entitySpan = loader.EntityPersisters.Length; hydratedObjects[i] = entitySpan == 0 ? null : new List(entitySpan); - EntityKey[] keys = new EntityKey[entitySpan]; + var keys = new EntityKey[entitySpan]; - RowSelection selection = queryParameters.RowSelection; - bool createSubselects = loader.IsSubselectLoadingEnabled; + var selection = queryParameters.RowSelection; + var createSubselects = loader.IsSubselectLoadingEnabled; _subselectResultKeys[i] = createSubselects ? new List() : null; - int maxRows = Loader.Loader.HasMaxRows(selection) ? selection.MaxRows : int.MaxValue; - bool advanceSelection = !dialect.SupportsLimitOffset || !loader.UseLimit(selection, dialect); + var maxRows = Loader.Loader.HasMaxRows(selection) ? selection.MaxRows : int.MaxValue; + var advanceSelection = !dialect.SupportsLimitOffset || !loader.UseLimit(selection, dialect); - var index = i; - yield return reader => + if (advanceSelection) { - if (advanceSelection) - { - Loader.Loader.Advance(reader, selection); - } - if (queryParameters.HasAutoDiscoverScalarTypes) - { - loader.AutoDiscoverTypes(reader, queryParameters, null); - } + Loader.Loader.Advance(reader, selection); + } - LockMode[] lockModeArray = loader.GetLockModes(queryParameters.LockModes); - EntityKey optionalObjectKey = Loader.Loader.GetOptionalObjectKey(queryParameters, Session); - int rowCount = 0; - var tmpResults = new List(); + var forcedResultTransformer = queryInfo.CacheKey?.ResultTransformer; + if (queryParameters.HasAutoDiscoverScalarTypes) + { + loader.AutoDiscoverTypes(reader, queryParameters, forcedResultTransformer); + } - int count; - for (count = 0; count < maxRows && reader.Read(); count++) - { - rowCount++; - - object o = - loader.GetRowFromResultSet( - reader, - Session, - queryParameters, - lockModeArray, - optionalObjectKey, - hydratedObjects[index], - keys, - true, - _queryInfos[index].CacheKey?.ResultTransformer - ); - if (loader.IsSubselectLoadingEnabled) - { - _subselectResultKeys[index].Add(keys); - keys = new EntityKey[entitySpan]; //can't reuse in this case - } - - tmpResults.Add(o); - } - _loaderResults[index] = tmpResults; + var lockModeArray = loader.GetLockModes(queryParameters.LockModes); + var optionalObjectKey = Loader.Loader.GetOptionalObjectKey(queryParameters, Session); + var tmpResults = new List(); - if (index == _queryInfos.Count - 1) + for (var count = 0; count < maxRows && reader.Read(); count++) + { + rowCount++; + + var o = + loader.GetRowFromResultSet( + reader, + Session, + queryParameters, + lockModeArray, + optionalObjectKey, + hydratedObjects[i], + keys, + true, + forcedResultTransformer + ); + if (loader.IsSubselectLoadingEnabled) { - InitializeEntitiesAndCollections(reader, hydratedObjects); + _subselectResultKeys[i].Add(keys); + keys = new EntityKey[entitySpan]; //can't reuse in this case } - return rowCount; - }; + + tmpResults.Add(o); + } + + _loaderResults[i] = tmpResults; + + reader.NextResult(); } + + InitializeEntitiesAndCollections(reader, hydratedObjects); + + return rowCount; } public void ProcessResults() @@ -171,9 +172,21 @@ public void ProcessResults() } // Handle cache if cacheable. - if (queryInfo.Cache != null) + if (queryInfo.IsCacheable) { - queryInfo.Loader.PutResultInQueryCache(Session, queryInfo.Parameters, queryInfo.Cache, queryInfo.CacheKey, _loaderResults[i]); + if (!queryInfo.IsResultFromCache) + { + queryInfo.Loader.PutResultInQueryCache( + Session, + queryInfo.Parameters, + queryInfo.Cache, + queryInfo.CacheKey, + _loaderResults[i]); + } + + _loaderResults[i] = + queryInfo.Loader.TransformCacheableResults( + queryInfo.Parameters, queryInfo.CacheKey.ResultTransformer, _loaderResults[i]); } } AfterLoadCallback?.Invoke(GetResults()); @@ -221,10 +234,13 @@ public IList GetResults() private void InitializeEntitiesAndCollections(DbDataReader reader, List[] hydratedObjects) { - for (int i = 0; i < _queryInfos.Count; i++) + for (var i = 0; i < _queryInfos.Count; i++) { - _queryInfos[i].Loader.InitializeEntitiesAndCollections( - hydratedObjects[i], reader, Session, Session.PersistenceContext.DefaultReadOnly); + var queryInfo = _queryInfos[i]; + if (queryInfo.IsResultFromCache) + continue; + queryInfo.Loader.InitializeEntitiesAndCollections( + hydratedObjects[i], reader, Session, Session.PersistenceContext.DefaultReadOnly); } } } From 8883f6cf5ebd33f2c2c2b37341f72f97479491b0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20Delaporte?= <12201973+fredericDelaporte@users.noreply.github.com> Date: Tue, 10 Jul 2018 17:09:03 +0200 Subject: [PATCH 2/3] fixup! Fix shortcomings of QueryBatcher initial implementation Additional cleanup/doc originating from #1788 but which should be here --- .../Async/Futures/QueryBatchFixture.cs | 1 + src/NHibernate/Async/Multi/IQueryBatchItem.cs | 10 +++-- src/NHibernate/Async/Multi/QueryBatch.cs | 4 +- .../Async/Multi/QueryBatchItemBase.cs | 12 +++--- src/NHibernate/Multi/IQueryBatchItem.cs | 37 ++++++++++++------- src/NHibernate/Multi/QueryBatch.cs | 4 +- src/NHibernate/Multi/QueryBatchItemBase.cs | 29 ++++++++------- 7 files changed, 56 insertions(+), 41 deletions(-) diff --git a/src/NHibernate.Test/Async/Futures/QueryBatchFixture.cs b/src/NHibernate.Test/Async/Futures/QueryBatchFixture.cs index 7ba634c44f4..8e0ec5061cb 100644 --- a/src/NHibernate.Test/Async/Futures/QueryBatchFixture.cs +++ b/src/NHibernate.Test/Async/Futures/QueryBatchFixture.cs @@ -384,6 +384,7 @@ public async Task UsingHqlToFutureWithCacheAndTransformerDoesntThrowAsync() .ToList(); Assert.That(results.Count, Is.EqualTo(2)); + await (t.CommitAsync()); } } diff --git a/src/NHibernate/Async/Multi/IQueryBatchItem.cs b/src/NHibernate/Async/Multi/IQueryBatchItem.cs index 20b8b8f78a7..0f8ef792167 100644 --- a/src/NHibernate/Async/Multi/IQueryBatchItem.cs +++ b/src/NHibernate/Async/Multi/IQueryBatchItem.cs @@ -23,9 +23,10 @@ public partial interface IQueryBatchItem { /// - /// Returns commands generated by query + /// Get the commands to execute for getting the not-already cached results of this query. /// /// A cancellation token that can be used to cancel the work + /// The commands for obtaining the results not already cached. Task> GetCommandsAsync(CancellationToken cancellationToken); /// @@ -36,13 +37,16 @@ public partial interface IQueryBatchItem Task ProcessResultsSetAsync(DbDataReader reader, CancellationToken cancellationToken); /// - /// Executed after all commands in batch are processed + /// Process the results of the query, including cached results. /// /// A cancellation token that can be used to cancel the work + /// Any result from the database must have been previously processed + /// through . Task ProcessResultsAsync(CancellationToken cancellationToken); /// - /// Immediate query execution in case the dialect does not support batches + /// Execute immediately the query as a single standalone query. Used in case the data-provider + /// does not support batches. /// /// A cancellation token that can be used to cancel the work Task ExecuteNonBatchedAsync(CancellationToken cancellationToken); diff --git a/src/NHibernate/Async/Multi/QueryBatch.cs b/src/NHibernate/Async/Multi/QueryBatch.cs index 3fb8783227e..68934aa8743 100644 --- a/src/NHibernate/Async/Multi/QueryBatch.cs +++ b/src/NHibernate/Async/Multi/QueryBatch.cs @@ -119,7 +119,7 @@ protected async Task ExecuteBatchedAsync(CancellationToken cancellationToken) // calling CombineQueries. await (CombineQueriesAsync(resultSetsCommand, cancellationToken)).ConfigureAwait(false); - bool statsEnabled = Session.Factory.Statistics.IsStatisticsEnabled; + var statsEnabled = Session.Factory.Statistics.IsStatisticsEnabled; Stopwatch stopWatch = null; if (statsEnabled) { @@ -132,7 +132,7 @@ protected async Task ExecuteBatchedAsync(CancellationToken cancellationToken) Log.Debug("Multi query with {0} queries: {1}", _queries.Count, resultSetsCommand.Sql); } - int rowCount = 0; + var rowCount = 0; try { if (resultSetsCommand.HasQueries) diff --git a/src/NHibernate/Async/Multi/QueryBatchItemBase.cs b/src/NHibernate/Async/Multi/QueryBatchItemBase.cs index c13522c2f38..ae9650b847f 100644 --- a/src/NHibernate/Async/Multi/QueryBatchItemBase.cs +++ b/src/NHibernate/Async/Multi/QueryBatchItemBase.cs @@ -25,12 +25,7 @@ namespace NHibernate.Multi public abstract partial class QueryBatchItemBase : IQueryBatchItem { - /// - /// Gets the commands to execute for getting the not-already cached results of this query. Does retrieves - /// already cached results by side-effect. - /// - /// A cancellation token that can be used to cancel the work - /// The commands for obtaining the results not already cached. + /// public async Task> GetCommandsAsync(CancellationToken cancellationToken) { cancellationToken.ThrowIfCancellationRequested(); @@ -60,6 +55,7 @@ public async Task> GetCommandsAsync(CancellationToken c return yields; } + /// public async Task ProcessResultsSetAsync(DbDataReader reader, CancellationToken cancellationToken) { cancellationToken.ThrowIfCancellationRequested(); @@ -140,10 +136,11 @@ public async Task ProcessResultsSetAsync(DbDataReader reader, CancellationT return rowCount; } + /// public async Task ProcessResultsAsync(CancellationToken cancellationToken) { cancellationToken.ThrowIfCancellationRequested(); - for (int i = 0; i < _queryInfos.Count; i++) + for (var i = 0; i < _queryInfos.Count; i++) { var queryInfo = _queryInfos[i]; if (_subselectResultKeys[i] != null) @@ -172,6 +169,7 @@ public async Task ProcessResultsAsync(CancellationToken cancellationToken) AfterLoadCallback?.Invoke(GetResults()); } + /// public async Task ExecuteNonBatchedAsync(CancellationToken cancellationToken) { cancellationToken.ThrowIfCancellationRequested(); diff --git a/src/NHibernate/Multi/IQueryBatchItem.cs b/src/NHibernate/Multi/IQueryBatchItem.cs index cef51c35e0e..2bceb711108 100644 --- a/src/NHibernate/Multi/IQueryBatchItem.cs +++ b/src/NHibernate/Multi/IQueryBatchItem.cs @@ -7,38 +7,49 @@ namespace NHibernate.Multi { /// - /// Interface for wrapping query to be batched by + /// Interface for wrapping query to be batched by . /// public interface IQueryBatchItem : IQueryBatchItem { /// - /// Returns loaded typed results by query. + /// Return loaded typed results by query. /// Must be called only after . /// IList GetResults(); /// - /// Callback is executed after results are loaded by batch. - /// Loaded results are provided in action parameter. + /// A callback, executed after results are loaded by the batch. + /// Loaded results are provided as the action parameter. /// Action> AfterLoadCallback { get; set; } } /// - /// Interface for wrapping query to be batched by + /// Interface for wrapping query to be batched by . /// public partial interface IQueryBatchItem { /// - /// Method is called right before batch execution. + /// Initialize the query. Method is called right before batch execution. /// Can be used for various delayed initialization logic. /// /// void Init(ISessionImplementor session); + + /// + /// Get the query spaces. + /// + /// + /// Query spaces indicates which entity classes are used by the query and need to be flushed + /// when auto-flush is enabled. It also indicates which cache update timestamps needs to be + /// checked for up-to-date-ness. + /// + IEnumerable GetQuerySpaces(); /// - /// Returns commands generated by query + /// Get the commands to execute for getting the not-already cached results of this query. /// + /// The commands for obtaining the results not already cached. IEnumerable GetCommands(); /// @@ -49,18 +60,16 @@ public partial interface IQueryBatchItem int ProcessResultsSet(DbDataReader reader); /// - /// Executed after all commands in batch are processed + /// Process the results of the query, including cached results. /// + /// Any result from the database must have been previously processed + /// through . void ProcessResults(); /// - /// Immediate query execution in case the dialect does not support batches + /// Execute immediately the query as a single standalone query. Used in case the data-provider + /// does not support batches. /// void ExecuteNonBatched(); - - /// - /// Get cache query spaces - /// - IEnumerable GetQuerySpaces(); } } diff --git a/src/NHibernate/Multi/QueryBatch.cs b/src/NHibernate/Multi/QueryBatch.cs index 66f50a32ab5..1720294d6b1 100644 --- a/src/NHibernate/Multi/QueryBatch.cs +++ b/src/NHibernate/Multi/QueryBatch.cs @@ -144,7 +144,7 @@ protected void ExecuteBatched() // calling CombineQueries. CombineQueries(resultSetsCommand); - bool statsEnabled = Session.Factory.Statistics.IsStatisticsEnabled; + var statsEnabled = Session.Factory.Statistics.IsStatisticsEnabled; Stopwatch stopWatch = null; if (statsEnabled) { @@ -157,7 +157,7 @@ protected void ExecuteBatched() Log.Debug("Multi query with {0} queries: {1}", _queries.Count, resultSetsCommand.Sql); } - int rowCount = 0; + var rowCount = 0; try { if (resultSetsCommand.HasQueries) diff --git a/src/NHibernate/Multi/QueryBatchItemBase.cs b/src/NHibernate/Multi/QueryBatchItemBase.cs index dfde5d3638f..1e0abe7439d 100644 --- a/src/NHibernate/Multi/QueryBatchItemBase.cs +++ b/src/NHibernate/Multi/QueryBatchItemBase.cs @@ -37,6 +37,7 @@ protected class QueryLoadInfo protected abstract List GetQueryLoadInfo(); + /// public virtual void Init(ISessionImplementor session) { Session = session; @@ -50,11 +51,13 @@ public virtual void Init(ISessionImplementor session) _finalResults = null; } - /// - /// Gets the commands to execute for getting the not-already cached results of this query. Does retrieves - /// already cached results by side-effect. - /// - /// The commands for obtaining the results not already cached. + /// + public IEnumerable GetQuerySpaces() + { + return _queryInfos.SelectMany(q => q.QuerySpaces); + } + + /// public IEnumerable GetCommands() { for (var index = 0; index < _queryInfos.Count; index++) @@ -82,6 +85,7 @@ public IEnumerable GetCommands() } } + /// public int ProcessResultsSet(DbDataReader reader) { var dialect = Session.Factory.Dialect; @@ -161,9 +165,10 @@ public int ProcessResultsSet(DbDataReader reader) return rowCount; } + /// public void ProcessResults() { - for (int i = 0; i < _queryInfos.Count; i++) + for (var i = 0; i < _queryInfos.Count; i++) { var queryInfo = _queryInfos[i]; if (_subselectResultKeys[i] != null) @@ -192,17 +197,13 @@ public void ProcessResults() AfterLoadCallback?.Invoke(GetResults()); } + /// public void ExecuteNonBatched() { _finalResults = GetResultsNonBatched(); AfterLoadCallback?.Invoke(_finalResults); } - public IEnumerable GetQuerySpaces() - { - return _queryInfos.SelectMany(q => q.QuerySpaces); - } - protected abstract IList GetResultsNonBatched(); protected List GetTypedResults() @@ -211,8 +212,8 @@ protected List GetTypedResults() { throw new HibernateException("Batch wasn't executed. You must call IQueryBatch.Execute() before accessing results."); } - List results = new List(_loaderResults.Sum(tr => tr.Count)); - for (int i = 0; i < _queryInfos.Count; i++) + var results = new List(_loaderResults.Sum(tr => tr.Count)); + for (var i = 0; i < _queryInfos.Count; i++) { var list = _queryInfos[i].Loader.GetResultList( _loaderResults[i], @@ -223,11 +224,13 @@ protected List GetTypedResults() return results; } + /// public IList GetResults() { return _finalResults ?? (_finalResults = DoGetResults()); } + /// public Action> AfterLoadCallback { get; set; } protected abstract List DoGetResults(); From 5e00be677f14b122224c299304354493b572c6de Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20Delaporte?= <12201973+fredericDelaporte@users.noreply.github.com> Date: Wed, 11 Jul 2018 09:43:57 +0200 Subject: [PATCH 3/3] fixup! Fix shortcomings of QueryBatcher initial implementation Fix SQL query for broader compatibility --- src/NHibernate.Test/Async/Futures/QueryBatchFixture.cs | 10 +++++----- src/NHibernate.Test/Futures/QueryBatchFixture.cs | 10 +++++----- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/src/NHibernate.Test/Async/Futures/QueryBatchFixture.cs b/src/NHibernate.Test/Async/Futures/QueryBatchFixture.cs index 8e0ec5061cb..87261158243 100644 --- a/src/NHibernate.Test/Async/Futures/QueryBatchFixture.cs +++ b/src/NHibernate.Test/Async/Futures/QueryBatchFixture.cs @@ -286,11 +286,11 @@ public async Task AutoDiscoverWorksWithFutureAsync() { var future = s - .CreateSQLQuery("select count(*) as count from EntitySimpleChild where Name like :pattern") - .AddScalar("count", NHibernateUtil.Int64) + .CreateSQLQuery("select count(*) as childCount from EntitySimpleChild where Name like :pattern") + .AddScalar("childCount", NHibernateUtil.Int64) .SetString("pattern", "Chi%") .SetCacheable(true) - .FutureValue(); + .FutureValue(); Assert.That(await (future.GetValueAsync()), Is.EqualTo(2L), "From DB"); await (t.CommitAsync()); @@ -301,8 +301,8 @@ public async Task AutoDiscoverWorksWithFutureAsync() { var future = s - .CreateSQLQuery("select count(*) as count from EntitySimpleChild where Name like :pattern") - .AddScalar("count", NHibernateUtil.Int64) + .CreateSQLQuery("select count(*) as childCount from EntitySimpleChild where Name like :pattern") + .AddScalar("childCount", NHibernateUtil.Int64) .SetString("pattern", "Chi%") .SetCacheable(true) .FutureValue(); diff --git a/src/NHibernate.Test/Futures/QueryBatchFixture.cs b/src/NHibernate.Test/Futures/QueryBatchFixture.cs index d5d46833d6f..c872d8bc2fc 100644 --- a/src/NHibernate.Test/Futures/QueryBatchFixture.cs +++ b/src/NHibernate.Test/Futures/QueryBatchFixture.cs @@ -274,11 +274,11 @@ public void AutoDiscoverWorksWithFuture() { var future = s - .CreateSQLQuery("select count(*) as count from EntitySimpleChild where Name like :pattern") - .AddScalar("count", NHibernateUtil.Int64) + .CreateSQLQuery("select count(*) as childCount from EntitySimpleChild where Name like :pattern") + .AddScalar("childCount", NHibernateUtil.Int64) .SetString("pattern", "Chi%") .SetCacheable(true) - .FutureValue(); + .FutureValue(); Assert.That(future.Value, Is.EqualTo(2L), "From DB"); t.Commit(); @@ -289,8 +289,8 @@ public void AutoDiscoverWorksWithFuture() { var future = s - .CreateSQLQuery("select count(*) as count from EntitySimpleChild where Name like :pattern") - .AddScalar("count", NHibernateUtil.Int64) + .CreateSQLQuery("select count(*) as childCount from EntitySimpleChild where Name like :pattern") + .AddScalar("childCount", NHibernateUtil.Int64) .SetString("pattern", "Chi%") .SetCacheable(true) .FutureValue();