Skip to content

Fix future queries with non lazy associations #2174

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Jul 16, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions src/AsyncGenerator.yml
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,10 @@
applyChanges: true
analyzation:
methodConversion:
#TODO 6.0: Remove ignore rule for IQueryBatchItem.ProcessResults
- conversion: Ignore
name: ProcessResults
containingTypeName: IQueryBatchItem
- conversion: Ignore
name: PostProcessInsert
containingTypeName: HqlSqlWalker
Expand Down
41 changes: 41 additions & 0 deletions src/NHibernate.Test/Async/Futures/QueryBatchFixture.cs
Original file line number Diff line number Diff line change
Expand Up @@ -498,6 +498,35 @@ public async Task CacheModeWorksWithFutureAsync()
}
}

//GH-2173
[Test]
public async Task CanFetchNonLazyEntitiesInSubsequentQueryAsync()
{
Sfi.Statistics.IsStatisticsEnabled = true;
using (var s = OpenSession())
using (var t = s.BeginTransaction())
{
await (s.SaveAsync(
new EntityEager
{
Name = "EagerManyToOneAssociation",
EagerEntity = new EntityEagerChild {Name = "association"}
}));
await (t.CommitAsync());
}

using (var s = OpenSession())
{
Sfi.Statistics.Clear();
//EntityEager.EagerEntity is lazy initialized instead of being loaded by the second query
s.QueryOver<EntityEager>().Fetch(SelectMode.Skip, x => x.EagerEntity).Future();
await (s.QueryOver<EntityEager>().Fetch(SelectMode.Fetch, x => x.EagerEntity).Future().GetEnumerableAsync());

if(SupportsMultipleQueries)
Assert.That(Sfi.Statistics.PrepareStatementCount, Is.EqualTo(1));
}
}

#region Test Setup

protected override HbmMapping GetMappings()
Expand Down Expand Up @@ -543,6 +572,10 @@ protected override HbmMapping GetMappings()
rc.Id(x => x.Id, m => m.Generator(Generators.GuidComb));
rc.Property(x => x.Name);

rc.ManyToOne(x => x.EagerEntity, m =>
{
m.Cascade(Mapping.ByCode.Cascade.Persist);
});
rc.Bag(ep => ep.ChildrenListSubselect,
m =>
{
Expand All @@ -560,6 +593,14 @@ protected override HbmMapping GetMappings()
},
a => a.OneToMany());
});
mapper.Class<EntityEagerChild>(
rc =>
{
rc.Lazy(false);

rc.Id(x => x.Id, m => m.Generator(Generators.GuidComb));
rc.Property(x => x.Name);
});
mapper.Class<EntitySubselectChild>(
rc =>
{
Expand Down
7 changes: 7 additions & 0 deletions src/NHibernate.Test/Futures/Entities.cs
Original file line number Diff line number Diff line change
Expand Up @@ -36,11 +36,18 @@ public class EntitySubselectChild
public virtual EntityEager Parent { get; set; }
}

public class EntityEagerChild
{
public Guid Id { get; set; }
public string Name { get; set; }
}

public class EntityEager
{
public Guid Id { get; set; }
public string Name { get; set; }

public EntityEagerChild EagerEntity { get; set; }
public IList<EntitySubselectChild> ChildrenListSubselect { get; set; }
public IList<EntitySimpleChild> ChildrenListEager { get; set; } //= new HashSet<EntitySimpleChild>();
}
Expand Down
41 changes: 41 additions & 0 deletions src/NHibernate.Test/Futures/QueryBatchFixture.cs
Original file line number Diff line number Diff line change
Expand Up @@ -486,6 +486,35 @@ public void CacheModeWorksWithFuture()
}
}

//GH-2173
[Test]
public void CanFetchNonLazyEntitiesInSubsequentQuery()
{
Sfi.Statistics.IsStatisticsEnabled = true;
using (var s = OpenSession())
using (var t = s.BeginTransaction())
{
s.Save(
new EntityEager
{
Name = "EagerManyToOneAssociation",
EagerEntity = new EntityEagerChild {Name = "association"}
});
t.Commit();
}

using (var s = OpenSession())
{
Sfi.Statistics.Clear();
//EntityEager.EagerEntity is lazy initialized instead of being loaded by the second query
s.QueryOver<EntityEager>().Fetch(SelectMode.Skip, x => x.EagerEntity).Future();
s.QueryOver<EntityEager>().Fetch(SelectMode.Fetch, x => x.EagerEntity).Future().GetEnumerable();

if(SupportsMultipleQueries)
Assert.That(Sfi.Statistics.PrepareStatementCount, Is.EqualTo(1));
}
}

#region Test Setup

protected override HbmMapping GetMappings()
Expand Down Expand Up @@ -531,6 +560,10 @@ protected override HbmMapping GetMappings()
rc.Id(x => x.Id, m => m.Generator(Generators.GuidComb));
rc.Property(x => x.Name);

