Skip to content

Support caching queries with autodiscovered types #28

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

Closed
Closed
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
119 changes: 67 additions & 52 deletions src/NHibernate.Test/SqlTest/Query/NativeSQLQueriesFixture.cs
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
using System.Collections;
using System.Linq;
using NHibernate.Criterion;
using NHibernate.Multi;
using NHibernate.Transform;
using NUnit.Framework;
using NHibernate.Criterion;

namespace NHibernate.Test.SqlTest.Query
{
Expand Down Expand Up @@ -51,6 +52,20 @@ protected override string MappingsAssembly
get { return "NHibernate.Test"; }
}

protected override void OnTearDown()
{
using (var session = OpenSession())
using (var transaction = session.BeginTransaction())
{
session.CreateQuery("delete from Employment").ExecuteUpdate();
session.CreateQuery("delete from System.Object").ExecuteUpdate();

transaction.Commit();
}

Sfi.QueryCache.Clear();
}

[Test]
public void FailOnNoAddEntityOrScalar()
{
Expand Down Expand Up @@ -125,18 +140,6 @@ public void SQLQueryInterface()
t.Commit();
s.Close();
}

using (var s = OpenSession())
using (var t = s.BeginTransaction())
{
s.Delete(emp);
s.Delete(gavin);
s.Delete(ifa);
s.Delete(jboss);

t.Commit();
s.Close();
}
}

[Test]
Expand Down Expand Up @@ -191,18 +194,6 @@ public void SQLQueryInterfaceCacheable()
t.Commit();
s.Close();
}

using (var s = OpenSession())
using (var t = s.BeginTransaction())
{
s.Delete(emp);
s.Delete(gavin);
s.Delete(ifa);
s.Delete(jboss);

t.Commit();
s.Close();
}
}

[Test(Description = "GH-2904")]
Expand Down Expand Up @@ -252,20 +243,11 @@ IList GetCacheableSqlQueryResults()
Assert.That(Sfi.Statistics.QueryCacheHitCount, Is.EqualTo(1), "results are expected from cache");
}
}

using (var s = OpenSession())
using (var t = s.BeginTransaction())
{
s.Delete(emp);
s.Delete(gavin);
s.Delete(ifa);
s.Delete(jboss);
t.Commit();
}
}

class ResultDto
{
public long orgId { get; set; }
public string regionCode { get; set; }
}

Expand Down Expand Up @@ -294,27 +276,67 @@ void AssertQuery(bool fromCache)
.List();
t.Commit();

Assert.AreEqual(1, l.Count);
//TODO: Uncomment if we properly fix caching auto discovery type queries with transformers
// var msg = "results are expected from " + (fromCache ? "cache" : "DB");
// Assert.That(Sfi.Statistics.QueryCacheMissCount, Is.EqualTo(fromCache ? 0 : 1), msg);
// Assert.That(Sfi.Statistics.QueryCacheHitCount, Is.EqualTo(fromCache ? 1 : 0), msg);
Assert.That(l.Count, Is.EqualTo(1));
var msg = "Results are expected from " + (fromCache ? "cache" : "DB");
Assert.That(Sfi.Statistics.QueryCacheMissCount, Is.EqualTo(fromCache ? 0 : 1), msg);
Assert.That(Sfi.Statistics.QueryCacheHitCount, Is.EqualTo(fromCache ? 1 : 0), msg);
}
}


AssertQuery(false);
AssertQuery(true);
}

using (var s = OpenSession())
using (var t = s.BeginTransaction())
[Test(Description = "GH-3169")]
public void CacheableScalarSQLMultiQueryWithTransformer()
{
Organization ifa = new Organization("IFA");

using (ISession s = OpenSession())
using (ITransaction t = s.BeginTransaction())
{
s.Delete(ifa);
s.Save(ifa);
t.Commit();
}

void AssertQuery(bool fromCache)
{
using (var s = OpenSession())
using (var t = s.BeginTransaction())
using (EnableStatisticsScope())
{
var q1 = s.CreateSQLQuery("select org.NAME as regionCode from ORGANIZATION org")
.AddScalar("regionCode", NHibernateUtil.String)
.SetResultTransformer(Transformers.AliasToBean<ResultDto>())
.SetCacheable(true);
var q2 = s.CreateSQLQuery("select org.ORGID as orgId from ORGANIZATION org")
.AddScalar("orgId", NHibernateUtil.Int64)
.SetResultTransformer(Transformers.AliasToBean<ResultDto>())
.SetCacheable(true);

var batch = s.CreateQueryBatch();
batch.Add<ResultDto>(q1);
batch.Add<ResultDto>(q2);
batch.Execute();

var l1 = batch.GetResult<ResultDto>(0);
var l2 = batch.GetResult<ResultDto>(1);

t.Commit();

Assert.That(l1.Count, Is.EqualTo(1), "Unexpected results count for the first query.");
Assert.That(l2.Count, Is.EqualTo(1), "Unexpected results count for the second query.");
var msg = "Results are expected from " + (fromCache ? "cache" : "DB");
Assert.That(Sfi.Statistics.QueryCacheMissCount, Is.EqualTo(fromCache ? 0 : 2), msg);
Assert.That(Sfi.Statistics.QueryCacheHitCount, Is.EqualTo(fromCache ? 2 : 0), msg);
}
}

AssertQuery(false);
AssertQuery(true);
}

