Skip to content

Commit e38566d

Browse files
Doan-Van-TuanfredericDelaporte
Doan-Van-Tuan
authored and
fredericDelaporte
committed
Fix for update with outer join in PostgreSQL (#1568)
* Add alias in "for update" clause to entity loading query containing outer join if database (e.g Postgres) does not support locking with outer join * Fixes #1565
1 parent 55e06d3 commit e38566d

14 files changed

+252
-21
lines changed

‎src/NHibernate.Test/Async/NHSpecificTest/GH1565/LockEntityWithOuterJoinTest.cs

+59
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
//------------------------------------------------------------------------------
2+
// <auto-generated>
3+
// This code was generated by AsyncGenerator.
4+
//
5+
// Changes to this file may cause incorrect behavior and will be lost if
6+
// the code is regenerated.
7+
// </auto-generated>
8+
//------------------------------------------------------------------------------
9+
10+
11+
using NUnit.Framework;
12+
13+
namespace NHibernate.Test.NHSpecificTest.GH1565
14+
{
15+
using System.Threading.Tasks;
16+
[TestFixture]
17+
public class LockEntityWithOuterJoinTestAsync : BugTestCase
18+
{
19+
[Test]
20+
public async Task LockWithOuterJoin_ShouldBePossibleAsync()
21+
{
22+
using (var session = OpenSession())
23+
{
24+
using (var transaction = session.BeginTransaction())
25+
{
26+
var entity = await (session.GetAsync<MainEntity>(id, LockMode.Upgrade));
27+
Assert.That(entity.Id, Is.EqualTo(id));
28+
await (transaction.CommitAsync());
29+
}
30+
}
31+
}
32+
33+
private int id;
34+
protected override void OnSetUp()
35+
{
36+
base.OnSetUp();
37+
using (var session = OpenSession())
38+
{
39+
using (var transaction = session.BeginTransaction())
40+
{
41+
session.FlushMode = FlushMode.Auto;
42+
var entity = new MainEntity();
43+
session.Save(entity);
44+
transaction.Commit();
45+
id = entity.Id;
46+
}
47+
}
48+
}
49+
50+
protected override void OnTearDown()
51+
{
52+
base.OnTearDown();
53+
using (var session = OpenSession())
54+
{
55+
session.CreateSQLQuery("delete from MainEntity").ExecuteUpdate();
56+
}
57+
}
58+
}
59+
}

‎src/NHibernate.Test/NHSpecificTest/GH1565/LockEntityWithOuterJoinTest.cs

+55
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
using NUnit.Framework;
2+
3+
namespace NHibernate.Test.NHSpecificTest.GH1565
4+
{
5+
[TestFixture]
6+
public class LockEntityWithOuterJoinTest : BugTestCase
7+
{
8+
[Test]
9+
public void LockWithOuterJoin_ShouldBePossible()
10+
{
11+
using (var session = OpenSession())
12+
{
13+
using (var transaction = session.BeginTransaction())
14+
{
15+
var entity = session.Get<MainEntity>(id, LockMode.Upgrade);
16+
Assert.That(entity.Id, Is.EqualTo(id));
17+
transaction.Commit();
18+
}
19+
}
20+
}
21+
22+
private int id;
23+
protected override void OnSetUp()
24+
{
25+
base.OnSetUp();
26+
using (var session = OpenSession())
27+
{
28+
using (var transaction = session.BeginTransaction())
29+
{
30+
session.FlushMode = FlushMode.Auto;
31+
var entity = new MainEntity();
32+
session.Save(entity);
33+
transaction.Commit();
34+
id = entity.Id;
35+
}
36+
}
37+
}
38+
39+
protected override void OnTearDown()
40+
{
41+
base.OnTearDown();
42+
using (var session = OpenSession())
43+
{
44+
session.CreateSQLQuery("delete from MainEntity").ExecuteUpdate();
45+
}
46+
}
47+
}
48+
49+
public class MainEntity
50+
{
51+
public virtual int Id { get; set; } = 0;
52+
53+
public virtual string Data { get; set; }
54+
}
55+
}

‎src/NHibernate.Test/NHSpecificTest/GH1565/Mappings.hbm.xml

+15
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
<?xml version="1.0" encoding="utf-8" ?>
2+
<hibernate-mapping xmlns="urn:nhibernate-mapping-2.2" assembly="NHibernate.Test"
3+
namespace="NHibernate.Test.NHSpecificTest.GH1565">
4+
5+
<class name="MainEntity">
6+
<id name="Id" unsaved-value="0">
7+
<generator class="native" />
8+
</id>
9+
10+
<join table="EntityData" inverse="true">
11+
<key column="EntityId"/>
12+
<property name="Data" />
13+
</join>
14+
</class>
15+
</hibernate-mapping>

‎src/NHibernate/Async/Dialect/InformixDialect.cs

+2-1
Original file line numberDiff line numberDiff line change
@@ -8,14 +8,15 @@
88
//------------------------------------------------------------------------------
99

1010

11+
using System;
1112
using System.Data;
1213
using System.Data.Common;
1314
using System.Text;
14-
using NHibernate.Cfg;
1515
using NHibernate.Dialect.Function;
1616
using NHibernate.Exceptions;
1717
using NHibernate.SqlCommand;
1818
using NHibernate.Util;
19+
using Environment = NHibernate.Cfg.Environment;
1920

2021
//using NHibernate.Dialect.Schema;
2122

‎src/NHibernate/Dialect/Dialect.cs

+25-9
Original file line numberDiff line numberDiff line change
@@ -501,15 +501,15 @@ public virtual ILockingStrategy GetLockingStrategy(ILockable lockable, LockMode
501501
/// <returns> The appropriate for update fragment. </returns>
502502
public virtual string GetForUpdateString(LockMode lockMode)
503503
{
504-
if (lockMode == LockMode.Upgrade)
504+
if (Equals(lockMode, LockMode.Upgrade))
505505
{
506506
return ForUpdateString;
507507
}
508-
if (lockMode == LockMode.UpgradeNoWait)
508+
if (Equals(lockMode, LockMode.UpgradeNoWait))
509509
{
510510
return ForUpdateNowaitString;
511511
}
512-
if (lockMode == LockMode.Force)
512+
if (Equals(lockMode, LockMode.Force))
513513
{
514514
return ForUpdateNowaitString;
515515
}
@@ -526,14 +526,30 @@ public virtual string ForUpdateString
526526
get { return " for update"; }
527527
}
528528

529-
/// <summary> Is <tt>FOR UPDATE OF</tt> syntax supported? </summary>
530-
/// <value> True if the database supports <tt>FOR UPDATE OF</tt> syntax; false otherwise. </value>
529+
/// <summary>Is <c>FOR UPDATE OF</c> syntax supported?</summary>
530+
/// <value><see langword="true"/> if the database supports <c>FOR UPDATE OF</c> syntax; <see langword="false"/> otherwise. </value>
531+
public virtual bool SupportsForUpdateOf
532+
// By default, just check UsesColumnsWithForUpdateOf. ForUpdateOf needs to be overriden only for dialects supporting
533+
// "For Update Of" on table aliases.
534+
=> UsesColumnsWithForUpdateOf;
535+
536+
/// <summary>Is <c>FOR UPDATE OF</c> syntax expecting columns?</summary>
537+
/// <value><see langword="true"/> if the database expects a column list with <c>FOR UPDATE OF</c> syntax,
538+
/// <see langword="false"/> if it expects table alias instead or do not support <c>FOR UPDATE OF</c> syntax.</value>
539+
// Since v5.1
540+
[Obsolete("Use UsesColumnsWithForUpdateOf instead")]
531541
public virtual bool ForUpdateOfColumns
532542
{
533543
// by default we report no support
534544
get { return false; }
535545
}
536546

547+
public virtual bool UsesColumnsWithForUpdateOf
548+
#pragma warning disable 618
549+
// For avoiding a breaking change, we need to call the old name by default.
550+
=> ForUpdateOfColumns;
551+
#pragma warning restore 618
552+
537553
/// <summary>
538554
/// Does this dialect support <tt>FOR UPDATE</tt> in conjunction with outer joined rows?
539555
/// </summary>
@@ -567,11 +583,11 @@ public virtual string ForUpdateNowaitString
567583
}
568584

569585
/// <summary>
570-
/// Get the <tt>FOR UPDATE OF column_list NOWAIT</tt> fragment appropriate
571-
/// for this dialect given the aliases of the columns to be write locked.
586+
/// Get the <c>FOR UPDATE OF column_list NOWAIT</c> fragment appropriate
587+
/// for this dialect given the aliases of the columns or tables to be write locked.
572588
/// </summary>
573-
/// <param name="aliases">The columns to be write locked. </param>
574-
/// <returns> The appropriate <tt>FOR UPDATE colunm_list NOWAIT</tt> clause string. </returns>
589+
/// <param name="aliases">The columns or tables to be write locked.</param>
590+
/// <returns>The appropriate <c>FOR UPDATE colunm_or_table_list NOWAIT</c> clause string.</returns>
575591
public virtual string GetForUpdateNowaitString(string aliases)
576592
{
577593
return GetForUpdateString(aliases);

‎src/NHibernate/Dialect/InformixDialect.cs

+10-3
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,12 @@
1+
using System;
12
using System.Data;
23
using System.Data.Common;
34
using System.Text;
4-
using NHibernate.Cfg;
55
using NHibernate.Dialect.Function;
66
using NHibernate.Exceptions;
77
using NHibernate.SqlCommand;
88
using NHibernate.Util;
9+
using Environment = NHibernate.Cfg.Environment;
910

1011
//using NHibernate.Dialect.Schema;
1112

@@ -175,13 +176,19 @@ public override string AddColumnString
175176
// throw new NotSupportedException();
176177
//}
177178

178-
/// <summary> Is <tt>FOR UPDATE OF</tt> syntax supported? </summary>
179-
/// <value> True if the database supports <tt>FOR UPDATE OF</tt> syntax; false otherwise. </value>
179+
/// <inheritdoc />
180+
// Since v5.1
181+
[Obsolete("Use UsesColumnsWithForUpdateOf instead")]
180182
public override bool ForUpdateOfColumns
181183
{
182184
get { return true; }
183185
}
184186

187+
/* 6.0 TODO: uncomment once ForUpdateOfColumns is removed.
188+
/// <inheritdoc />
189+
public override bool UsesColumnsWithForUpdateOf => true;
190+
*/
191+
185192
/// <summary>
186193
/// Does this dialect support <tt>FOR UPDATE</tt> in conjunction with outer joined rows?
187194
/// </summary>

‎src/NHibernate/Dialect/Oracle8iDialect.cs

+7
Original file line numberDiff line numberDiff line change
@@ -485,11 +485,18 @@ public override bool UseMaxForLimit
485485
get { return true; }
486486
}
487487

488+
// Since v5.1
489+
[Obsolete("Use UsesColumnsWithForUpdateOf instead")]
488490
public override bool ForUpdateOfColumns
489491
{
490492
get { return true; }
491493
}
492494

495+
/* 6.0 TODO: uncomment once ForUpdateOfColumns is removed.
496+
/// <inheritdoc />
497+
public override bool UsesColumnsWithForUpdateOf => true;
498+
*/
499+
493500
public override bool SupportsUnionAll
494501
{
495502
get { return true; }

‎src/NHibernate/Dialect/PostgreSQLDialect.cs

+6
Original file line numberDiff line numberDiff line change
@@ -227,6 +227,12 @@ public override SqlString GetLimitString(SqlString queryString, SqlString offset
227227
return pagingBuilder.ToSqlString();
228228
}
229229

230+
/// <inheritdoc />
231+
public override bool SupportsForUpdateOf => true;
232+
233+
/// <inheritdoc />
234+
public override bool SupportsOuterJoinForUpdate => false;
235+
230236
public override string GetForUpdateString(string aliases)
231237
{
232238
return ForUpdateString + " of " + aliases;

‎src/NHibernate/Dialect/SybaseSQLAnywhere10Dialect.cs

+18
Original file line numberDiff line numberDiff line change
@@ -643,11 +643,29 @@ public override string GetForUpdateString(LockMode lockMode)
643643
/// In this dialect, we avoid this issue by supporting only
644644
/// <tt>FOR UPDATE BY LOCK</tt>.
645645
/// </summary>
646+
// Since v5.1
647+
[Obsolete("Use UsesColumnsWithForUpdateOf instead")]
646648
public override bool ForUpdateOfColumns
647649
{
648650
get { return false; }
649651
}
650652

653+
/* 6.0 TODO: uncomment once ForUpdateOfColumns is removed.
654+
/// <summary>
655+
/// SQL Anywhere does support <c>FOR UPDATE OF</c> syntax. However,
656+
/// in SQL Anywhere one cannot specify both <c>FOR UPDATE OF</c> syntax
657+
/// and <c>FOR UPDATE BY LOCK</c> in the same statement. To achieve INTENT
658+
/// locking when using <c>FOR UPDATE OF</c> syntax one must use a table hint
659+
/// in the query's FROM clause, ie.
660+
/// <code>
661+
/// SELECT * FROM FOO WITH( UPDLOCK ) FOR UPDATE OF ( column-list ).
662+
/// </code>
663+
/// In this dialect, we avoid this issue by supporting only
664+
/// <c>FOR UPDATE BY LOCK</c>.
665+
/// </summary>
666+
public override bool UsesColumnsWithForUpdateOf => false;
667+
*/
668+
651669
/// <summary>
652670
/// SQL Anywhere supports <tt>FOR UPDATE</tt> over cursors containing
653671
/// outer joins.

‎src/NHibernate/Loader/AbstractEntityJoinWalker.cs

+1-1
Original file line numberDiff line numberDiff line change
@@ -118,7 +118,7 @@ private void InitStatementString(SqlString projection, SqlString condition, SqlS
118118
JoinFragment ojf = MergeOuterJoins(associations);
119119

120120
SqlSelectBuilder select = new SqlSelectBuilder(Factory)
121-
.SetLockMode(lockMode)
121+
.SetLockMode(lockMode, alias)
122122
.SetSelectClause(selectClause)
123123
.SetFromClause(Dialect.AppendLockHint(lockMode, persister.FromTableFragment(alias)) +persister.FromJoinFragment(alias, true, true))
124124
.SetWhereClause(condition)

‎src/NHibernate/Loader/Criteria/CriteriaLoader.cs

+1-1
Original file line numberDiff line numberDiff line change
@@ -165,7 +165,7 @@ protected override SqlString ApplyLocks(SqlString sqlSelectString, IDictionary<s
165165
}
166166

167167
Dictionary<string, LockMode> aliasedLockModes = new Dictionary<string, LockMode>();
168-
Dictionary<string, string[]> keyColumnNames = dialect.ForUpdateOfColumns ? new Dictionary<string, string[]>() : null;
168+
Dictionary<string, string[]> keyColumnNames = dialect.UsesColumnsWithForUpdateOf ? new Dictionary<string, string[]>() : null;
169169
string[] drivingSqlAliases = Aliases;
170170

171171
//NH-3710: if we are issuing an aggregation function, Aliases will be null

‎src/NHibernate/Loader/Hql/QueryLoader.cs

+1-1
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ protected override SqlString ApplyLocks(SqlString sql, IDictionary<string, LockM
7070
// we are given a map of user-alias -> lock mode
7171
// create a new map of sql-alias -> lock mode
7272
var aliasedLockModes = new Dictionary<string, LockMode>();
73-
Dictionary<string, string[]> keyColumnNames = dialect.ForUpdateOfColumns ? new Dictionary<string, string[]>() : null;
73+
Dictionary<string, string[]> keyColumnNames = dialect.UsesColumnsWithForUpdateOf ? new Dictionary<string, string[]>() : null;
7474

7575
foreach (var entry in lockModes)
7676
{

‎src/NHibernate/SqlCommand/ForUpdateFragment.cs

+2-2
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ public ForUpdateFragment(Dialect.Dialect dialect, IDictionary<string, LockMode>
3030
if (LockMode.Read.LessThan(lockMode))
3131
{
3232
string tableAlias = me.Key;
33-
if (dialect.ForUpdateOfColumns)
33+
if (dialect.UsesColumnsWithForUpdateOf)
3434
{
3535
string[] keyColumns = keyColumnNames[tableAlias];
3636
if (keyColumns == null)
@@ -89,4 +89,4 @@ public string ToSqlStringFragment()
8989
: dialect.GetForUpdateString(aliases.ToString());
9090
}
9191
}
92-
}
92+
}

0 commit comments

Comments
 (0)