rc.ManyToOne(x => x.EagerEntity, m =>
{
m.Cascade(Mapping.ByCode.Cascade.Persist);
});
rc.Bag(ep => ep.ChildrenListSubselect,
m =>
{
Expand All @@ -548,6 +581,14 @@ protected override HbmMapping GetMappings()
},
a => a.OneToMany());
});
mapper.Class<EntityEagerChild>(
rc =>
{
rc.Lazy(false);

rc.Id(x => x.Id, m => m.Generator(Generators.GuidComb));
rc.Property(x => x.Name);
});
mapper.Class<EntitySubselectChild>(
rc =>
{
Expand Down
5 changes: 5 additions & 0 deletions src/NHibernate/Async/Multi/IQueryBatchItem.cs
Original file line number Diff line number Diff line change
Expand Up @@ -36,4 +36,9 @@ public partial interface IQueryBatchItem
/// <param name="cancellationToken">A cancellation token that can be used to cancel the work</param>
Task ExecuteNonBatchedAsync(CancellationToken cancellationToken);
}

internal partial interface IQueryBatchItemWithAsyncProcessResults
{
Task ProcessResultsAsync(CancellationToken cancellationToken);
}
}
27 changes: 19 additions & 8 deletions src/NHibernate/Async/Multi/QueryBatch.cs
Original file line number Diff line number Diff line change
Expand Up @@ -127,37 +127,48 @@ protected async Task ExecuteBatchedAsync(CancellationToken cancellationToken)
}

var rowCount = 0;
CacheBatcher cacheBatcher = null;
try
{
if (resultSetsCommand.HasQueries)
{
cacheBatcher = new CacheBatcher(Session);
using (var reader = await (resultSetsCommand.GetReaderAsync(Timeout, cancellationToken)).ConfigureAwait(false))
{
var cacheBatcher = new CacheBatcher(Session);
foreach (var query in _queries)
{
if (query.CachingInformation != null)
{
foreach (var cachingInfo in query.CachingInformation.Where(ci => ci.IsCacheable))
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cacheBatcher is also needed when query is not cacheable but loaded entities are cacheable - so it's better to always set it.

foreach (var cachingInfo in query.CachingInformation)
{
cachingInfo.SetCacheBatcher(cacheBatcher);
}
}

rowCount += await (query.ProcessResultsSetAsync(reader, cancellationToken)).ConfigureAwait(false);
}
await (cacheBatcher.ExecuteBatchAsync(cancellationToken)).ConfigureAwait(false);
}
}

// Query cacheable results must be cached untransformed: the put does not need to wait for
// the ProcessResults.
await (PutCacheableResultsAsync(cancellationToken)).ConfigureAwait(false);

foreach (var query in _queries)
{
query.ProcessResults();
//TODO 6.0: Replace with query.ProcessResults();
if (query is IQueryBatchItemWithAsyncProcessResults q)
await (q.ProcessResultsAsync(cancellationToken)).ConfigureAwait(false);
else
query.ProcessResults();
}

var executeBatchTask = cacheBatcher?.ExecuteBatchAsync(cancellationToken);

if (executeBatchTask != null)

{

await (executeBatchTask).ConfigureAwait(false);

}
await (PutCacheableResultsAsync(cancellationToken)).ConfigureAwait(false);
}
catch (OperationCanceledException) { throw; }
catch (Exception sqle)
Expand Down
37 changes: 33 additions & 4 deletions src/NHibernate/Async/Multi/QueryBatchItemBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ namespace NHibernate.Multi
{
using System.Threading.Tasks;
using System.Threading;
public abstract partial class QueryBatchItemBase<TResult> : IQueryBatchItem<TResult>
public abstract partial class QueryBatchItemBase<TResult> : IQueryBatchItem<TResult>, IQueryBatchItemWithAsyncProcessResults
{

/// <inheritdoc />
Expand Down Expand Up @@ -103,17 +103,46 @@ public async Task<int> ProcessResultsSetAsync(DbDataReader reader, CancellationT

queryInfo.Result = tmpResults;
if (queryInfo.CanPutToCache)
queryInfo.ResultToCache = tmpResults;
queryInfo.ResultToCache = new List<object>(tmpResults);
Copy link
Member Author

@bahusoid bahusoid Jun 13, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When merging to master this change should be ignored. The following code is expected in master (no need to wrap in new List):

queryInfo.ResultToCache = queryCacheBuilder.Result;


await (reader.NextResultAsync(cancellationToken)).ConfigureAwait(false);
}

await (InitializeEntitiesAndCollectionsAsync(reader, hydratedObjects, cancellationToken)).ConfigureAwait(false);

StopLoadingCollections(reader);
_reader = reader;
_hydratedObjects = hydratedObjects;
return rowCount;
}
}

