Skip to content

Commit 7f3dec8

Browse files
fredericDelaportefredericDelaporte
committed
Fix filter/where fragment appended after lock hint
When a dialect has a read-only lock hint, the filter and/or collection's where fragment is added after the lock hint, which generates an invalid query.
1 parent adc1aaf commit 7f3dec8

File tree

5 files changed

+234
-29
lines changed

5 files changed

+234
-29
lines changed

‎src/NHibernate.Test/CollectionTest/Domain.cs

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
using System.Collections.Generic;
2+
3+
namespace NHibernate.Test.CollectionTest
4+
{
5+
public class Env
6+
{
7+
public virtual long Id { get; set; }
8+
public virtual IList<MachineRequest> RequestsFailed { get; set; }
9+
public virtual IDictionary<long, MachineRequest> FailedRequestsById { get; set; }
10+
}
11+
12+
public class MachineRequest
13+
{
14+
public virtual long Id { get; set; }
15+
public virtual int RequestCompletionStatus { get; set; }
16+
public virtual long EnvId { get; set; }
17+
}
18+
}

‎src/NHibernate.Test/CollectionTest/Domain.hbm.xml

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
<?xml version="1.0" encoding="utf-8" ?>
2+
<hibernate-mapping xmlns="urn:nhibernate-mapping-2.2"
3+
namespace="NHibernate.Test.CollectionTest"
4+
assembly="NHibernate.Test">
5+
6+
<class name="Env">
7+
<id name="Id">
8+
<generator class="assigned"/>
9+
</id>
10+
<bag name="RequestsFailed" inverse="true" lazy="extra"
11+
where="RequestCompletionStatus != 1">
12+
<key column="EnvId"/>
13+
<one-to-many class="MachineRequest"/>
14+
<filter name="CurrentOnly"/>
15+
</bag>
16+
<map name="FailedRequestsById" inverse="true"
17+
where="RequestCompletionStatus != 1">
18+
<key column="EnvId"/>
19+
<map-key type="Int64" column="Id"/>
20+
<one-to-many class="MachineRequest"/>
21+
<filter name="CurrentOnly"/>
22+
</map>
23+
</class>
24+
25+
<class name="MachineRequest">
26+
<id name="Id">
27+
<generator class="assigned"/>
28+
</id>
29+
<property name="EnvId"/>
30+
<property name="RequestCompletionStatus"/>
31+
</class>
32+
33+
<filter-def name="CurrentOnly" condition="1 = 0"/>
34+
</hibernate-mapping>

‎src/NHibernate.Test/CollectionTest/WhereWithReadLockFixture.cs

