diff --git a/doc/reference/modules/component_mapping.xml b/doc/reference/modules/component_mapping.xml index 33b4a21342a..b93343da65c 100644 --- a/doc/reference/modules/component_mapping.xml +++ b/doc/reference/modules/component_mapping.xml @@ -138,16 +138,6 @@ model and persistence semantics are still slightly different. - - A composite element mapping does not support null-able properties if you are using - a <set>. There is no separate primary key column in the - composite element table. NHibernate uses each column's value to identify a record - when deleting objects, which is not possible with null values. You have to either - use only not-null properties in a composite-element or choose a - <list>, <map>, - <bag> or <idbag>. - - A special case of a composite element is a composite element with a nested <many-to-one> element. This mapping allows you to map extra diff --git a/src/NHibernate.Test/Async/NHSpecificTest/GH1170/Fixture.cs b/src/NHibernate.Test/Async/NHSpecificTest/GH1170/Fixture.cs new file mode 100644 index 00000000000..765fe756ae4 --- /dev/null +++ b/src/NHibernate.Test/Async/NHSpecificTest/GH1170/Fixture.cs @@ -0,0 +1,102 @@ +//------------------------------------------------------------------------------ +// +// This code was generated by AsyncGenerator. +// +// Changes to this file may cause incorrect behavior and will be lost if +// the code is regenerated. +// +//------------------------------------------------------------------------------ + + +using System.Linq; +using NUnit.Framework; +using NHibernate.Linq; + +namespace NHibernate.Test.NHSpecificTest.GH1170 +{ + using System.Threading.Tasks; + [TestFixture] + public class FixtureAsync : BugTestCase + { + // Only the set case is tested, because other cases were not affected: + // - bags delete everything first. + // - indexed collections use their index, which is currently not mappable as a composite index with nullable + // column. All index columns are forced to not-nullable by mapping implementation. When using a formula in + // index, they use the element, but its columns are also forced to not-nullable. + + [Test] + public async Task DeleteComponentWithNullAsync() + { + using (var session = OpenSession()) + using (var tx = session.BeginTransaction()) + { + var parent = await (session.Query().SingleAsync()); + Assert.That( + parent.ChildComponents, + Has.Count.EqualTo(2).And.One.Property(nameof(ChildComponent.SomeString)).Null); + parent.ChildComponents.Remove(parent.ChildComponents.Single(c => c.SomeString == null)); + await (tx.CommitAsync()); + } + + using (var session = OpenSession()) + using (var tx = session.BeginTransaction()) + { + var parent = await (session.Query().SingleAsync()); + Assert.That( + parent.ChildComponents, + Has.Count.EqualTo(1).And.None.Property(nameof(ChildComponent.SomeString)).Null); + await (tx.CommitAsync()); + } + } + + [Test] + public async Task UpdateComponentWithNullAsync() + { + // Updates on set are indeed handled as delete/insert, so this test is not really needed. + using (var session = OpenSession()) + using (var tx = session.BeginTransaction()) + { + var parent = await (session.Query().SingleAsync()); + Assert.That( + parent.ChildComponents, + Has.Count.EqualTo(2).And.One.Property(nameof(ChildComponent.SomeString)).Null); + parent.ChildComponents.Single(c => c.SomeString == null).SomeString = "no more null"; + await (tx.CommitAsync()); + } + + using (var session = OpenSession()) + using (var tx = session.BeginTransaction()) + { + var parent = await (session.Query().SingleAsync()); + Assert.That( + parent.ChildComponents, + Has.Count.EqualTo(2).And.None.Property(nameof(ChildComponent.SomeString)).Null); + await (tx.CommitAsync()); + } + } + + protected override void OnSetUp() + { + using (var session = OpenSession()) + using (var tx = session.BeginTransaction()) + { + var parent = new Parent(); + parent.ChildComponents.Add(new ChildComponent { SomeBool = true, SomeString = "something" }); + parent.ChildComponents.Add(new ChildComponent { SomeBool = false, SomeString = null }); + session.Save(parent); + + tx.Commit(); + } + } + + protected override void OnTearDown() + { + using (var session = OpenSession()) + using (var tx = session.BeginTransaction()) + { + session.Delete("from Parent"); + tx.Commit(); + } + } + } +} diff --git a/src/NHibernate.Test/NHSpecificTest/GH1170/Fixture.cs b/src/NHibernate.Test/NHSpecificTest/GH1170/Fixture.cs new file mode 100644 index 00000000000..1031e6e0b19 --- /dev/null +++ b/src/NHibernate.Test/NHSpecificTest/GH1170/Fixture.cs @@ -0,0 +1,90 @@ +using System.Linq; +using NUnit.Framework; + +namespace NHibernate.Test.NHSpecificTest.GH1170 +{ + [TestFixture] + public class Fixture : BugTestCase + { + // Only the set case is tested, because other cases were not affected: + // - bags delete everything first. + // - indexed collections use their index, which is currently not mappable as a composite index with nullable + // column. All index columns are forced to not-nullable by mapping implementation. When using a formula in + // index, they use the element, but its columns are also forced to not-nullable. + + [Test] + public void DeleteComponentWithNull() + { + using (var session = OpenSession()) + using (var tx = session.BeginTransaction()) + { + var parent = session.Query().Single(); + Assert.That( + parent.ChildComponents, + Has.Count.EqualTo(2).And.One.Property(nameof(ChildComponent.SomeString)).Null); + parent.ChildComponents.Remove(parent.ChildComponents.Single(c => c.SomeString == null)); + tx.Commit(); + } + + using (var session = OpenSession()) + using (var tx = session.BeginTransaction()) + { + var parent = session.Query().Single(); + Assert.That( + parent.ChildComponents, + Has.Count.EqualTo(1).And.None.Property(nameof(ChildComponent.SomeString)).Null); + tx.Commit(); + } + } + + [Test] + public void UpdateComponentWithNull() + { + // Updates on set are indeed handled as delete/insert, so this test is not really needed. + using (var session = OpenSession()) + using (var tx = session.BeginTransaction()) + { + var parent = session.Query().Single(); + Assert.That( + parent.ChildComponents, + Has.Count.EqualTo(2).And.One.Property(nameof(ChildComponent.SomeString)).Null); + parent.ChildComponents.Single(c => c.SomeString == null).SomeString = "no more null"; + tx.Commit(); + } + + using (var session = OpenSession()) + using (var tx = session.BeginTransaction()) + { + var parent = session.Query().Single(); + Assert.That( + parent.ChildComponents, + Has.Count.EqualTo(2).And.None.Property(nameof(ChildComponent.SomeString)).Null); + tx.Commit(); + } + } + + protected override void OnSetUp() + { + using (var session = OpenSession()) + using (var tx = session.BeginTransaction()) + { + var parent = new Parent(); + parent.ChildComponents.Add(new ChildComponent { SomeBool = true, SomeString = "something" }); + parent.ChildComponents.Add(new ChildComponent { SomeBool = false, SomeString = null }); + session.Save(parent); + + tx.Commit(); + } + } + + protected override void OnTearDown() + { + using (var session = OpenSession()) + using (var tx = session.BeginTransaction()) + { + session.Delete("from Parent"); + tx.Commit(); + } + } + } +} diff --git a/src/NHibernate.Test/NHSpecificTest/GH1170/Mappings.hbm.xml b/src/NHibernate.Test/NHSpecificTest/GH1170/Mappings.hbm.xml new file mode 100644 index 00000000000..19385fb651f --- /dev/null +++ b/src/NHibernate.Test/NHSpecificTest/GH1170/Mappings.hbm.xml @@ -0,0 +1,17 @@ + + + + + + + + + + + + + + + diff --git a/src/NHibernate.Test/NHSpecificTest/GH1170/Model.cs b/src/NHibernate.Test/NHSpecificTest/GH1170/Model.cs new file mode 100644 index 00000000000..804e8d9317c --- /dev/null +++ b/src/NHibernate.Test/NHSpecificTest/GH1170/Model.cs @@ -0,0 +1,37 @@ +using System; +using System.Collections.Generic; + +namespace NHibernate.Test.NHSpecificTest.GH1170 +{ + public class Parent + { + public virtual ICollection ChildComponents { get; set; } = new List(); + public virtual Guid Id { get; set; } + } + + public class ChildComponent + { + public virtual bool SomeBool { get; set; } + public virtual string SomeString { get; set; } + + protected bool Equals(ChildComponent other) + { + return SomeBool == other.SomeBool && string.Equals(SomeString, other.SomeString); + } + + public override bool Equals(object obj) + { + if (ReferenceEquals(null, obj)) return false; + if (ReferenceEquals(this, obj)) return true; + return obj is ChildComponent other && Equals(other); + } + + public override int GetHashCode() + { + unchecked + { + return (SomeBool.GetHashCode() * 397) ^ (SomeString != null ? SomeString.GetHashCode() : 0); + } + } + } +} diff --git a/src/NHibernate/Async/Persister/Collection/AbstractCollectionPersister.cs b/src/NHibernate/Async/Persister/Collection/AbstractCollectionPersister.cs index f4dd966ea68..f51d70416d0 100644 --- a/src/NHibernate/Async/Persister/Collection/AbstractCollectionPersister.cs +++ b/src/NHibernate/Async/Persister/Collection/AbstractCollectionPersister.cs @@ -10,6 +10,7 @@ using System; using System.Collections; +using System.Collections.Concurrent; using System.Collections.Generic; using System.Data; using System.Data.Common; @@ -32,7 +33,7 @@ using NHibernate.SqlTypes; using NHibernate.Type; using NHibernate.Util; -using Array=NHibernate.Mapping.Array; +using Array = NHibernate.Mapping.Array; namespace NHibernate.Persister.Collection { @@ -138,7 +139,7 @@ protected async Task WriteIndexAsync(DbCommand st, object idx, int i, ISess return i + ArrayHelper.CountTrue(indexColumnIsSettable); } - protected Task WriteElementToWhereAsync(DbCommand st, object elt, int i, ISessionImplementor session, CancellationToken cancellationToken) + protected Task WriteElementToWhereAsync(DbCommand st, object elt, bool[] columnNullness, int i, ISessionImplementor session, CancellationToken cancellationToken) { if (elementIsPureFormula) { @@ -152,11 +153,26 @@ protected Task WriteElementToWhereAsync(DbCommand st, object elt, int i, IS async Task InternalWriteElementToWhereAsync() { - await (ElementType.NullSafeSetAsync(st, elt, i, elementColumnIsInPrimaryKey, session, cancellationToken)).ConfigureAwait(false); - return i + elementColumnAliases.Length; + var settable = Combine(elementColumnIsInPrimaryKey, columnNullness); + + await (ElementType.NullSafeSetAsync(st, elt, i, settable, session, cancellationToken)).ConfigureAwait(false); + return i + settable.Count(s => s); + } + } + + // Since v5.2 + [Obsolete("Use overload with columnNullness instead")] + protected Task WriteElementToWhereAsync(DbCommand st, object elt, int i, ISessionImplementor session, CancellationToken cancellationToken) + { + if (cancellationToken.IsCancellationRequested) + { + return Task.FromCanceled(cancellationToken); } + return WriteElementToWhereAsync(st, elt, null, i, session, cancellationToken); } + // No column nullness handling here: although a composite index could have null columns, the mapping + // current implementation forbirds this by forcing not-null to true on all columns. protected Task WriteIndexToWhereAsync(DbCommand st, object index, int i, ISessionImplementor session, CancellationToken cancellationToken) { if (indexContainsFormula) @@ -333,19 +349,18 @@ public async Task DeleteRowsAsync(IPersistentCollection collection, object id, I DbCommand st; var expectation = Expectations.AppropriateExpectation(deleteCheckStyle); //var callable = DeleteCallable; + var commandInfo = GetDeleteCommand(deleteByIndex, entry, out var columnNullness); var useBatch = expectation.CanBeBatched; if (useBatch) { - st = - await (session.Batcher.PrepareBatchCommandAsync(SqlDeleteRowString.CommandType, SqlDeleteRowString.Text, - SqlDeleteRowString.ParameterTypes, cancellationToken)).ConfigureAwait(false); + st = await (session.Batcher.PrepareBatchCommandAsync( + commandInfo.CommandType, commandInfo.Text, commandInfo.ParameterTypes, cancellationToken)).ConfigureAwait(false); } else { - st = - await (session.Batcher.PrepareCommandAsync(SqlDeleteRowString.CommandType, SqlDeleteRowString.Text, - SqlDeleteRowString.ParameterTypes, cancellationToken)).ConfigureAwait(false); + st = await (session.Batcher.PrepareCommandAsync( + commandInfo.CommandType, commandInfo.Text, commandInfo.ParameterTypes, cancellationToken)).ConfigureAwait(false); } try { @@ -364,7 +379,7 @@ public async Task DeleteRowsAsync(IPersistentCollection collection, object id, I } else { - await (WriteElementToWhereAsync(st, entry, loc, session, cancellationToken)).ConfigureAwait(false); + await (WriteElementToWhereAsync(st, entry, columnNullness, loc, session, cancellationToken)).ConfigureAwait(false); } } if (useBatch) diff --git a/src/NHibernate/Async/Persister/Collection/BasicCollectionPersister.cs b/src/NHibernate/Async/Persister/Collection/BasicCollectionPersister.cs index 413a69d7f26..01aee90c2c0 100644 --- a/src/NHibernate/Async/Persister/Collection/BasicCollectionPersister.cs +++ b/src/NHibernate/Async/Persister/Collection/BasicCollectionPersister.cs @@ -20,10 +20,10 @@ using NHibernate.Loader.Collection; using NHibernate.Persister.Entity; using NHibernate.SqlCommand; -using NHibernate.SqlTypes; using NHibernate.Type; using NHibernate.Util; using System.Collections.Generic; +using NHibernate.SqlTypes; namespace NHibernate.Persister.Collection { @@ -85,7 +85,10 @@ protected override async Task DoUpdateRowsAsync(object id, IPersistentColle } else { - await (WriteElementToWhereAsync(st, collection.GetSnapshotElement(entry, i), loc, session, cancellationToken)).ConfigureAwait(false); + // No nullness handled on update: updates does not occurs with sets or bags, and + // indexed collections allowing formula (maps) force their element columns to + // not-nullable. + await (WriteElementToWhereAsync(st, collection.GetSnapshotElement(entry, i), null, loc, session, cancellationToken)).ConfigureAwait(false); } } diff --git a/src/NHibernate/Async/Persister/Collection/OneToManyPersister.cs b/src/NHibernate/Async/Persister/Collection/OneToManyPersister.cs index e3f62b9e21a..16ebcbe9834 100644 --- a/src/NHibernate/Async/Persister/Collection/OneToManyPersister.cs +++ b/src/NHibernate/Async/Persister/Collection/OneToManyPersister.cs @@ -55,7 +55,7 @@ protected override async Task DoUpdateRowsAsync(object id, IPersistentColle { if (await (collection.NeedsUpdatingAsync(entry, i, ElementType, cancellationToken)).ConfigureAwait(false)) { - DbCommand st = null; + DbCommand st; // will still be issued when it used to be null if (useBatch) { @@ -71,7 +71,9 @@ protected override async Task DoUpdateRowsAsync(object id, IPersistentColle try { int loc = await (WriteKeyAsync(st, id, offset, session, cancellationToken)).ConfigureAwait(false); - await (WriteElementToWhereAsync(st, collection.GetSnapshotElement(entry, i), loc, session, cancellationToken)).ConfigureAwait(false); + // No columnNullness handling: the element is the entity key and should not contain null + // values. + await (WriteElementToWhereAsync(st, collection.GetSnapshotElement(entry, i), null, loc, session, cancellationToken)).ConfigureAwait(false); if (useBatch) { await (session.Batcher.AddToBatchAsync(deleteExpectation, cancellationToken)).ConfigureAwait(false); @@ -117,7 +119,7 @@ protected override async Task DoUpdateRowsAsync(object id, IPersistentColle { if (await (collection.NeedsUpdatingAsync(entry, i, ElementType, cancellationToken)).ConfigureAwait(false)) { - DbCommand st = null; + DbCommand st; if (useBatch) { st = await (session.Batcher.PrepareBatchCommandAsync(SqlInsertRowString.CommandType, sql.Text, @@ -137,7 +139,9 @@ protected override async Task DoUpdateRowsAsync(object id, IPersistentColle { loc = await (WriteIndexToWhereAsync(st, collection.GetIndex(entry, i, this), loc, session, cancellationToken)).ConfigureAwait(false); } - await (WriteElementToWhereAsync(st, collection.GetElement(entry), loc, session, cancellationToken)).ConfigureAwait(false); + // No columnNullness handling: the element is the entity key and should not contain null + // values. + await (WriteElementToWhereAsync(st, collection.GetElement(entry), null, loc, session, cancellationToken)).ConfigureAwait(false); if (useBatch) { await (session.Batcher.AddToBatchAsync(insertExpectation, cancellationToken)).ConfigureAwait(false); diff --git a/src/NHibernate/Async/Type/IType.cs b/src/NHibernate/Async/Type/IType.cs index 222d5fc673f..d38f89d89f7 100644 --- a/src/NHibernate/Async/Type/IType.cs +++ b/src/NHibernate/Async/Type/IType.cs @@ -191,4 +191,4 @@ public partial interface IType : ICacheAssembler /// The value to be merged. Task ReplaceAsync(object original, object target, ISessionImplementor session, object owner, IDictionary copyCache, ForeignKeyDirection foreignKeyDirection, CancellationToken cancellationToken); } -} \ No newline at end of file +} diff --git a/src/NHibernate/Persister/Collection/AbstractCollectionPersister.cs b/src/NHibernate/Persister/Collection/AbstractCollectionPersister.cs index 1c38dcb25b0..e92096d85d0 100644 --- a/src/NHibernate/Persister/Collection/AbstractCollectionPersister.cs +++ b/src/NHibernate/Persister/Collection/AbstractCollectionPersister.cs @@ -1,5 +1,6 @@ using System; using System.Collections; +using System.Collections.Concurrent; using System.Collections.Generic; using System.Data; using System.Data.Common; @@ -22,7 +23,7 @@ using NHibernate.SqlTypes; using NHibernate.Type; using NHibernate.Util; -using Array=NHibernate.Mapping.Array; +using Array = NHibernate.Mapping.Array; namespace NHibernate.Persister.Collection { @@ -41,6 +42,8 @@ public abstract partial class AbstractCollectionPersister : ICollectionMetadata, private readonly SqlCommandInfo sqlInsertRowString; private readonly SqlCommandInfo sqlUpdateRowString; private readonly SqlCommandInfo sqlDeleteRowString; + private readonly ConcurrentDictionary sqlDeleteRowStringByNullness = + new ConcurrentDictionary(ArrayHelper.ArrayComparer.Default); private readonly SqlString sqlSelectRowByIndexString; private readonly SqlString sqlDetectRowByIndexString; private readonly SqlString sqlDetectRowByElementString; @@ -468,7 +471,10 @@ public AbstractCollectionPersister(Mapping.Collection collection, ICacheConcurre ?? ExecuteUpdateResultCheckStyle.DetermineDefault(collection.CustomSQLUpdate, updateCallable); } + // 6.0 TODO: call GenerateDeleteRowString(null); instead. +#pragma warning disable 618 sqlDeleteRowString = GenerateDeleteRowString(); +#pragma warning restore 618 if (collection.CustomSQLDelete == null) { deleteCallable = false; @@ -777,17 +783,59 @@ protected object IncrementIndexByBase(object index) return index; } - protected int WriteElementToWhere(DbCommand st, object elt, int i, ISessionImplementor session) + protected int WriteElementToWhere(DbCommand st, object elt, bool[] columnNullness, int i, ISessionImplementor session) { if (elementIsPureFormula) { throw new AssertionFailure("cannot use a formula-based element in the where condition"); } - ElementType.NullSafeSet(st, elt, i, elementColumnIsInPrimaryKey, session); - return i + elementColumnAliases.Length; + var settable = Combine(elementColumnIsInPrimaryKey, columnNullness); + + ElementType.NullSafeSet(st, elt, i, settable, session); + return i + settable.Count(s => s); + } + + // Since v5.2 + [Obsolete("Use overload with columnNullness instead")] + protected int WriteElementToWhere(DbCommand st, object elt, int i, ISessionImplementor session) + { + return WriteElementToWhere(st, elt, null, i, session); + } + + /// + /// Combine arrays indicating settability and nullness of columns into one, considering null columns as not + /// settable. + /// + /// Settable columns. will consider them as all settable. + /// Nullness of columns. will consider them as all + /// non-null. indicates a non-null column, indicates a null + /// column. + /// The resulting settability of columns, or if both argument are + /// . + /// thrown if and + /// have inconsistent lengthes. + protected static bool[] Combine(bool[] settable, bool[] columnNullness) + { + if (columnNullness == null) + return settable; + if (settable == null) + return columnNullness; + + if (columnNullness.Length != settable.Length) + throw new InvalidOperationException("Inconsistent nullness and settable columns lengthes"); + + var result = new bool[settable.Length]; + for (var idx = 0; idx < settable.Length; idx++) + { + result[idx] = columnNullness[idx] && settable[idx]; + } + + return result; } + // No column nullness handling here: although a composite index could have null columns, the mapping + // current implementation forbirds this by forcing not-null to true on all columns. protected int WriteIndexToWhere(DbCommand st, object index, int i, ISessionImplementor session) { if (indexContainsFormula) @@ -1167,19 +1215,18 @@ public void DeleteRows(IPersistentCollection collection, object id, ISessionImpl DbCommand st; var expectation = Expectations.AppropriateExpectation(deleteCheckStyle); //var callable = DeleteCallable; + var commandInfo = GetDeleteCommand(deleteByIndex, entry, out var columnNullness); var useBatch = expectation.CanBeBatched; if (useBatch) { - st = - session.Batcher.PrepareBatchCommand(SqlDeleteRowString.CommandType, SqlDeleteRowString.Text, - SqlDeleteRowString.ParameterTypes); + st = session.Batcher.PrepareBatchCommand( + commandInfo.CommandType, commandInfo.Text, commandInfo.ParameterTypes); } else { - st = - session.Batcher.PrepareCommand(SqlDeleteRowString.CommandType, SqlDeleteRowString.Text, - SqlDeleteRowString.ParameterTypes); + st = session.Batcher.PrepareCommand( + commandInfo.CommandType, commandInfo.Text, commandInfo.ParameterTypes); } try { @@ -1198,7 +1245,7 @@ public void DeleteRows(IPersistentCollection collection, object id, ISessionImpl } else { - WriteElementToWhere(st, entry, loc, session); + WriteElementToWhere(st, entry, columnNullness, loc, session); } } if (useBatch) @@ -1244,6 +1291,26 @@ public void DeleteRows(IPersistentCollection collection, object id, ISessionImpl } } + private SqlCommandInfo GetDeleteCommand(bool deleteByIndex, object entry, out bool[] columnNullness) + { + var commandInfo = SqlDeleteRowString; + columnNullness = null; + // No column nullness handling if deleteByIndex: although a composite index could have null columns, the + // mapping current implementation forbirds this by forcing not-null to true on all columns. + if (!hasIdentifier && !deleteByIndex) + { + columnNullness = ElementType.ToColumnNullness(entry, Factory); + if (columnNullness.Any(cn => !cn)) + commandInfo = sqlDeleteRowStringByNullness.GetOrAdd( + columnNullness, + GenerateDeleteRowString); + else + columnNullness = null; + } + + return commandInfo; + } + public void InsertRows(IPersistentCollection collection, object id, ISessionImplementor session) { if (!isInverse && RowInsertEnabled) @@ -1368,11 +1435,35 @@ public string[] ToColumns(string propertyName) } protected abstract SqlCommandInfo GenerateDeleteString(); - protected abstract SqlCommandInfo GenerateDeleteRowString(); + // No column nullness handling here: updates currently only occur on cases not allowing null. protected abstract SqlCommandInfo GenerateUpdateRowString(); protected abstract SqlCommandInfo GenerateInsertRowString(); protected abstract SqlCommandInfo GenerateIdentityInsertRowString(); + /// + /// Generate the SQL delete that deletes a particular row. + /// + /// A SQL delete. + // Since v5.2 + [Obsolete("Use or override overload with columnNullness instead")] + protected virtual SqlCommandInfo GenerateDeleteRowString() + { + return GenerateDeleteRowString(null); + } + + /// + /// Generate the SQL delete that deletes a particular row. + /// + /// If non-null, an array of boolean indicating which mapped columns of the index + /// or element would be null. indicates a non-null column, + /// indicates a null column. + /// A SQL delete. + // 6.0 TODO: make abstract + protected virtual SqlCommandInfo GenerateDeleteRowString(bool[] columnNullness) + { + throw new NotSupportedException($"{GetType().FullName} does not support queries handling nullness."); + } + public void UpdateRows(IPersistentCollection collection, object id, ISessionImplementor session) { if (!isInverse && collection.RowUpdatePossible) diff --git a/src/NHibernate/Persister/Collection/BasicCollectionPersister.cs b/src/NHibernate/Persister/Collection/BasicCollectionPersister.cs index db8e0775f66..6343ae4b6ce 100644 --- a/src/NHibernate/Persister/Collection/BasicCollectionPersister.cs +++ b/src/NHibernate/Persister/Collection/BasicCollectionPersister.cs @@ -10,10 +10,10 @@ using NHibernate.Loader.Collection; using NHibernate.Persister.Entity; using NHibernate.SqlCommand; -using NHibernate.SqlTypes; using NHibernate.Type; using NHibernate.Util; using System.Collections.Generic; +using NHibernate.SqlTypes; namespace NHibernate.Persister.Collection { @@ -113,29 +113,40 @@ protected override SqlCommandInfo GenerateUpdateRowString() return update.ToSqlCommandInfo(); } - /// - /// Generate the SQL DELETE that deletes a particular row - /// - /// - protected override SqlCommandInfo GenerateDeleteRowString() + /// + protected override SqlCommandInfo GenerateDeleteRowString(bool[] columnNullness) { - SqlDeleteBuilder delete = new SqlDeleteBuilder(Factory.Dialect, Factory); + var delete = new SqlDeleteBuilder(Factory.Dialect, Factory); delete.SetTableName(qualifiedTableName); + if (hasIdentifier) { - delete.AddWhereFragment(new string[] { IdentifierColumnName }, IdentifierType, " = "); - } - else if (HasIndex && !indexContainsFormula) - { - delete - .AddWhereFragment(KeyColumnNames, KeyType, " = ") - .AddWhereFragment(IndexColumnNames, IndexType, " = "); + delete.AddWhereFragment(new[] { IdentifierColumnName }, IdentifierType, " = "); } else { - string[] cnames = ArrayHelper.Join(KeyColumnNames, ElementColumnNames, elementColumnIsInPrimaryKey); - SqlType[] ctypes = ArrayHelper.Join(KeyType.SqlTypes(Factory), ElementType.SqlTypes(Factory), elementColumnIsInPrimaryKey); + var useIndex = HasIndex && !indexContainsFormula; + var additionalFilterType = useIndex ? IndexType : ElementType; + var additionalFilterColumns = useIndex ? IndexColumnNames : ElementColumnNames; + var includes = useIndex ? null : Combine(elementColumnIsInPrimaryKey, columnNullness); + + var cnames = includes == null + ? ArrayHelper.Join(KeyColumnNames, additionalFilterColumns) + : ArrayHelper.Join(KeyColumnNames, additionalFilterColumns, includes); + var ctypes = includes == null + ? ArrayHelper.Join(KeyType.SqlTypes(Factory), additionalFilterType.SqlTypes(Factory)) + : ArrayHelper.Join(KeyType.SqlTypes(Factory), additionalFilterType.SqlTypes(Factory), includes); delete.AddWhereFragment(cnames, ctypes, " = "); + + if (columnNullness != null) + { + for (var i = 0; i < columnNullness.Length; i++) + { + if (columnNullness[i]) + continue; + delete.AddWhereFragment($"{additionalFilterColumns[i]} is null"); + } + } } if (Factory.Settings.IsCommentsEnabled) @@ -206,7 +217,10 @@ protected override int DoUpdateRows(object id, IPersistentCollection collection, } else { - WriteElementToWhere(st, collection.GetSnapshotElement(entry, i), loc, session); + // No nullness handled on update: updates does not occurs with sets or bags, and + // indexed collections allowing formula (maps) force their element columns to + // not-nullable. + WriteElementToWhere(st, collection.GetSnapshotElement(entry, i), null, loc, session); } } diff --git a/src/NHibernate/Persister/Collection/OneToManyPersister.cs b/src/NHibernate/Persister/Collection/OneToManyPersister.cs index d3daee50098..9677fef88a5 100644 --- a/src/NHibernate/Persister/Collection/OneToManyPersister.cs +++ b/src/NHibernate/Persister/Collection/OneToManyPersister.cs @@ -114,18 +114,19 @@ protected override SqlCommandInfo GenerateInsertRowString() /// /// Not needed for one-to-many association /// - /// protected override SqlCommandInfo GenerateUpdateRowString() { return null; } + /// /// /// Generate the SQL UPDATE that updates a particular row's foreign - /// key to null + /// key to null. /// - /// - protected override SqlCommandInfo GenerateDeleteRowString() + /// Unused, the element is the entity key and should not contain null + /// values. + protected override SqlCommandInfo GenerateDeleteRowString(bool[] columnNullness) { var update = new SqlUpdateBuilder(Factory.Dialect, Factory); update.SetTableName(qualifiedTableName) @@ -184,7 +185,7 @@ protected override int DoUpdateRows(object id, IPersistentCollection collection, { if (collection.NeedsUpdating(entry, i, ElementType)) { - DbCommand st = null; + DbCommand st; // will still be issued when it used to be null if (useBatch) { @@ -200,7 +201,9 @@ protected override int DoUpdateRows(object id, IPersistentCollection collection, try { int loc = WriteKey(st, id, offset, session); - WriteElementToWhere(st, collection.GetSnapshotElement(entry, i), loc, session); + // No columnNullness handling: the element is the entity key and should not contain null + // values. + WriteElementToWhere(st, collection.GetSnapshotElement(entry, i), null, loc, session); if (useBatch) { session.Batcher.AddToBatch(deleteExpectation); @@ -245,7 +248,7 @@ protected override int DoUpdateRows(object id, IPersistentCollection collection, { if (collection.NeedsUpdating(entry, i, ElementType)) { - DbCommand st = null; + DbCommand st; if (useBatch) { st = session.Batcher.PrepareBatchCommand(SqlInsertRowString.CommandType, sql.Text, @@ -265,7 +268,9 @@ protected override int DoUpdateRows(object id, IPersistentCollection collection, { loc = WriteIndexToWhere(st, collection.GetIndex(entry, i, this), loc, session); } - WriteElementToWhere(st, collection.GetElement(entry), loc, session); + // No columnNullness handling: the element is the entity key and should not contain null + // values. + WriteElementToWhere(st, collection.GetElement(entry), null, loc, session); if (useBatch) { session.Batcher.AddToBatch(insertExpectation); diff --git a/src/NHibernate/Type/IType.cs b/src/NHibernate/Type/IType.cs index 84ebe3808eb..c6c05bf8fdd 100644 --- a/src/NHibernate/Type/IType.cs +++ b/src/NHibernate/Type/IType.cs @@ -326,10 +326,11 @@ public partial interface IType : ICacheAssembler /// /// Given an instance of the type, return an array of boolean, indicating - /// which mapped columns would be null. + /// which mapped columns would be null. indicates + /// a non-null column, indicates a null column. /// - /// an instance of the type - /// + /// An instance of the type. + /// The mapping. bool[] ToColumnNullness(object value, IMapping mapping); } -} \ No newline at end of file +} diff --git a/src/NHibernate/Util/ArrayHelper.cs b/src/NHibernate/Util/ArrayHelper.cs index 531dd33a599..06b9cbd9df5 100644 --- a/src/NHibernate/Util/ArrayHelper.cs +++ b/src/NHibernate/Util/ArrayHelper.cs @@ -179,45 +179,12 @@ public static int CountTrue(bool[] array) public static bool ArrayEquals(T[] a, T[] b) { - if (a == b) - return true; - - if (a == null || b == null) - return false; - - if (a.Length != b.Length) - return false; - - for (int i = 0; i < a.Length; i++) - { - if (!Equals(a[i], b[i])) - return false; - } - - return true; + return ArrayComparer.Default.Equals(a, b); } public static bool ArrayEquals(byte[] a, byte[] b) { - if (a == b) - return true; - - if (a == null || b == null) - return false; - - if (a.Length != b.Length) - return false; - - int i = 0; - int len = a.Length; - while (i < len) - { - if (a[i] != b[i]) - return false; - - i++; - } - return true; + return ArrayComparer.Default.Equals(a, b); } /// @@ -230,12 +197,54 @@ public static bool ArrayEquals(byte[] a, byte[] b) /// public static int ArrayGetHashCode(T[] array) { - int hc = array.Length; + return ArrayComparer.Default.GetHashCode(array); + } + + internal class ArrayComparer : IEqualityComparer + { + private readonly IEqualityComparer _elementComparer; + + internal static ArrayComparer Default { get; } = new ArrayComparer(); - foreach (var e in array) - hc = unchecked(hc*31 + e.GetHashCode()); + internal ArrayComparer() : this(EqualityComparer.Default) { } - return hc; + internal ArrayComparer(IEqualityComparer elementComparer) + { + _elementComparer = elementComparer ?? throw new ArgumentNullException(nameof(elementComparer)); + } + + public bool Equals(T[] a, T[] b) + { + if (a == b) + return true; + + if (a == null || b == null) + return false; + + if (a.Length != b.Length) + return false; + + for (var i = 0; i < a.Length; i++) + { + if (!_elementComparer.Equals(a[i], b[i])) + return false; + } + + return true; + } + + public int GetHashCode(T[] array) + { + if (array == null) + return 0; + + var hc = array.Length; + + foreach (var e in array) + hc = unchecked(hc * 31 + _elementComparer.GetHashCode(e)); + + return hc; + } } } }