diff --git a/src/EFCore.PG/Storage/Internal/Mapping/NpgsqlHstoreTypeMapping.cs b/src/EFCore.PG/Storage/Internal/Mapping/NpgsqlHstoreTypeMapping.cs index 85083848e..782a279b4 100644 --- a/src/EFCore.PG/Storage/Internal/Mapping/NpgsqlHstoreTypeMapping.cs +++ b/src/EFCore.PG/Storage/Internal/Mapping/NpgsqlHstoreTypeMapping.cs @@ -1,5 +1,8 @@ -using System.Collections.Generic; +using System; +using System.Collections.Generic; +using System.Collections.Immutable; using System.Text; +using JetBrains.Annotations; using Microsoft.EntityFrameworkCore.ChangeTracking; using Microsoft.EntityFrameworkCore.Storage; using NpgsqlTypes; @@ -7,24 +10,24 @@ namespace Npgsql.EntityFrameworkCore.PostgreSQL.Storage.Internal.Mapping { /// - /// The type mapping for the PostgreSQL hstore type. + /// The type mapping for the PostgreSQL hstore type. Supports both + /// and over strings. /// /// /// See: https://www.postgresql.org/docs/current/static/hstore.html /// public class NpgsqlHstoreTypeMapping : NpgsqlTypeMapping { - static readonly HstoreComparer ComparerInstance = new HstoreComparer(); + static readonly HstoreMutableComparer MutableComparerInstance = new HstoreMutableComparer(); - /// - /// Constructs an instance of the class. - /// - public NpgsqlHstoreTypeMapping() + public NpgsqlHstoreTypeMapping([NotNull] Type clrType) : base( new RelationalTypeMappingParameters( - new CoreTypeMappingParameters(typeof(Dictionary), null, ComparerInstance), - "hstore" - ), NpgsqlDbType.Hstore) {} + new CoreTypeMappingParameters(clrType, comparer: GetComparer(clrType)), + "hstore"), + NpgsqlDbType.Hstore) + { + } protected NpgsqlHstoreTypeMapping(RelationalTypeMappingParameters parameters) : base(parameters, NpgsqlDbType.Hstore) {} @@ -35,7 +38,7 @@ protected override RelationalTypeMapping Clone(RelationalTypeMappingParameters p protected override string GenerateNonNullSqlLiteral(object value) { var sb = new StringBuilder("HSTORE '"); - foreach (var kv in (Dictionary)value) + foreach (var kv in (IReadOnlyDictionary)value) { sb.Append('"'); sb.Append(kv.Key); // TODO: Escape @@ -56,19 +59,36 @@ protected override string GenerateNonNullSqlLiteral(object value) return sb.ToString(); } - sealed class HstoreComparer : ValueComparer> + static ValueComparer GetComparer(Type clrType) + { + if (clrType == typeof(Dictionary)) + return MutableComparerInstance; + + if (clrType == typeof(ImmutableDictionary)) + { + // Because ImmutableDictionary is immutable, we can use the default value comparer, which doesn't + // clone for snapshot and just does reference comparison. + // We could compare contents here if the references are different, but that would penalize the 99% case + // where a different reference means different contents, which would only save a very rare database update. + return null; + } + + throw new ArgumentException($"CLR type must be {nameof(Dictionary)} or {nameof(ImmutableDictionary)}"); + } + + sealed class HstoreMutableComparer : ValueComparer> { - public HstoreComparer() : base( + public HstoreMutableComparer() : base( (a, b) => Compare(a,b), o => o.GetHashCode(), o => o == null ? null : new Dictionary(o)) {} - static bool Compare(Dictionary a, Dictionary b) + static bool Compare(IReadOnlyDictionary a, IReadOnlyDictionary b) { - if (a == null) - return b == null; - if (b == null) + if (a is null) + return b is null; + if (b is null) return false; if (a.Count != b.Count) return false; diff --git a/src/EFCore.PG/Storage/Internal/NpgsqlTypeMappingSource.cs b/src/EFCore.PG/Storage/Internal/NpgsqlTypeMappingSource.cs index 9a24d6f40..1fd946ef7 100644 --- a/src/EFCore.PG/Storage/Internal/NpgsqlTypeMappingSource.cs +++ b/src/EFCore.PG/Storage/Internal/NpgsqlTypeMappingSource.cs @@ -2,6 +2,7 @@ using System.Collections; using System.Collections.Concurrent; using System.Collections.Generic; +using System.Collections.Immutable; using System.Data; using System.Diagnostics; using System.Linq; @@ -106,12 +107,13 @@ public class NpgsqlTypeMappingSource : RelationalTypeMappingSource readonly NpgsqlRangeTypeMapping _daterange; // Other types - readonly NpgsqlBoolTypeMapping _bool = new NpgsqlBoolTypeMapping(); - readonly NpgsqlBitTypeMapping _bit = new NpgsqlBitTypeMapping(); - readonly NpgsqlVarbitTypeMapping _varbit = new NpgsqlVarbitTypeMapping(); - readonly NpgsqlByteArrayTypeMapping _bytea = new NpgsqlByteArrayTypeMapping(); - readonly NpgsqlHstoreTypeMapping _hstore = new NpgsqlHstoreTypeMapping(); - readonly NpgsqlTidTypeMapping _tid = new NpgsqlTidTypeMapping(); + readonly NpgsqlBoolTypeMapping _bool = new NpgsqlBoolTypeMapping(); + readonly NpgsqlBitTypeMapping _bit = new NpgsqlBitTypeMapping(); + readonly NpgsqlVarbitTypeMapping _varbit = new NpgsqlVarbitTypeMapping(); + readonly NpgsqlByteArrayTypeMapping _bytea = new NpgsqlByteArrayTypeMapping(); + readonly NpgsqlHstoreTypeMapping _hstore = new NpgsqlHstoreTypeMapping(typeof(Dictionary)); + readonly NpgsqlHstoreTypeMapping _immutableHstore = new NpgsqlHstoreTypeMapping(typeof(ImmutableDictionary)); + readonly NpgsqlTidTypeMapping _tid = new NpgsqlTidTypeMapping(); // Special stuff // ReSharper disable once InconsistentNaming @@ -186,7 +188,7 @@ public NpgsqlTypeMappingSource([NotNull] TypeMappingSourceDependencies dependenc { "bit", new[] { _bit } }, { "bit varying", new[] { _varbit } }, { "varbit", new[] { _varbit } }, - { "hstore", new[] { _hstore } }, + { "hstore", new RelationalTypeMapping[] { _hstore, _immutableHstore } }, { "point", new[] { _point } }, { "box", new[] { _box } }, { "line", new[] { _line } }, @@ -215,46 +217,47 @@ public NpgsqlTypeMappingSource([NotNull] TypeMappingSourceDependencies dependenc var clrTypeMappings = new Dictionary { - { typeof(bool), _bool }, - { typeof(byte[]), _bytea }, - { typeof(float), _float4 }, - { typeof(double), _float8 }, - { typeof(decimal), _numeric }, - { typeof(Guid), _uuid }, - { typeof(byte), _int2Byte }, - { typeof(short), _int2 }, - { typeof(int), _int4 }, - { typeof(long), _int8 }, - { typeof(string), _text }, - { typeof(JsonDocument), _jsonbDocument }, - { typeof(JsonElement), _jsonbElement }, - { typeof(char), _singleChar }, - { typeof(DateTime), _timestamp }, - { typeof(TimeSpan), _interval }, - { typeof(DateTimeOffset), _timestamptzDto }, - { typeof(PhysicalAddress), _macaddr }, - { typeof(IPAddress), _inet }, - { typeof((IPAddress, int)), _cidr }, - { typeof(BitArray), _varbit }, - { typeof(Dictionary), _hstore }, - { typeof(NpgsqlTid), _tid }, - - { typeof(NpgsqlPoint), _point }, - { typeof(NpgsqlBox), _box }, - { typeof(NpgsqlLine), _line }, - { typeof(NpgsqlLSeg), _lseg }, - { typeof(NpgsqlPath), _path }, - { typeof(NpgsqlPolygon), _polygon }, - { typeof(NpgsqlCircle), _circle }, - - { typeof(NpgsqlRange), _int4range }, - { typeof(NpgsqlRange), _int8range }, - { typeof(NpgsqlRange), _numrange }, - { typeof(NpgsqlRange), _tsrange }, - - { typeof(NpgsqlTsQuery), _tsquery }, - { typeof(NpgsqlTsVector), _tsvector }, - { typeof(NpgsqlTsRankingNormalization), _rankingNormalization } + { typeof(bool), _bool }, + { typeof(byte[]), _bytea }, + { typeof(float), _float4 }, + { typeof(double), _float8 }, + { typeof(decimal), _numeric }, + { typeof(Guid), _uuid }, + { typeof(byte), _int2Byte }, + { typeof(short), _int2 }, + { typeof(int), _int4 }, + { typeof(long), _int8 }, + { typeof(string), _text }, + { typeof(JsonDocument), _jsonbDocument }, + { typeof(JsonElement), _jsonbElement }, + { typeof(char), _singleChar }, + { typeof(DateTime), _timestamp }, + { typeof(TimeSpan), _interval }, + { typeof(DateTimeOffset), _timestamptzDto }, + { typeof(PhysicalAddress), _macaddr }, + { typeof(IPAddress), _inet }, + { typeof((IPAddress, int)), _cidr }, + { typeof(BitArray), _varbit }, + { typeof(ImmutableDictionary), _immutableHstore }, + { typeof(Dictionary), _hstore }, + { typeof(NpgsqlTid), _tid }, + + { typeof(NpgsqlPoint), _point }, + { typeof(NpgsqlBox), _box }, + { typeof(NpgsqlLine), _line }, + { typeof(NpgsqlLSeg), _lseg }, + { typeof(NpgsqlPath), _path }, + { typeof(NpgsqlPolygon), _polygon }, + { typeof(NpgsqlCircle), _circle }, + + { typeof(NpgsqlRange), _int4range }, + { typeof(NpgsqlRange), _int8range }, + { typeof(NpgsqlRange), _numrange }, + { typeof(NpgsqlRange), _tsrange }, + + { typeof(NpgsqlTsQuery), _tsquery }, + { typeof(NpgsqlTsVector), _tsvector }, + { typeof(NpgsqlTsRankingNormalization), _rankingNormalization } }; StoreTypeMappings = new ConcurrentDictionary(storeTypeMappings, StringComparer.OrdinalIgnoreCase); diff --git a/test/EFCore.PG.FunctionalTests/BuiltInDataTypesNpgsqlTest.cs b/test/EFCore.PG.FunctionalTests/BuiltInDataTypesNpgsqlTest.cs index fb3f17c6a..6128ae4fa 100644 --- a/test/EFCore.PG.FunctionalTests/BuiltInDataTypesNpgsqlTest.cs +++ b/test/EFCore.PG.FunctionalTests/BuiltInDataTypesNpgsqlTest.cs @@ -1,5 +1,6 @@ using System; using System.Collections.Generic; +using System.Collections.Immutable; using System.ComponentModel.DataAnnotations.Schema; using System.Linq; using System.Net.NetworkInformation; @@ -115,6 +116,7 @@ public virtual void Can_query_using_any_mapped_data_type() StringAsJsonb = @"{""a"": ""b""}", StringAsJson = @"{""a"": ""b""}", DictionaryAsHstore = new Dictionary { { "a", "b" } }, + ImmutableDictionaryAsHstore = ImmutableDictionary.Empty.Add("c", "d"), NpgsqlRangeAsRange = new NpgsqlRange(4, true, 8, false), IntArrayAsIntArray= new[] { 2, 3 }, @@ -248,36 +250,39 @@ public virtual void Can_query_using_any_mapped_data_type() var param30 = new Dictionary { { "a", "b" } }; Assert.Same(entity, context.Set().Single(e => e.Int == 999 && e.DictionaryAsHstore == param30)); - var param31 = new NpgsqlRange(4, true, 8, false); - Assert.Same(entity, context.Set().Single(e => e.Int == 999 && e.NpgsqlRangeAsRange == param31)); + var param31 = ImmutableDictionary.Empty.Add("c", "d"); + Assert.Same(entity, context.Set().Single(e => e.Int == 999 && e.ImmutableDictionaryAsHstore == param31)); - var param32 = new[] { 2, 3 }; - Assert.Same(entity, context.Set().Single(e => e.Int == 999 && e.IntArrayAsIntArray == param32)); + var param32 = new NpgsqlRange(4, true, 8, false); + Assert.Same(entity, context.Set().Single(e => e.Int == 999 && e.NpgsqlRangeAsRange == param32)); - var param33 = new[] { PhysicalAddress.Parse("08-00-2B-01-02-03"), PhysicalAddress.Parse("08-00-2B-01-02-04") }; - Assert.Same(entity, context.Set().Single(e => e.Int == 999 && e.PhysicalAddressArrayAsMacaddrArray == param33)); + var param33 = new[] { 2, 3 }; + Assert.Same(entity, context.Set().Single(e => e.Int == 999 && e.IntArrayAsIntArray == param33)); + + var param34 = new[] { PhysicalAddress.Parse("08-00-2B-01-02-03"), PhysicalAddress.Parse("08-00-2B-01-02-04") }; + Assert.Same(entity, context.Set().Single(e => e.Int == 999 && e.PhysicalAddressArrayAsMacaddrArray == param34)); // ReSharper disable once ConvertToConstant.Local - var param34 = (uint)int.MaxValue + 1; - Assert.Same(entity, context.Set().Single(e => e.Int == 999 && e.UintAsXid == param34)); + var param35 = (uint)int.MaxValue + 1; + Assert.Same(entity, context.Set().Single(e => e.Int == 999 && e.UintAsXid == param35)); - var param35 = NpgsqlTsQuery.Parse("a & b"); - Assert.Same(entity, context.Set().Single(e => e.Int == 999 && e.SearchQuery == param35)); + var param36 = NpgsqlTsQuery.Parse("a & b"); + Assert.Same(entity, context.Set().Single(e => e.Int == 999 && e.SearchQuery == param36)); - var param36 = NpgsqlTsVector.Parse("a b"); - Assert.Same(entity, context.Set().Single(e => e.Int == 999 && e.SearchVector == param36)); + var param37 = NpgsqlTsVector.Parse("a b"); + Assert.Same(entity, context.Set().Single(e => e.Int == 999 && e.SearchVector == param37)); // ReSharper disable once ConvertToConstant.Local - var param37 = NpgsqlTsRankingNormalization.DivideByLength; - Assert.Same(entity, context.Set().Single(e => e.Int == 999 && e.RankingNormalization == param37)); + var param38 = NpgsqlTsRankingNormalization.DivideByLength; + Assert.Same(entity, context.Set().Single(e => e.Int == 999 && e.RankingNormalization == param38)); // ReSharper disable once ConvertToConstant.Local - var param38 = 12724u; - Assert.Same(entity, context.Set().Single(e => e.Int == 999 && e.Regconfig == param38)); + var param39 = 12724u; + Assert.Same(entity, context.Set().Single(e => e.Int == 999 && e.Regconfig == param39)); // ReSharper disable once ConvertToConstant.Local - var param39 = Mood.Sad; - Assert.Same(entity, context.Set().Single(e => e.Int == 999 && e.Mood == param39)); + var param40 = Mood.Sad; + Assert.Same(entity, context.Set().Single(e => e.Int == 999 && e.Mood == param40)); } } @@ -408,32 +413,35 @@ public virtual void Can_query_using_any_mapped_data_types_with_nulls() Dictionary param30 = null; Assert.Same(entity, context.Set().Single(e => e.Int == 911 && e.DictionaryAsHstore == param30)); - NpgsqlRange? param31 = null; - Assert.Same(entity, context.Set().Single(e => e.Int == 911 && e.NpgsqlRangeAsRange == param31)); + ImmutableDictionary param31 = null; + Assert.Same(entity, context.Set().Single(e => e.Int == 911 && e.ImmutableDictionaryAsHstore == param31)); + + NpgsqlRange? param32 = null; + Assert.Same(entity, context.Set().Single(e => e.Int == 911 && e.NpgsqlRangeAsRange == param32)); - int[] param32 = null; - Assert.Same(entity, context.Set().Single(e => e.Int == 911 && e.IntArrayAsIntArray == param32)); + int[] param33 = null; + Assert.Same(entity, context.Set().Single(e => e.Int == 911 && e.IntArrayAsIntArray == param33)); - PhysicalAddress[] param33 = null; - Assert.Same(entity, context.Set().Single(e => e.Int == 911 && e.PhysicalAddressArrayAsMacaddrArray== param33)); + PhysicalAddress[] param34 = null; + Assert.Same(entity, context.Set().Single(e => e.Int == 911 && e.PhysicalAddressArrayAsMacaddrArray== param34)); - uint? param34 = null; - Assert.Same(entity, context.Set().Single(e => e.Int == 911 && e.UintAsXid == param34)); + uint? param35 = null; + Assert.Same(entity, context.Set().Single(e => e.Int == 911 && e.UintAsXid == param35)); - NpgsqlTsQuery param35 = null; - Assert.Same(entity, context.Set().Single(e => e.Int == 911 && e.SearchQuery == param35)); + NpgsqlTsQuery param36 = null; + Assert.Same(entity, context.Set().Single(e => e.Int == 911 && e.SearchQuery == param36)); - NpgsqlTsVector param36 = null; - Assert.Same(entity, context.Set().Single(e => e.Int == 911 && e.SearchVector == param36)); + NpgsqlTsVector param37 = null; + Assert.Same(entity, context.Set().Single(e => e.Int == 911 && e.SearchVector == param37)); - NpgsqlTsRankingNormalization? param37 = null; - Assert.Same(entity, context.Set().Single(e => e.Int == 911 && e.RankingNormalization == param37)); + NpgsqlTsRankingNormalization? param38 = null; + Assert.Same(entity, context.Set().Single(e => e.Int == 911 && e.RankingNormalization == param38)); - uint? param38 = null; - Assert.Same(entity, context.Set().Single(e => e.Int == 911 && e.Regconfig == param38)); + uint? param39 = null; + Assert.Same(entity, context.Set().Single(e => e.Int == 911 && e.Regconfig == param39)); - Mood? param39 = null; - Assert.Same(entity, context.Set().Single(e => e.Int == 911 && e.Mood == param39)); + Mood? param40 = null; + Assert.Same(entity, context.Set().Single(e => e.Int == 911 && e.Mood == param40)); } } @@ -682,6 +690,7 @@ static void AssertNullMappedNullableDataTypes(MappedNullableDataTypes entity, in Assert.Null(entity.StringAsJsonb); Assert.Null(entity.StringAsJson); Assert.Null(entity.DictionaryAsHstore); + Assert.Null(entity.ImmutableDictionaryAsHstore); Assert.Null(entity.NpgsqlRangeAsRange); Assert.Null(entity.IntArrayAsIntArray); @@ -1327,6 +1336,9 @@ protected class MappedNullableDataTypes [Column(TypeName = "hstore")] public Dictionary DictionaryAsHstore { get; set; } + [Column(TypeName = "hstore")] + public ImmutableDictionary ImmutableDictionaryAsHstore { get; set; } + [Column(TypeName = "int4range")] public NpgsqlRange? NpgsqlRangeAsRange { get; set; } diff --git a/test/EFCore.PG.Tests/Storage/NpgsqlTypeMappingTest.cs b/test/EFCore.PG.Tests/Storage/NpgsqlTypeMappingTest.cs index 2c6290f16..12b2d8536 100644 --- a/test/EFCore.PG.Tests/Storage/NpgsqlTypeMappingTest.cs +++ b/test/EFCore.PG.Tests/Storage/NpgsqlTypeMappingTest.cs @@ -1,6 +1,7 @@ using System; using System.Collections; using System.Collections.Generic; +using System.Collections.Immutable; using System.Net; using System.Net.NetworkInformation; using System.Text.Json; @@ -308,7 +309,7 @@ public void GenerateSqlLiteral_returns_hstore_literal() })); [Fact] - public void ValueComparer_hstore() + public void ValueComparer_hstore_as_Dictionary() { var source = new Dictionary { @@ -325,6 +326,21 @@ public void ValueComparer_hstore() Assert.False(comparer.Equals(source, snapshot)); } + [Fact] + public void ValueComparer_hstore_as_ImmutableDictionary() + { + var source = ImmutableDictionary.Empty + .Add("k1", "v1") + .Add("k2", "v2"); + + var comparer = Mapper.FindMapping(typeof(ImmutableDictionary), "hstore").Comparer; + var snapshot = comparer.Snapshot(source); + Assert.Equal(source, snapshot); + Assert.True(comparer.Equals(source, snapshot)); + source = source.Remove("k1"); + Assert.False(comparer.Equals(source, snapshot)); + } + [Fact] public void GenerateSqlLiteral_returns_enum_literal() {