Lines changed: 129 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,129 @@
1+
using System.Reflection;
2+
using NHibernate.Cfg;
3+
using NHibernate.Dialect;
4+
using NHibernate.Engine;
5+
using NHibernate.Id;
6+
using NHibernate.Persister.Collection;
7+
using NHibernate.SqlCommand;
8+
using NUnit.Framework;
9+
10+
namespace NHibernate.Test.CollectionTest
11+
{
12+
public class DialectWithReadLockHint : GenericDialect
13+
{
14+
public const string ReadOnlyLock = " for read only";
15+
16+
public override string GetForUpdateString(LockMode lockMode)
17+
{
18+
if (lockMode == LockMode.Read)
19+
{
20+
return ReadOnlyLock;
21+
}
22+
23+
return string.Empty;
24+
}
25+
}
26+
27+
// SQL Anywhere has this, but has no CI currently. So testing with an ad-hoc generic dialect.
28+
// Trouble originally spotted with NHSpecificTest.BagWithLazyExtraAndFilter.Fixture.CanUseFilterForLazyExtra
29+
// test, which was locally failing for SQL Anywhere.
30+
[TestFixture]
31+
public class WhereWithReadLockFixture
32+
{
33+
private ISessionFactoryImplementor _sessionFactory;
34+
private ICollectionPersister _bagPersister;
35+
private ICollectionPersister _mapPersister;
36+
37+
[OneTimeSetUp]
38+
public void OneTimeSetUp()
39+
{
40+
var configuration = TestConfigurationHelper.GetDefaultConfiguration();
41+
configuration.AddResource(
42+
typeof(WhereWithReadLockFixture).Namespace + ".Domain.hbm.xml",
43+
typeof(WhereWithReadLockFixture).Assembly);
44+
configuration.SetProperty(Environment.Dialect, typeof(DialectWithReadLockHint).AssemblyQualifiedName);
45+
46+
_sessionFactory = (ISessionFactoryImplementor) configuration.BuildSessionFactory();
47+
_bagPersister =
48+
_sessionFactory.GetCollectionPersister(typeof(Env).FullName + "." + nameof(Env.RequestsFailed));
49+
Assert.That(
50+
_bagPersister,
51+
Is.InstanceOf(typeof(AbstractCollectionPersister)),
52+
"Unexpected bag persister type");
53+
_mapPersister =
54+
_sessionFactory.GetCollectionPersister(typeof(Env).FullName + "." + nameof(Env.FailedRequestsById));
55+
Assert.That(
56+
_mapPersister,
57+
Is.InstanceOf(typeof(AbstractCollectionPersister)),
58+
"Unexpected map persister type");
59+
}
60+
61+
[OneTimeTearDown]
62+
public void OneTimeTearDown()
63+
{
64+
_sessionFactory?.Dispose();
65+
}
66+
67+
[Test]
68+
public void GenerateLockHintAtEndForExtraLazyCount()
69+
{
70+
var selectMethod = typeof(AbstractCollectionPersister).GetMethod(
71+
"GenerateSelectSizeString",
72+
BindingFlags.Instance | BindingFlags.NonPublic);
73+
Assert.That(selectMethod, Is.Not.Null, "Unable to find GenerateSelectSizeString method");
74+
75+
using (var s = _sessionFactory.OpenSession())
76+
{
77+
var select = (SqlString) selectMethod.Invoke(_bagPersister, new object[] { s });
78+
Assert.That(select.ToString(), Does.EndWith(DialectWithReadLockHint.ReadOnlyLock));
79+
80+
s.EnableFilter("CurrentOnly");
81+
select = (SqlString) selectMethod.Invoke(_bagPersister, new object[] { s });
82+
Assert.That(select.ToString(), Does.EndWith(DialectWithReadLockHint.ReadOnlyLock));
83+
}
84+
}
85+
86+
[Test]
87+
public void GenerateLockHintAtEndForDetectRowByIndex()
88+
{
89+
var sqlField = typeof(AbstractCollectionPersister).GetField(
90+
"sqlDetectRowByIndexString",
91+
BindingFlags.Instance | BindingFlags.NonPublic);
92+
Assert.That(sqlField, Is.Not.Null, "Unable to find sqlDetectRowByIndexString field");
93+
94+
var sql = (SqlString) sqlField.GetValue(_mapPersister);
95+
Assert.That(sql.ToString(), Does.EndWith(DialectWithReadLockHint.ReadOnlyLock));
96+
}
97+
98+
[Test]
99+
public void GenerateLockHintAtEndForSelectRowByIndex()
100+
{
101+
var sqlField = typeof(AbstractCollectionPersister).GetField(
102+
"sqlSelectRowByIndexString",
103+
BindingFlags.Instance | BindingFlags.NonPublic);
104+
Assert.That(sqlField, Is.Not.Null, "Unable to find sqlSelectRowByIndexString field");
105+
106+
var sql = (SqlString) sqlField.GetValue(_mapPersister);
107+
Assert.That(sql.ToString(), Does.EndWith(DialectWithReadLockHint.ReadOnlyLock));
108+
}
109+
110+
[Test]
111+
public void GenerateLockHintAtEndForDetectRowByElement()
112+
{
113+
var sqlField = typeof(AbstractCollectionPersister).GetField(
114+
"sqlDetectRowByElementString",
115+
BindingFlags.Instance | BindingFlags.NonPublic);
116+
Assert.That(sqlField, Is.Not.Null, "Unable to find sqlDetectRowByElementString field");
117+
118+
var sql = (SqlString) sqlField.GetValue(_mapPersister);
119+
Assert.That(sql.ToString(), Does.EndWith(DialectWithReadLockHint.ReadOnlyLock));
120+
}
121+
122+
[Test]
123+
public void GenerateLockHintAtEndForSelectByUniqueKey()
124+
{
125+
var sql = ((IPostInsertIdentityPersister) _bagPersister).GetSelectByUniqueKeyString("blah");
126+
Assert.That(sql.ToString(), Does.EndWith(DialectWithReadLockHint.ReadOnlyLock));
127+
}
128+
}
129+
}

‎src/NHibernate/Persister/Collection/AbstractCollectionPersister.cs

Lines changed: 34 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -866,23 +866,25 @@ public string SelectFragment(string alias, string columnSuffix)
866866

867867
return frag.ToSqlStringFragment(false);
868868
}
869-
private SqlString AddWhereFragment(SqlString sql)
869+
870+
private void AddWhereFragment(SqlSimpleSelectBuilder sql)
870871
{
871872
if (!hasWhere)
872-
return sql;
873-
return sql.Append(" and ").Append(sqlWhereString);
873+
return;
874+
sql.AddWhereFragment(sqlWhereString);
874875
}
875876

876877
private SqlString GenerateSelectSizeString(ISessionImplementor sessionImplementor)
877878
{
878-
string selectValue = GetCountSqlSelectClause();
879+
var selectValue = GetCountSqlSelectClause();
879880

880-
return new SqlSimpleSelectBuilder(dialect, factory)
881-
.SetTableName(TableName)
882-
.AddWhereFragment(KeyColumnNames, KeyType, "=")
883-
.AddColumn(selectValue)
884-
.ToSqlString()
885-
.Append(FilterFragment(TableName, sessionImplementor.EnabledFilters));
881+
return
882+
new SqlSimpleSelectBuilder(dialect, factory)
883+
.SetTableName(TableName)
884+
.AddWhereFragment(KeyColumnNames, KeyType, "=")
885+
.AddWhereFragment(FilterFragment(TableName, sessionImplementor.EnabledFilters))
886+
.AddColumn(selectValue)
887+
.ToSqlString();
886888
}
887889