/// <inheritdoc cref="IQueryBatchItem.ProcessResults" />
public async Task ProcessResultsAsync(CancellationToken cancellationToken)
{
cancellationToken.ThrowIfCancellationRequested();
ThrowIfNotInitialized();

using (Session.SwitchCacheMode(_cacheMode))
await (InitializeEntitiesAndCollectionsAsync(_reader, _hydratedObjects, cancellationToken)).ConfigureAwait(false);

for (var i = 0; i < _queryInfos.Count; i++)
{
var queryInfo = _queryInfos[i];
if (_subselectResultKeys[i] != null)
{
queryInfo.Loader.CreateSubselects(_subselectResultKeys[i], queryInfo.Parameters, Session);
}

if (queryInfo.IsCacheable)
{
// This transformation must not be applied to ResultToCache.
queryInfo.Result =
queryInfo.Loader.TransformCacheableResults(
queryInfo.Parameters, queryInfo.CacheKey.ResultTransformer, queryInfo.Result);
}
}
AfterLoadCallback?.Invoke(GetResults());
}

/// <inheritdoc />
public async Task ExecuteNonBatchedAsync(CancellationToken cancellationToken)
{
Expand Down
14 changes: 14 additions & 0 deletions src/NHibernate/Engine/Loading/CollectionLoadContext.cs
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,8 @@ public IPersistentCollection GetLoadingCollection(ICollectionPersister persister
}
else
{
if (loadingCollectionEntry.StopLoading)
return null;
if (loadingCollectionEntry.ResultSet == resultSet)
{
log.Debug("found loading collection bound to current result set processing; reading row");
Expand Down Expand Up @@ -398,5 +400,17 @@ public override string ToString()
{
return base.ToString() + "<rs=" + ResultSet + ">";
}

internal void StopLoadingCollections(ICollectionPersister[] collectionPersisters)
{
foreach (var collectionKey in localLoadingCollectionKeys)
{
var loadingCollectionEntry = LoadContext.LocateLoadingCollectionEntry(collectionKey);
if (loadingCollectionEntry != null && Array.IndexOf(collectionPersisters, loadingCollectionEntry.Persister) >= 0)
{
loadingCollectionEntry.StopLoading = true;
}
}
}
}
}
2 changes: 2 additions & 0 deletions src/NHibernate/Engine/Loading/LoadingCollectionEntry.cs
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,8 @@ public IPersistentCollection Collection
get { return collection; }
}

public bool StopLoading { get; set; }

public override string ToString()
{
return GetType().FullName + "<rs=" + ResultSet + ", coll=" + MessageHelper.InfoString(Persister.Role, Key) + ">@" + Convert.ToString(GetHashCode(), 16);
Expand Down
12 changes: 12 additions & 0 deletions src/NHibernate/Loader/Loader.cs
Original file line number Diff line number Diff line change
Expand Up @@ -672,6 +672,18 @@ internal void InitializeEntitiesAndCollections(
}
}

/// <summary>
/// Stops further collection population without actual collection initialization.
/// </summary>
internal void StopLoadingCollections(ISessionImplementor session, DbDataReader reader)
{
var collectionPersisters = CollectionPersisters;
if (collectionPersisters == null || collectionPersisters.Length == 0)
return;

session.PersistenceContext.LoadContexts.GetCollectionLoadContext(reader).StopLoadingCollections(collectionPersisters);
}

private void EndCollectionLoad(DbDataReader reader, ISessionImplementor session, ICollectionPersister collectionPersister)
{
//this is a query and we are loading multiple instances of the same collection role
Expand Down
6 changes: 6 additions & 0 deletions src/NHibernate/Multi/IQueryBatchItem.cs
Original file line number Diff line number Diff line change
Expand Up @@ -80,4 +80,10 @@ public partial interface IQueryBatchItem
/// </summary>
void ExecuteNonBatched();
}

//TODO 6.0: Remove along with ignore rule for IQueryBatchItem.ProcessResults in AsyncGenerator.yml
internal partial interface IQueryBatchItemWithAsyncProcessResults
{
void ProcessResults();
}
}
19 changes: 11 additions & 8 deletions src/NHibernate/Multi/QueryBatch.cs
Original file line number Diff line number Diff line change
Expand Up @@ -153,37 +153,40 @@ protected void ExecuteBatched()
}

var rowCount = 0;
CacheBatcher cacheBatcher = null;
try
{
if (resultSetsCommand.HasQueries)
{
cacheBatcher = new CacheBatcher(Session);
using (var reader = resultSetsCommand.GetReader(Timeout))
{
var cacheBatcher = new CacheBatcher(Session);
foreach (var query in _queries)
{
if (query.CachingInformation != null)
{
foreach (var cachingInfo in query.CachingInformation.Where(ci => ci.IsCacheable))
foreach (var cachingInfo in query.CachingInformation)
{
cachingInfo.SetCacheBatcher(cacheBatcher);
}
}

rowCount += query.ProcessResultsSet(reader);
}
cacheBatcher.ExecuteBatch();
}
}

// Query cacheable results must be cached untransformed: the put does not need to wait for
// the ProcessResults.
PutCacheableResults();

foreach (var query in _queries)
{
query.ProcessResults();
//TODO 6.0: Replace with query.ProcessResults();
if (query is IQueryBatchItemWithAsyncProcessResults q)
q.ProcessResults();
else
query.ProcessResults();
}

cacheBatcher?.ExecuteBatch();
PutCacheableResults();
}
catch (Exception sqle)
{
Expand Down
Loading