[Test]
[Test]
public void ResultSetMappingDefinition()
{
ISession s = OpenSession();
Expand All @@ -339,11 +361,6 @@ public void ResultSetMappingDefinition()
.List();
Assert.AreEqual(l.Count, 1);

s.Delete(emp);
s.Delete(gavin);
s.Delete(ifa);
s.Delete(jboss);

t.Commit();
s.Close();
}
Expand Down Expand Up @@ -426,8 +443,6 @@ public void ScalarValues()
Assert.AreEqual(o[1], "JBoss");
Assert.AreEqual(o[0], idJBoss);

s.Delete(ifa);
s.Delete(jboss);
t.Commit();
s.Close();
}
Expand Down
39 changes: 39 additions & 0 deletions src/NHibernate/Cache/QueryAliasesKey.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
using System;

namespace NHibernate.Cache
{
[Serializable]
public class QueryAliasesKey : IEquatable<QueryAliasesKey>
{
private readonly QueryKey _queryKey;

/// <summary>
/// Initializes a new instance of the <see cref="QueryAliasesKey"/> class.
/// </summary>
/// <param name="queryKey">The <see cref="QueryKey"/> of the query for which aliases have to be stored.</param>
public QueryAliasesKey(QueryKey queryKey)
{
_queryKey = queryKey ?? throw new ArgumentNullException(nameof(queryKey));
}

public override bool Equals(object other)
{
return Equals(other as QueryAliasesKey);
}

public bool Equals(QueryAliasesKey other)
{
return other != null && _queryKey.Equals(other._queryKey);
}

public override int GetHashCode()
{
return _queryKey.GetHashCode();
}

public override string ToString()
{
return "QueryAlisesKey: " + _queryKey.ToString();
}
}
}
68 changes: 64 additions & 4 deletions src/NHibernate/Cache/StandardQueryCache.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
using System.Collections.Generic;
using System.Linq;
using NHibernate.Cfg;
using NHibernate.Collection;
using NHibernate.Engine;
using NHibernate.Persister.Collection;
using NHibernate.Type;
Expand Down Expand Up @@ -86,8 +85,15 @@ public bool Put(
{
// 6.0 TODO: inline the call.
#pragma warning disable 612
return Put(key, returnTypes, result, queryParameters.NaturalKeyLookup, session);
var cached = Put(key, returnTypes, result, queryParameters.NaturalKeyLookup, session);
#pragma warning restore 612

if (cached && key.ResultTransformer?.AutoDiscoverTypes == true && key.ResultTransformer.AutoDiscoveredAliases != null)
{
Cache.Put(new QueryAliasesKey(key), key.ResultTransformer.AutoDiscoveredAliases);
}
Comment on lines +91 to +94
Copy link
Author

Choose a reason for hiding this comment

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

In case we need aliases, this adds them to the cache for allowing retrieving them.

I though about using a local concurrent dictionary, but that would not work with distributed caches. We need theses aliases to be in distributed caches too.

Copy link
Author

Choose a reason for hiding this comment

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

By the way, this works only for non-obsoleted Get. So, the bug would remain for users having an obsolete custom query cache implementation. I think this is acceptable.


return cached;
}

// Since 5.2
Expand Down Expand Up @@ -126,8 +132,22 @@ public IList Get(
{
// 6.0 TODO: inline the call.
#pragma warning disable 612
return Get(key, returnTypes, queryParameters.NaturalKeyLookup, spaces, session);
var result = Get(key, returnTypes, queryParameters.NaturalKeyLookup, spaces, session);
#pragma warning restore 612

if (result != null && key.ResultTransformer?.AutoDiscoverTypes == true && result.Count > 0)
{
var aliasesKey = new QueryAliasesKey(key);
if (!(Cache.Get(aliasesKey) is string[] aliases))
{
// Cannot properly initialize the result transformer, treat it as a cache miss
Log.Debug("query aliases were not found in cache: {0}", aliasesKey);
return null;
}
key.ResultTransformer.SupplyAutoDiscoveredParameters(queryParameters.ResultTransformer, aliases);
}

return result;
}
finally
{
Expand Down Expand Up @@ -184,9 +204,16 @@ public bool[] PutMany(
if (queryParameters[i].NaturalKeyLookup && result.Count == 0)
continue;

var key = keys[i];
cached[i] = true;
cachedKeys.Add(keys[i]);
cachedKeys.Add(key);
cachedResults.Add(GetCacheableResult(returnTypes[i], session, result, ts));

if (key.ResultTransformer?.AutoDiscoverTypes == true && key.ResultTransformer.AutoDiscoveredAliases != null)
{
cachedKeys.Add(new QueryAliasesKey(key));
cachedResults.Add(key.ResultTransformer.AutoDiscoveredAliases);
}
}

_cache.PutMany(cachedKeys.ToArray(), cachedResults.ToArray());
Expand Down Expand Up @@ -244,6 +271,8 @@ public IList[] GetMany(
{
session.PersistenceContext.BatchFetchQueue.InitializeQueryCacheQueue();

var queryAliasesKeys = new QueryAliasesKey[keys.Length];
var hasAliasesToFetch = false;
for (var i = 0; i < keys.Length; i++)
{
var cacheable = (IList) cacheables[i];
Expand All @@ -267,6 +296,37 @@ public IList[] GetMany(

finalReturnTypes[i] = GetReturnTypes(key, returnTypes[i], cacheable);
PerformBeforeAssemble(finalReturnTypes[i], session, cacheable);

if (key.ResultTransformer?.AutoDiscoverTypes == true && cacheable.Count > 0)
{
queryAliasesKeys[i] = new QueryAliasesKey(key);
hasAliasesToFetch = true;
}
}

if (hasAliasesToFetch)
{
var allAliases = _cache.GetMany(queryAliasesKeys.Where(k => k != null).ToArray());

var aliasesIndex = 0;
for (var i = 0; i < keys.Length; i++)
{
var queryAliasesKey = queryAliasesKeys[i];
if (queryAliasesKey == null)
continue;

if (allAliases[aliasesIndex] is string[] aliases)
{
keys[i].ResultTransformer.SupplyAutoDiscoveredParameters(queryParameters[i].ResultTransformer, aliases);
}
else
{
// Cannot properly initialize the result transformer, treat it as a cache miss
Log.Debug("query aliases were not found in cache: {0}", queryAliasesKey);
finalReturnTypes[i] = null;
}
aliasesIndex++;
}
}

for (var i = 0; i < keys.Length; i++)
Expand Down
3 changes: 1 addition & 2 deletions src/NHibernate/Loader/Loader.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1829,8 +1829,7 @@ protected IList List(ISessionImplementor session, QueryParameters queryParameter

internal bool IsCacheable(QueryParameters queryParameters)
{
return _factory.Settings.IsQueryCacheEnabled && queryParameters.Cacheable
&& !(queryParameters.HasAutoDiscoverScalarTypes && queryParameters.ResultTransformer != null);
return _factory.Settings.IsQueryCacheEnabled && queryParameters.Cacheable;
}

private IList ListIgnoreQueryCache(ISessionImplementor session, QueryParameters queryParameters)
Expand Down
7 changes: 7 additions & 0 deletions src/NHibernate/Transform/CacheableResultTransformer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,11 @@ public class CacheableResultTransformer : IResultTransformer

public bool AutoDiscoverTypes { get; }

/// <summary>
/// The auto-discovered aliaises.
/// </summary>
public string[] AutoDiscoveredAliases { get; private set; }

private readonly SqlString _autoDiscoveredQuery;
private readonly bool _skipTransformer;
private int _tupleLength;
Expand Down Expand Up @@ -202,6 +207,8 @@ internal void SupplyAutoDiscoveredParameters(IResultTransformer transformer, str
throw new InvalidOperationException(
"Cannot supply auto-discovered parameters when it is not enabled on the transformer.");

AutoDiscoveredAliases = aliases;

var includeInTuple = ArrayHelper.Fill(true, aliases?.Length ?? 0);
InitializeTransformer(includeInTuple, GetIncludeInTransform(transformer, aliases, includeInTuple));
}
Expand Down