diff --git a/src/NHibernate.Test/Async/NHSpecificTest/GH1565/LockEntityWithOuterJoinTest.cs b/src/NHibernate.Test/Async/NHSpecificTest/GH1565/LockEntityWithOuterJoinTest.cs new file mode 100644 index 00000000000..56e189e798f --- /dev/null +++ b/src/NHibernate.Test/Async/NHSpecificTest/GH1565/LockEntityWithOuterJoinTest.cs @@ -0,0 +1,59 @@ +//------------------------------------------------------------------------------ +// +// This code was generated by AsyncGenerator. +// +// Changes to this file may cause incorrect behavior and will be lost if +// the code is regenerated. +// +//------------------------------------------------------------------------------ + + +using NUnit.Framework; + +namespace NHibernate.Test.NHSpecificTest.GH1565 +{ + using System.Threading.Tasks; + [TestFixture] + public class LockEntityWithOuterJoinTestAsync : BugTestCase + { + [Test] + public async Task LockWithOuterJoin_ShouldBePossibleAsync() + { + using (var session = OpenSession()) + { + using (var transaction = session.BeginTransaction()) + { + var entity = await (session.GetAsync(id, LockMode.Upgrade)); + Assert.That(entity.Id, Is.EqualTo(id)); + await (transaction.CommitAsync()); + } + } + } + + private int id; + protected override void OnSetUp() + { + base.OnSetUp(); + using (var session = OpenSession()) + { + using (var transaction = session.BeginTransaction()) + { + session.FlushMode = FlushMode.Auto; + var entity = new MainEntity(); + session.Save(entity); + transaction.Commit(); + id = entity.Id; + } + } + } + + protected override void OnTearDown() + { + base.OnTearDown(); + using (var session = OpenSession()) + { + session.CreateSQLQuery("delete from MainEntity").ExecuteUpdate(); + } + } + } +} diff --git a/src/NHibernate.Test/NHSpecificTest/GH1565/LockEntityWithOuterJoinTest.cs b/src/NHibernate.Test/NHSpecificTest/GH1565/LockEntityWithOuterJoinTest.cs new file mode 100644 index 00000000000..2815ff61c20 --- /dev/null +++ b/src/NHibernate.Test/NHSpecificTest/GH1565/LockEntityWithOuterJoinTest.cs @@ -0,0 +1,55 @@ +using NUnit.Framework; + +namespace NHibernate.Test.NHSpecificTest.GH1565 +{ + [TestFixture] + public class LockEntityWithOuterJoinTest : BugTestCase + { + [Test] + public void LockWithOuterJoin_ShouldBePossible() + { + using (var session = OpenSession()) + { + using (var transaction = session.BeginTransaction()) + { + var entity = session.Get(id, LockMode.Upgrade); + Assert.That(entity.Id, Is.EqualTo(id)); + transaction.Commit(); + } + } + } + + private int id; + protected override void OnSetUp() + { + base.OnSetUp(); + using (var session = OpenSession()) + { + using (var transaction = session.BeginTransaction()) + { + session.FlushMode = FlushMode.Auto; + var entity = new MainEntity(); + session.Save(entity); + transaction.Commit(); + id = entity.Id; + } + } + } + + protected override void OnTearDown() + { + base.OnTearDown(); + using (var session = OpenSession()) + { + session.CreateSQLQuery("delete from MainEntity").ExecuteUpdate(); + } + } + } + + public class MainEntity + { + public virtual int Id { get; set; } = 0; + + public virtual string Data { get; set; } + } +} diff --git a/src/NHibernate.Test/NHSpecificTest/GH1565/Mappings.hbm.xml b/src/NHibernate.Test/NHSpecificTest/GH1565/Mappings.hbm.xml new file mode 100644 index 00000000000..45efe8beb14 --- /dev/null +++ b/src/NHibernate.Test/NHSpecificTest/GH1565/Mappings.hbm.xml @@ -0,0 +1,15 @@ + + + + + + + + + + + + + + diff --git a/src/NHibernate/Async/Dialect/InformixDialect.cs b/src/NHibernate/Async/Dialect/InformixDialect.cs index 88d46a2335a..ebcae053076 100644 --- a/src/NHibernate/Async/Dialect/InformixDialect.cs +++ b/src/NHibernate/Async/Dialect/InformixDialect.cs @@ -8,14 +8,15 @@ //------------------------------------------------------------------------------ +using System; using System.Data; using System.Data.Common; using System.Text; -using NHibernate.Cfg; using NHibernate.Dialect.Function; using NHibernate.Exceptions; using NHibernate.SqlCommand; using NHibernate.Util; +using Environment = NHibernate.Cfg.Environment; //using NHibernate.Dialect.Schema; diff --git a/src/NHibernate/Async/Loader/Hql/QueryLoader.cs b/src/NHibernate/Async/Loader/Hql/QueryLoader.cs index ed6e39a3c2e..b50cff730c6 100644 --- a/src/NHibernate/Async/Loader/Hql/QueryLoader.cs +++ b/src/NHibernate/Async/Loader/Hql/QueryLoader.cs @@ -121,4 +121,4 @@ internal async Task GetEnumerableAsync(QueryParameters queryParamet return result; } } -} \ No newline at end of file +} diff --git a/src/NHibernate/Dialect/Dialect.cs b/src/NHibernate/Dialect/Dialect.cs index d709ae605ba..16976405d49 100644 --- a/src/NHibernate/Dialect/Dialect.cs +++ b/src/NHibernate/Dialect/Dialect.cs @@ -501,15 +501,15 @@ public virtual ILockingStrategy GetLockingStrategy(ILockable lockable, LockMode /// The appropriate for update fragment. public virtual string GetForUpdateString(LockMode lockMode) { - if (lockMode == LockMode.Upgrade) + if (Equals(lockMode, LockMode.Upgrade)) { return ForUpdateString; } - if (lockMode == LockMode.UpgradeNoWait) + if (Equals(lockMode, LockMode.UpgradeNoWait)) { return ForUpdateNowaitString; } - if (lockMode == LockMode.Force) + if (Equals(lockMode, LockMode.Force)) { return ForUpdateNowaitString; } @@ -526,14 +526,30 @@ public virtual string ForUpdateString get { return " for update"; } } - /// Is FOR UPDATE OF syntax supported? - /// True if the database supports FOR UPDATE OF syntax; false otherwise. + /// Is FOR UPDATE OF syntax supported? + /// if the database supports FOR UPDATE OF syntax; otherwise. + public virtual bool SupportsForUpdateOf + // By default, just check UsesColumnsWithForUpdateOf. ForUpdateOf needs to be overriden only for dialects supporting + // "For Update Of" on table aliases. + => UsesColumnsWithForUpdateOf; + + /// Is FOR UPDATE OF syntax expecting columns? + /// if the database expects a column list with FOR UPDATE OF syntax, + /// if it expects table alias instead or do not support FOR UPDATE OF syntax. + // Since v5.1 + [Obsolete("Use UsesColumnsWithForUpdateOf instead")] public virtual bool ForUpdateOfColumns { // by default we report no support get { return false; } } + public virtual bool UsesColumnsWithForUpdateOf +#pragma warning disable 618 + // For avoiding a breaking change, we need to call the old name by default. + => ForUpdateOfColumns; +#pragma warning restore 618 + /// /// Does this dialect support FOR UPDATE in conjunction with outer joined rows? /// @@ -567,11 +583,11 @@ public virtual string ForUpdateNowaitString } /// - /// Get the FOR UPDATE OF column_list NOWAIT fragment appropriate - /// for this dialect given the aliases of the columns to be write locked. + /// Get the FOR UPDATE OF column_list NOWAIT fragment appropriate + /// for this dialect given the aliases of the columns or tables to be write locked. /// - /// The columns to be write locked. - /// The appropriate FOR UPDATE colunm_list NOWAIT clause string. + /// The columns or tables to be write locked. + /// The appropriate FOR UPDATE colunm_or_table_list NOWAIT clause string. public virtual string GetForUpdateNowaitString(string aliases) { return GetForUpdateString(aliases); diff --git a/src/NHibernate/Dialect/InformixDialect.cs b/src/NHibernate/Dialect/InformixDialect.cs index 5ff406fd2d8..e430af4b29e 100644 --- a/src/NHibernate/Dialect/InformixDialect.cs +++ b/src/NHibernate/Dialect/InformixDialect.cs @@ -1,11 +1,12 @@ +using System; using System.Data; using System.Data.Common; using System.Text; -using NHibernate.Cfg; using NHibernate.Dialect.Function; using NHibernate.Exceptions; using NHibernate.SqlCommand; using NHibernate.Util; +using Environment = NHibernate.Cfg.Environment; //using NHibernate.Dialect.Schema; @@ -175,13 +176,19 @@ public override string AddColumnString // throw new NotSupportedException(); //} - /// Is FOR UPDATE OF syntax supported? - /// True if the database supports FOR UPDATE OF syntax; false otherwise. + /// + // Since v5.1 + [Obsolete("Use UsesColumnsWithForUpdateOf instead")] public override bool ForUpdateOfColumns { get { return true; } } + /* 6.0 TODO: uncomment once ForUpdateOfColumns is removed. + /// + public override bool UsesColumnsWithForUpdateOf => true; + */ + /// /// Does this dialect support FOR UPDATE in conjunction with outer joined rows? /// diff --git a/src/NHibernate/Dialect/Oracle8iDialect.cs b/src/NHibernate/Dialect/Oracle8iDialect.cs index b70f90910fb..d540f635d1b 100644 --- a/src/NHibernate/Dialect/Oracle8iDialect.cs +++ b/src/NHibernate/Dialect/Oracle8iDialect.cs @@ -485,11 +485,18 @@ public override bool UseMaxForLimit get { return true; } } + // Since v5.1 + [Obsolete("Use UsesColumnsWithForUpdateOf instead")] public override bool ForUpdateOfColumns { get { return true; } } + /* 6.0 TODO: uncomment once ForUpdateOfColumns is removed. + /// + public override bool UsesColumnsWithForUpdateOf => true; + */ + public override bool SupportsUnionAll { get { return true; } diff --git a/src/NHibernate/Dialect/PostgreSQLDialect.cs b/src/NHibernate/Dialect/PostgreSQLDialect.cs index 849b2d8a0e7..c1653a36a6f 100644 --- a/src/NHibernate/Dialect/PostgreSQLDialect.cs +++ b/src/NHibernate/Dialect/PostgreSQLDialect.cs @@ -227,6 +227,12 @@ public override SqlString GetLimitString(SqlString queryString, SqlString offset return pagingBuilder.ToSqlString(); } + /// + public override bool SupportsForUpdateOf => true; + + /// + public override bool SupportsOuterJoinForUpdate => false; + public override string GetForUpdateString(string aliases) { return ForUpdateString + " of " + aliases; diff --git a/src/NHibernate/Dialect/SybaseSQLAnywhere10Dialect.cs b/src/NHibernate/Dialect/SybaseSQLAnywhere10Dialect.cs index 8d7ce1dc2ed..bd07ff22c2e 100644 --- a/src/NHibernate/Dialect/SybaseSQLAnywhere10Dialect.cs +++ b/src/NHibernate/Dialect/SybaseSQLAnywhere10Dialect.cs @@ -643,11 +643,29 @@ public override string GetForUpdateString(LockMode lockMode) /// In this dialect, we avoid this issue by supporting only /// FOR UPDATE BY LOCK. /// + // Since v5.1 + [Obsolete("Use UsesColumnsWithForUpdateOf instead")] public override bool ForUpdateOfColumns { get { return false; } } + /* 6.0 TODO: uncomment once ForUpdateOfColumns is removed. + /// + /// SQL Anywhere does support FOR UPDATE OF syntax. However, + /// in SQL Anywhere one cannot specify both FOR UPDATE OF syntax + /// and FOR UPDATE BY LOCK in the same statement. To achieve INTENT + /// locking when using FOR UPDATE OF syntax one must use a table hint + /// in the query's FROM clause, ie. + /// + /// SELECT * FROM FOO WITH( UPDLOCK ) FOR UPDATE OF ( column-list ). + /// + /// In this dialect, we avoid this issue by supporting only + /// FOR UPDATE BY LOCK. + /// + public override bool UsesColumnsWithForUpdateOf => false; + */ + /// /// SQL Anywhere supports FOR UPDATE over cursors containing /// outer joins. diff --git a/src/NHibernate/Loader/AbstractEntityJoinWalker.cs b/src/NHibernate/Loader/AbstractEntityJoinWalker.cs index a789cbaeb90..94b90bdd588 100644 --- a/src/NHibernate/Loader/AbstractEntityJoinWalker.cs +++ b/src/NHibernate/Loader/AbstractEntityJoinWalker.cs @@ -118,7 +118,7 @@ private void InitStatementString(SqlString projection, SqlString condition, SqlS JoinFragment ojf = MergeOuterJoins(associations); SqlSelectBuilder select = new SqlSelectBuilder(Factory) - .SetLockMode(lockMode) + .SetLockMode(lockMode, alias) .SetSelectClause(selectClause) .SetFromClause(Dialect.AppendLockHint(lockMode, persister.FromTableFragment(alias)) +persister.FromJoinFragment(alias, true, true)) .SetWhereClause(condition) diff --git a/src/NHibernate/Loader/Criteria/CriteriaLoader.cs b/src/NHibernate/Loader/Criteria/CriteriaLoader.cs index 655ea38fdab..26a5cefd079 100644 --- a/src/NHibernate/Loader/Criteria/CriteriaLoader.cs +++ b/src/NHibernate/Loader/Criteria/CriteriaLoader.cs @@ -171,7 +171,7 @@ protected override SqlString ApplyLocks(SqlString sqlSelectString, IDictionary aliasedLockModes = new Dictionary(); - Dictionary keyColumnNames = dialect.ForUpdateOfColumns ? new Dictionary() : null; + Dictionary keyColumnNames = dialect.UsesColumnsWithForUpdateOf ? new Dictionary() : null; string[] drivingSqlAliases = Aliases; //NH-3710: if we are issuing an aggregation function, Aliases will be null diff --git a/src/NHibernate/Loader/Hql/QueryLoader.cs b/src/NHibernate/Loader/Hql/QueryLoader.cs index 23c769da343..16d6b198400 100644 --- a/src/NHibernate/Loader/Hql/QueryLoader.cs +++ b/src/NHibernate/Loader/Hql/QueryLoader.cs @@ -71,7 +71,7 @@ protected override SqlString ApplyLocks(SqlString sql, IDictionary lock mode // create a new map of sql-alias -> lock mode var aliasedLockModes = new Dictionary(); - Dictionary keyColumnNames = dialect.ForUpdateOfColumns ? new Dictionary() : null; + Dictionary keyColumnNames = dialect.UsesColumnsWithForUpdateOf ? new Dictionary() : null; foreach (var entry in lockModes) { @@ -456,4 +456,4 @@ protected override IEnumerable GetParameterSpecificatio return _queryTranslator.CollectedParameterSpecifications; } } -} \ No newline at end of file +} diff --git a/src/NHibernate/SqlCommand/ForUpdateFragment.cs b/src/NHibernate/SqlCommand/ForUpdateFragment.cs index 3a30299f115..4e5661abb4b 100644 --- a/src/NHibernate/SqlCommand/ForUpdateFragment.cs +++ b/src/NHibernate/SqlCommand/ForUpdateFragment.cs @@ -30,7 +30,7 @@ public ForUpdateFragment(Dialect.Dialect dialect, IDictionary if (LockMode.Read.LessThan(lockMode)) { string tableAlias = me.Key; - if (dialect.ForUpdateOfColumns) + if (dialect.UsesColumnsWithForUpdateOf) { string[] keyColumns = keyColumnNames[tableAlias]; if (keyColumns == null) @@ -89,4 +89,4 @@ public string ToSqlStringFragment() : dialect.GetForUpdateString(aliases.ToString()); } } -} \ No newline at end of file +} diff --git a/src/NHibernate/SqlCommand/SqlSelectBuilder.cs b/src/NHibernate/SqlCommand/SqlSelectBuilder.cs index 418c0295570..e287fb8b5c4 100644 --- a/src/NHibernate/SqlCommand/SqlSelectBuilder.cs +++ b/src/NHibernate/SqlCommand/SqlSelectBuilder.cs @@ -1,4 +1,5 @@ +using System; using NHibernate.Engine; using NHibernate.Type; using NHibernate.Util; @@ -21,6 +22,7 @@ public class SqlSelectBuilder : SqlBaseBuilder, ISqlStringBuilder private SqlString groupByClause; private SqlString havingClause; private LockMode lockMode; + private string mainTableAlias; private string comment; public SqlSelectBuilder(ISessionFactoryImplementor factory) @@ -62,8 +64,7 @@ public SqlSelectBuilder SetFromClause(string tableName, string alias) /// The SqlSelectBuilder public SqlSelectBuilder SetFromClause(SqlString fromClause) { - // it is safe to do this because a fromClause will have no - // parameters + // it is safe to do this because a fromClause will have no parameters return SetFromClause(fromClause.ToString()); } @@ -181,12 +182,21 @@ public SqlSelectBuilder SetHavingClause(SqlString havingSqlString) return this; } + [Obsolete("For some DBMS's such as PostgreSQL, a lock on query with OUTER JOIN is not possible without specifying the not-null side. " + + "Use the new method SetLockMode(LockMode, mainTableAlias) instead.")] public SqlSelectBuilder SetLockMode(LockMode lockMode) { this.lockMode = lockMode; return this; } + public SqlSelectBuilder SetLockMode(LockMode lockMode, string mainTableAlias) + { + this.lockMode = lockMode; + this.mainTableAlias = mainTableAlias; + return this; + } + #region ISqlStringBuilder Members /// @@ -267,7 +277,7 @@ public SqlString ToSqlString() if (lockMode != null) { - sqlBuilder.Add(Dialect.GetForUpdateString(lockMode)); + sqlBuilder.Add(GetForUpdateString()); } if (log.IsDebugEnabled()) @@ -291,6 +301,43 @@ public SqlString ToSqlString() return sqlBuilder.ToSqlString(); } + private string GetForUpdateString() + { + if (!Dialect.SupportsOuterJoinForUpdate && HasOuterJoin()) + { + var isUpgrade = Equals(lockMode, LockMode.Upgrade); + var isUpgradeNoWait = !isUpgrade && ( + Equals(lockMode, LockMode.UpgradeNoWait) || Equals(lockMode, LockMode.Force)); + if (!isUpgrade && !isUpgradeNoWait) + return string.Empty; + + if (!Dialect.SupportsForUpdateOf) + { + log.Warn( + "Unsupported 'for update' case: 'for update' query with an outer join using a dialect not" + + "supporting it and not supporting 'for update of' clause. Discarding 'for" + + "update' clause."); + return string.Empty; + } + + if (Dialect.UsesColumnsWithForUpdateOf) + { + log.Warn( + "Unimplemented 'for update' case: 'for update' query with an outer join using a dialect not" + + "supporting it and requiring columns for its 'for update of' syntax. Discarding 'for" + + "update' clause."); + return string.Empty; + } + + return isUpgrade ? Dialect.GetForUpdateString(mainTableAlias) : Dialect.GetForUpdateNowaitString(mainTableAlias); + } + + return Dialect.GetForUpdateString(lockMode); + + bool HasOuterJoin() => + outerJoinsAfterFrom?.IsEmptyOrWhitespace() == false || + StringHelper.ContainsCaseInsensitive(fromClause, "outer join"); + } #endregion } }