888890
protected virtual string GetCountSqlSelectClause()
@@ -906,14 +908,15 @@ private SqlString GenerateDetectRowByIndexString()
906908
}
907909

908910
// TODO NH: may be we need something else when Index is mixed with Formula
909-
var sqlString=
911+
var builder =
910912
new SqlSimpleSelectBuilder(dialect, factory)
911913
.SetTableName(TableName)
912914
.AddWhereFragment(KeyColumnNames, KeyType, "=")
913915
.AddWhereFragment(IndexColumnNames, IndexType, "=")
914916
.AddWhereFragment(indexFormulas, IndexType, "=")
915-
.AddColumn("1").ToSqlString();
916-
return AddWhereFragment(sqlString);
917+
.AddColumn("1");
918+
AddWhereFragment(builder);
919+
return builder.ToSqlString();
917920
}
918921

919922
private SqlString GenerateSelectRowByIndexString()
@@ -923,26 +926,29 @@ private SqlString GenerateSelectRowByIndexString()
923926
return null;
924927
}
925928

926-
var sqlString=new SqlSimpleSelectBuilder(dialect, factory)
927-
.SetTableName(TableName)
928-
.AddWhereFragment(KeyColumnNames, KeyType, "=")
929-
.AddWhereFragment(IndexColumnNames, IndexType, "=")
930-
.AddWhereFragment(indexFormulas, IndexType, "=")
931-
.AddColumns(ElementColumnNames, elementColumnAliases)
932-
.AddColumns(indexFormulas, indexColumnAliases).ToSqlString();
933-
return AddWhereFragment(sqlString);
929+
var builder =
930+
new SqlSimpleSelectBuilder(dialect, factory)
931+
.SetTableName(TableName)
932+
.AddWhereFragment(KeyColumnNames, KeyType, "=")
933+
.AddWhereFragment(IndexColumnNames, IndexType, "=")
934+
.AddWhereFragment(indexFormulas, IndexType, "=")
935+
.AddColumns(ElementColumnNames, elementColumnAliases)
936+
.AddColumns(indexFormulas, indexColumnAliases);
937+
AddWhereFragment(builder);
938+
return builder.ToSqlString();
934939
}
935940

936941
private SqlString GenerateDetectRowByElementString()
937942
{
938-
var sqlString=
943+
var builder =
939944
new SqlSimpleSelectBuilder(dialect, factory)
940-
.SetTableName(TableName)
941-
.AddWhereFragment(KeyColumnNames, KeyType, "=")
942-
.AddWhereFragment(ElementColumnNames, ElementType, "=")
943-
.AddWhereFragment(elementFormulas, ElementType, "=")
944-
.AddColumn("1").ToSqlString();
945-
return AddWhereFragment(sqlString);
945+
.SetTableName(TableName)
946+
.AddWhereFragment(KeyColumnNames, KeyType, "=")
947+
.AddWhereFragment(ElementColumnNames, ElementType, "=")
948+
.AddWhereFragment(elementFormulas, ElementType, "=")
949+
.AddColumn("1");
950+
AddWhereFragment(builder);
951+
return builder.ToSqlString();
946952
}
947953

948954
protected virtual SelectFragment GenerateSelectFragment(string alias, string columnSuffix)

‎src/NHibernate/SqlCommand/SqlSimpleSelectBuilder.cs

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
using System;
12
using System.Collections.Generic;
23
using NHibernate.Engine;
34
using NHibernate.Type;
@@ -170,6 +171,23 @@ public SqlSimpleSelectBuilder AddWhereFragment(string[] columnNames, IType type,
170171
return this;
171172
}
172173

174+
/// <summary>
175+
/// Adds an arbitrary where fragment.
176+
/// </summary>
177+
/// <param name="fragment">The fragment.</param>
178+
/// <returns>The SqlSimpleSelectBuilder</returns>
179+
public SqlSimpleSelectBuilder AddWhereFragment(string fragment)
180+
{
181+
if (string.IsNullOrWhiteSpace(fragment))
182+
return this;
183+
184+
if (fragment.Trim().StartsWith("and ", StringComparison.OrdinalIgnoreCase))
185+
fragment = fragment.Substring(fragment.IndexOf("and", StringComparison.OrdinalIgnoreCase) + 3);
186+
187+
whereStrings.Add(new SqlString(fragment));
188+
return this;
189+
}
190+
173191
public virtual SqlSimpleSelectBuilder SetComment(System.String comment)
174192
{
175193
this.comment = comment;
@@ -238,4 +256,4 @@ public SqlString ToSqlString()
238256

239257
#endregion
240258
}
241-
}
259+
}

0 commit comments

Comments
 (0)