Correct date/time mapping with precision (#2580)

Closes #2579

(cherry picked from commit 9573961)

src/EFCore.PG.NodaTime/Storage/Internal/LegacyTimestampInstantMapping.cs

@@ -62,6 +62,15 @@ public override RelationalTypeMapping Clone(string storeType, int? size)
     public override CoreTypeMapping Clone(ValueConverter? converter)
         => new LegacyTimestampInstantMapping(Parameters.WithComposedConverter(converter));
 
+    /// <summary>
+    ///     This is an internal API that supports the Entity Framework Core infrastructure and not subject to
+    ///     the same compatibility standards as public APIs. It may be changed or removed without notice in
+    ///     any release. You should only use it directly in your code with extreme caution and knowing that
+    ///     doing so can result in application failures when updating to a new Entity Framework Core release.
+    /// </summary>
+    protected override string ProcessStoreType(RelationalTypeMappingParameters parameters, string storeType, string _)
+        => parameters.Precision is null ? storeType : $"timestamp({parameters.Precision}) without time zone";
+
     /// <summary>
     ///     This is an internal API that supports the Entity Framework Core infrastructure and not subject to
     ///     the same compatibility standards as public APIs. It may be changed or removed without notice in

src/EFCore.PG.NodaTime/Storage/Internal/NpgsqlNodaTimeTypeMappingSourcePlugin.cs

@@ -236,15 +236,16 @@ public NpgsqlNodaTimeTypeMappingSourcePlugin(ISqlGenerationHelper sqlGenerationH
     ///     doing so can result in application failures when updating to a new Entity Framework Core release.
     /// </summary>
     public virtual RelationalTypeMapping? FindMapping(in RelationalTypeMappingInfo mappingInfo)
-        => FindExistingMapping(mappingInfo) ?? FindArrayMapping(mappingInfo);
+        => FindBaseMapping(mappingInfo)?.Clone(mappingInfo)
+            ?? FindArrayMapping(mappingInfo)?.Clone(mappingInfo);
 
     /// <summary>
     ///     This is an internal API that supports the Entity Framework Core infrastructure and not subject to
     ///     the same compatibility standards as public APIs. It may be changed or removed without notice in
     ///     any release. You should only use it directly in your code with extreme caution and knowing that
     ///     doing so can result in application failures when updating to a new Entity Framework Core release.
     /// </summary>
-    protected virtual RelationalTypeMapping? FindExistingMapping(in RelationalTypeMappingInfo mappingInfo)
+    protected virtual RelationalTypeMapping? FindBaseMapping(in RelationalTypeMappingInfo mappingInfo)
     {
         var clrType = mappingInfo.ClrType;
         var storeTypeName = mappingInfo.StoreTypeName;
@@ -289,16 +290,9 @@ public NpgsqlNodaTimeTypeMappingSourcePlugin(ISqlGenerationHelper sqlGenerationH
             }
         }
 
-        if (clrType is null || !ClrTypeMappings.TryGetValue(clrType, out var mapping))
-        {
-            return null;
-        }
-
-        // All PostgreSQL date/time types accept a precision except for date
-        // TODO: Cache size/precision/scale mappings?
-        return mappingInfo.Precision.HasValue && mapping.ClrType != typeof(LocalDate)
-            ? mapping.Clone($"{mapping.StoreType}({mappingInfo.Precision.Value})", null)
-            : mapping;
+        return clrType is not null && ClrTypeMappings.TryGetValue(clrType, out var mapping)
+            ? mapping
+            : null;
     }
 
     /// <summary>

src/EFCore.PG.NodaTime/Storage/Internal/TimeMapping.cs

@@ -67,6 +67,15 @@ public override RelationalTypeMapping Clone(string storeType, int? size)
     public override CoreTypeMapping Clone(ValueConverter? converter)
         => new TimeMapping(Parameters.WithComposedConverter(converter));
 
+    /// <summary>
+    ///     This is an internal API that supports the Entity Framework Core infrastructure and not subject to
+    ///     the same compatibility standards as public APIs. It may be changed or removed without notice in
+    ///     any release. You should only use it directly in your code with extreme caution and knowing that
+    ///     doing so can result in application failures when updating to a new Entity Framework Core release.
+    /// </summary>
+    protected override string ProcessStoreType(RelationalTypeMappingParameters parameters, string storeType, string _)
+        => parameters.Precision is null ? storeType : $"time({parameters.Precision})";
+
     /// <summary>
     ///     This is an internal API that supports the Entity Framework Core infrastructure and not subject to
     ///     the same compatibility standards as public APIs. It may be changed or removed without notice in

src/EFCore.PG.NodaTime/Storage/Internal/TimeTzMapping.cs

@@ -79,6 +79,15 @@ public override RelationalTypeMapping Clone(string storeType, int? size)
     public override CoreTypeMapping Clone(ValueConverter? converter)
         => new TimeTzMapping(Parameters.WithComposedConverter(converter));
 
+    /// <summary>
+    ///     This is an internal API that supports the Entity Framework Core infrastructure and not subject to
+    ///     the same compatibility standards as public APIs. It may be changed or removed without notice in
+    ///     any release. You should only use it directly in your code with extreme caution and knowing that
+    ///     doing so can result in application failures when updating to a new Entity Framework Core release.
+    /// </summary>
+    protected override string ProcessStoreType(RelationalTypeMappingParameters parameters, string storeType, string _)
+        => parameters.Precision is null ? storeType : $"timestamp({parameters.Precision}) with time zone";
+
     /// <summary>
     ///     This is an internal API that supports the Entity Framework Core infrastructure and not subject to
     ///     the same compatibility standards as public APIs. It may be changed or removed without notice in

src/EFCore.PG.NodaTime/Storage/Internal/TimestampLocalDateTimeMapping.cs

@@ -66,6 +66,15 @@ public override RelationalTypeMapping Clone(string storeType, int? size)
     public override CoreTypeMapping Clone(ValueConverter? converter)
         => new TimestampLocalDateTimeMapping(Parameters.WithComposedConverter(converter));
 
+    /// <summary>
+    ///     This is an internal API that supports the Entity Framework Core infrastructure and not subject to
+    ///     the same compatibility standards as public APIs. It may be changed or removed without notice in
+    ///     any release. You should only use it directly in your code with extreme caution and knowing that
+    ///     doing so can result in application failures when updating to a new Entity Framework Core release.
+    /// </summary>
+    protected override string ProcessStoreType(RelationalTypeMappingParameters parameters, string storeType, string _)
+        => parameters.Precision is null ? storeType : $"timestamp({parameters.Precision}) without time zone";
+
     /// <summary>
     ///     This is an internal API that supports the Entity Framework Core infrastructure and not subject to
     ///     the same compatibility standards as public APIs. It may be changed or removed without notice in

src/EFCore.PG.NodaTime/Storage/Internal/TimestampTzInstantMapping.cs

@@ -56,6 +56,15 @@ public override RelationalTypeMapping Clone(string storeType, int? size)
     public override CoreTypeMapping Clone(ValueConverter? converter)
         => new TimestampTzInstantMapping(Parameters.WithComposedConverter(converter));
 
+    /// <summary>
+    ///     This is an internal API that supports the Entity Framework Core infrastructure and not subject to
+    ///     the same compatibility standards as public APIs. It may be changed or removed without notice in
+    ///     any release. You should only use it directly in your code with extreme caution and knowing that
+    ///     doing so can result in application failures when updating to a new Entity Framework Core release.
+    /// </summary>
+    protected override string ProcessStoreType(RelationalTypeMappingParameters parameters, string storeType, string _)
+        => parameters.Precision is null ? storeType : $"timestamp({parameters.Precision}) with time zone";
+
     /// <summary>
     ///     This is an internal API that supports the Entity Framework Core infrastructure and not subject to
     ///     the same compatibility standards as public APIs. It may be changed or removed without notice in

src/EFCore.PG.NodaTime/Storage/Internal/TimestampTzOffsetDateTimeMapping.cs

@@ -66,6 +66,15 @@ public override RelationalTypeMapping Clone(string storeType, int? size)
     public override CoreTypeMapping Clone(ValueConverter? converter)
         => new TimestampTzOffsetDateTimeMapping(Parameters.WithComposedConverter(converter));
 
+    /// <summary>
+    ///     This is an internal API that supports the Entity Framework Core infrastructure and not subject to
+    ///     the same compatibility standards as public APIs. It may be changed or removed without notice in
+    ///     any release. You should only use it directly in your code with extreme caution and knowing that
+    ///     doing so can result in application failures when updating to a new Entity Framework Core release.
+    /// </summary>
+    protected override string ProcessStoreType(RelationalTypeMappingParameters parameters, string storeType, string _)
+        => parameters.Precision is null ? storeType : $"timestamp({parameters.Precision}) with time zone";
+
     /// <summary>
     ///     This is an internal API that supports the Entity Framework Core infrastructure and not subject to
     ///     the same compatibility standards as public APIs. It may be changed or removed without notice in

src/EFCore.PG.NodaTime/Storage/Internal/TimestampTzZonedDateTimeMapping.cs

@@ -61,6 +61,15 @@ public override RelationalTypeMapping Clone(string storeType, int? size)
     public override CoreTypeMapping Clone(ValueConverter? converter)
         => new TimestampTzZonedDateTimeMapping(Parameters.WithComposedConverter(converter));
 
+    /// <summary>
+    ///     This is an internal API that supports the Entity Framework Core infrastructure and not subject to
+    ///     the same compatibility standards as public APIs. It may be changed or removed without notice in
+    ///     any release. You should only use it directly in your code with extreme caution and knowing that
+    ///     doing so can result in application failures when updating to a new Entity Framework Core release.
+    /// </summary>
+    protected override string ProcessStoreType(RelationalTypeMappingParameters parameters, string storeType, string _)
+        => parameters.Precision is null ? storeType : $"timestamp({parameters.Precision}) with time zone";
+
     /// <summary>
     ///     This is an internal API that supports the Entity Framework Core infrastructure and not subject to
     ///     the same compatibility standards as public APIs. It may be changed or removed without notice in

src/EFCore.PG/Storage/Internal/Mapping/NpgsqlTimeTypeMapping.cs

@@ -36,6 +36,15 @@ protected NpgsqlTimeTypeMapping(RelationalTypeMappingParameters parameters)
     protected override RelationalTypeMapping Clone(RelationalTypeMappingParameters parameters)
         => new NpgsqlTimeTypeMapping(parameters);
 
+    /// <summary>
+    ///     This is an internal API that supports the Entity Framework Core infrastructure and not subject to
+    ///     the same compatibility standards as public APIs. It may be changed or removed without notice in
+    ///     any release. You should only use it directly in your code with extreme caution and knowing that
+    ///     doing so can result in application failures when updating to a new Entity Framework Core release.
+    /// </summary>
+    protected override string ProcessStoreType(RelationalTypeMappingParameters parameters, string storeType, string _)
+        => parameters.Precision is null ? storeType : $"time({parameters.Precision}) without time zone";
+
     /// <summary>
     ///     This is an internal API that supports the Entity Framework Core infrastructure and not subject to
     ///     the same compatibility standards as public APIs. It may be changed or removed without notice in

src/EFCore.PG/Storage/Internal/Mapping/NpgsqlTimeTzTypeMapping.cs

@@ -34,6 +34,15 @@ protected NpgsqlTimeTzTypeMapping(RelationalTypeMappingParameters parameters)
     protected override RelationalTypeMapping Clone(RelationalTypeMappingParameters parameters)
         => new NpgsqlTimeTzTypeMapping(parameters);
 
+    /// <summary>
+    ///     This is an internal API that supports the Entity Framework Core infrastructure and not subject to
+    ///     the same compatibility standards as public APIs. It may be changed or removed without notice in
+    ///     any release. You should only use it directly in your code with extreme caution and knowing that
+    ///     doing so can result in application failures when updating to a new Entity Framework Core release.
+    /// </summary>
+    protected override string ProcessStoreType(RelationalTypeMappingParameters parameters, string storeType, string _)
+        => parameters.Precision is null ? storeType : $"time({parameters.Precision}) with time zone";
+
     /// <summary>
     ///     This is an internal API that supports the Entity Framework Core infrastructure and not subject to
     ///     the same compatibility standards as public APIs. It may be changed or removed without notice in

src/EFCore.PG/Storage/Internal/NpgsqlTypeMappingSource.cs

@@ -493,11 +493,11 @@ protected virtual void SetupEnumMappings(ISqlGenerationHelper sqlGenerationHelpe
     /// </summary>
     protected override RelationalTypeMapping? FindMapping(in RelationalTypeMappingInfo mappingInfo) =>
         // First, try any plugins, allowing them to override built-in mappings (e.g. NodaTime)
-        base.FindMapping(mappingInfo) ??
-        FindBaseMapping(mappingInfo)?.Clone(mappingInfo) ??
-        FindArrayMapping(mappingInfo)?.Clone(mappingInfo) ??
-        FindRowValueMapping(mappingInfo)?.Clone(mappingInfo) ??
-        FindUserRangeMapping(mappingInfo);
+        base.FindMapping(mappingInfo)
+        ?? FindBaseMapping(mappingInfo)?.Clone(mappingInfo)
+        ?? FindArrayMapping(mappingInfo)?.Clone(mappingInfo)
+        ?? FindRowValueMapping(mappingInfo)?.Clone(mappingInfo)
+        ?? FindUserRangeMapping(mappingInfo);
 
     /// <summary>
     ///     This is an internal API that supports the Entity Framework Core infrastructure and not subject to

test/EFCore.PG.NodaTime.FunctionalTests/NpgsqlNodaTimeTypeMappingTest.cs

@@ -26,9 +26,15 @@ public void Instant_maps_to_timestamp_legacy()
     {
         var mapping = GetMapping(typeof(Instant), "timestamp");
         Assert.Same(typeof(Instant), mapping.ClrType);
-        Assert.Equal("timestamp without time zone", mapping.StoreType);
+        Assert.Equal("timestamp", mapping.StoreType);
     }
 
+    [Fact]
+    public void Instant_with_precision()
+        => Assert.Equal(
+            "timestamp(3) with time zone",
+            Mapper.FindMapping(typeof(Instant), "timestamp with time zone", precision: 3)!.StoreType);
+
     [Fact]
     public void GenerateSqlLiteral_returns_LocalDateTime_literal()
     {
@@ -520,26 +526,21 @@ public void GenerateCodeLiteral_returns_OffsetTime_literal()
 
     [Fact]
     public void Duration_is_properly_mapped()
-    {
-        var mapping = GetMapping(typeof(Duration));
-
-        Assert.Equal("interval", mapping.StoreType);
-        Assert.Same(typeof(Duration), mapping.ClrType);
-
-        Assert.Same(mapping, GetMapping(typeof(Duration), "interval"));
-    }
+        => Assert.All(new[] { GetMapping(typeof(Duration)), GetMapping(typeof(Duration), "interval") },
+            m =>
+            {
+                Assert.Equal("interval", m.StoreType);
+                Assert.Same(typeof(Duration), m.ClrType);
+            });
 
     [Fact]
     public void Period_is_properly_mapped()
-    {
-        var mapping = GetMapping(typeof(Period));
-
-        Assert.Equal("interval", mapping.StoreType);
-        Assert.Same(typeof(Period), mapping.ClrType);
-
-        Assert.Same(mapping, GetMapping(typeof(Period)));
-        Assert.Same(mapping, GetMapping(typeof(Period), "interval"));
-    }
+        => Assert.All(new[] { GetMapping(typeof(Period)), GetMapping(typeof(Period), "interval") },
+            m =>
+            {
+                Assert.Equal("interval", m.StoreType);
+                Assert.Same(typeof(Period), m.ClrType);
+            });
 
     [Fact]
     public void GenerateSqlLiteral_returns_Period_literal()

test/EFCore.PG.Tests/Storage/NpgsqlTypeMappingTest.cs

@@ -28,6 +28,12 @@ public void Timestamp_maps_to_DateTime_by_default()
     public void Timestamptz_maps_to_DateTime_by_default()
         => Assert.Same(typeof(DateTime), GetMapping("timestamp with time zone").ClrType);
 
+    [Fact]
+    public void DateTime_with_precision()
+        => Assert.Equal(
+            "timestamp(3) with time zone",
+            Mapper.FindMapping(typeof(DateTime), "timestamp with time zone", precision: 3)!.StoreType);
+
     [Fact]
     public void GenerateSqlLiteral_returns_date_literal()
     {