Fix timestamptz interval arithmetic

src/EFCore.PG.NodaTime/Query/Internal/NpgsqlNodaTimeMemberTranslatorPlugin.cs

@@ -1,4 +1,5 @@
 using Npgsql.EntityFrameworkCore.PostgreSQL.Query;
+using Npgsql.EntityFrameworkCore.PostgreSQL.Query.ExpressionTranslators.Internal;
 using Npgsql.EntityFrameworkCore.PostgreSQL.Storage.Internal;
 
 namespace Npgsql.EntityFrameworkCore.PostgreSQL.NodaTime.Query.Internal;
@@ -107,26 +108,12 @@ public class NpgsqlNodaTimeMemberTranslator(
         return null;
     }
 
+    // Duration has the same component (e.g. Hours) and total (e.g. TotalHours) members as TimeSpan
+    // with the same semantics, so we can reuse the translation.
+#pragma warning disable EF1001
     private SqlExpression? TranslateDuration(SqlExpression instance, MemberInfo member)
-    {
-        return member.Name switch
-        {
-            nameof(Duration.TotalDays) => TranslateDurationTotalMember(instance, 86400),
-            nameof(Duration.TotalHours) => TranslateDurationTotalMember(instance, 3600),
-            nameof(Duration.TotalMinutes) => TranslateDurationTotalMember(instance, 60),
-            nameof(Duration.TotalSeconds) => GetDatePartExpressionDouble(instance, "epoch"),
-            nameof(Duration.TotalMilliseconds) => TranslateDurationTotalMember(instance, 0.001),
-            nameof(Duration.Days) => GetDatePartExpression(instance, "day"),
-            nameof(Duration.Hours) => GetDatePartExpression(instance, "hour"),
-            nameof(Duration.Minutes) => GetDatePartExpression(instance, "minute"),
-            nameof(Duration.Seconds) => GetDatePartExpression(instance, "second", true),
-            nameof(Duration.Milliseconds) => null, // Too annoying, floating point and sub-millisecond handling
-            _ => null,
-        };
-
-        SqlExpression TranslateDurationTotalMember(SqlExpression instance, double divisor)
-            => _sqlExpressionFactory.Divide(GetDatePartExpressionDouble(instance, "epoch"), _sqlExpressionFactory.Constant(divisor));
-    }
+        => NpgsqlTimeSpanMemberTranslator.TranslateIntervalMember(_sqlExpressionFactory, instance, member.Name);
+#pragma warning restore EF1001
 
     private SqlExpression? TranslateInterval(SqlExpression instance, MemberInfo member)
     {

src/EFCore.PG/Query/ExpressionTranslators/Internal/NpgsqlDateTimeMethodTranslator.cs

@@ -1,5 +1,6 @@
 using Npgsql.EntityFrameworkCore.PostgreSQL.Query.Expressions;
 using Npgsql.EntityFrameworkCore.PostgreSQL.Query.Expressions.Internal;
+using Npgsql.EntityFrameworkCore.PostgreSQL.Storage.Internal;
 using Npgsql.EntityFrameworkCore.PostgreSQL.Storage.Internal.Mapping;
 using static Npgsql.EntityFrameworkCore.PostgreSQL.Utilities.Statics;
 
@@ -113,9 +114,29 @@ public class NpgsqlDateTimeMethodTranslator(
             _ => null
         };
 
-        return datePart is not null && CreateIntervalExpression(arguments[0], datePart) is SqlExpression interval
-            ? _sqlExpressionFactory.Add(instance, interval, instance.TypeMapping)
-            : null;
+        if (datePart is null || CreateIntervalExpression(arguments[0], datePart) is not { } interval)
+        {
+            return null;
+        }
+
+        // With the legacy timestamp behavior, a DateTimeOffset has the local offset, and AddMonths/AddYears add calendar months in that
+        // offset, like PostgreSQL's timestamptz + interval in the session time zone; so that arithmetic is left as-is (the SQL binary
+        // expression is created with its type mapping, so NpgsqlSqlExpressionFactory doesn't perform it in UTC).
+        // AddDays etc. add an exact amount of time, so they're performed in UTC like other DateTimeOffset arithmetic.
+        if (NpgsqlTypeMappingSource.LegacyTimestampBehavior
+            && instance.Type == typeof(DateTimeOffset)
+            && datePart is "years" or "months")
+        {
+            var dateTimeOffset = _sqlExpressionFactory.ApplyDefaultTypeMapping(instance);
+            return new SqlBinaryExpression(
+                ExpressionType.Add, dateTimeOffset, interval, typeof(DateTimeOffset), dateTimeOffset.TypeMapping);
+        }
+
+        // PostgreSQL timestamptz + interval adds days and months in the session time zone,
+        // whereas DateTime/DateTimeOffset values mapped to timestamptz are UTC.
+        // The arithmetic is performed on a UTC timestamp instead by NpgsqlSqlExpressionFactory,
+        // once the instance's type mapping is known (e.g. a parameter's is inferred from the rest of the query).
+        return _sqlExpressionFactory.Add(instance, interval, instance.TypeMapping);
     }
 
     private SqlExpression? TranslateDateTime(
@@ -241,19 +262,16 @@ public class NpgsqlDateTimeMethodTranslator(
             }
 
             // For months and years, date + interval yields a timestamp (since interval could have a time component), so we need to cast
-            // the results back to date
-            if (method.DeclaringType == typeof(DateOnly) && method.Name == nameof(DateOnly.AddMonths)
-                && CreateIntervalExpression(arguments[0], "months") is SqlExpression interval1)
-            {
-                return _sqlExpressionFactory.Convert(
-                    _sqlExpressionFactory.Add(instance, interval1, instance.TypeMapping), typeof(DateOnly));
-            }
-
-            if (method.DeclaringType == typeof(DateOnly) && method.Name == nameof(DateOnly.AddYears)
-                && CreateIntervalExpression(arguments[0], "years") is SqlExpression interval2)
+            // the results back to date.
+            // DateOnly is always mapped to date: apply that mapping to an unmapped instance (e.g. a parameter), as it would otherwise be
+            // inferred from the interval.
+            if (method.DeclaringType == typeof(DateOnly)
+                && method.Name is nameof(DateOnly.AddMonths) or nameof(DateOnly.AddYears)
+                && CreateIntervalExpression(arguments[0], method.Name == nameof(DateOnly.AddMonths) ? "months" : "years") is { } interval)
             {
+                var date = _sqlExpressionFactory.ApplyDefaultTypeMapping(instance);
                 return _sqlExpressionFactory.Convert(
-                    _sqlExpressionFactory.Add(instance, interval2, instance.TypeMapping), typeof(DateOnly));
+                    _sqlExpressionFactory.Add(date, interval, _timestampMapping), typeof(DateOnly), date.TypeMapping);
             }
         }
 
@@ -380,7 +398,10 @@ public class NpgsqlDateTimeMethodTranslator(
                 return null;
             }
 
-            return _sqlExpressionFactory.Fragment(FormattableString.Invariant($"INTERVAL '{constantExpression.Value} {datePart}'"));
+            // The fragment is typed as an interval, so that e.g. timestamptz + interval is recognized as such (see
+            // NpgsqlSqlExpressionFactory), including when the timestamp's type mapping is only inferred later (e.g. a parameter).
+            return new SqlFragmentExpression(
+                FormattableString.Invariant($"INTERVAL '{constantExpression.Value} {datePart}'"), typeof(TimeSpan), _intervalMapping);
         }
 
         // For non-constants, we can't parameterize INTERVAL '1 days'. Instead, we use CAST($1 || ' days' AS interval).

src/EFCore.PG/Query/ExpressionTranslators/Internal/NpgsqlTimeSpanMemberTranslator.cs

@@ -1,3 +1,4 @@
+using Npgsql.EntityFrameworkCore.PostgreSQL.Storage.Internal.Mapping;
 using static Npgsql.EntityFrameworkCore.PostgreSQL.Utilities.Statics;
 
 namespace Npgsql.EntityFrameworkCore.PostgreSQL.Query.ExpressionTranslators.Internal;
@@ -40,48 +41,91 @@ public NpgsqlTimeSpanMemberTranslator(ISqlExpressionFactory sqlExpressionFactory
         Check.NotNull(member, nameof(member));
         Check.NotNull(returnType, nameof(returnType));
 
-        if (member.DeclaringType == typeof(TimeSpan) && instance is not null)
-        {
-            return member.Name switch
-            {
-                nameof(TimeSpan.Days) => Floor(DatePart("day", instance)),
-                nameof(TimeSpan.Hours) => Floor(DatePart("hour", instance)),
-                nameof(TimeSpan.Minutes) => Floor(DatePart("minute", instance)),
-                nameof(TimeSpan.Seconds) => Floor(DatePart("second", instance)),
-                nameof(TimeSpan.Milliseconds) => _sqlExpressionFactory.Modulo(
-                    Floor(DatePart("millisecond", instance)),
-                    _sqlExpressionFactory.Constant(1000)),
+        return member.DeclaringType == typeof(TimeSpan) && instance is not null
+            ? TranslateIntervalMember(_sqlExpressionFactory, instance, member.Name)
+            : null;
+    }
+
+    /// <summary>
+    ///     Translates a member of a type representing an exact amount of time which is mapped to PostgreSQL interval,
+    ///     i.e. <see cref="TimeSpan" /> and NodaTime's Duration, which have the same component (e.g. <c>Hours</c>)
+    ///     and total (e.g. <c>TotalHours</c>) members, with the same semantics.
+    /// </summary>
+    /// <remarks>
+    ///     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.
+    /// </remarks>
+    public static SqlExpression? TranslateIntervalMember(
+        ISqlExpressionFactory sqlExpressionFactory,
+        SqlExpression interval,
+        string memberName)
+    {
+        // A TimeSpan can also be mapped to PostgreSQL time (e.g. DateTime.TimeOfDay), which is a time of day from 00:00 to 24:00,
+        // so its date_part() values are the components already (except for 24:00, i.e. one day), and justify_hours() doesn't accept it.
+        var isTime = interval.TypeMapping is INpgsqlTypeMapping { NpgsqlDbType: NpgsqlDbType.Time };
 
-                nameof(TimeSpan.TotalDays) => TranslateDurationTotalMember(instance, 86400),
-                nameof(TimeSpan.TotalHours) => TranslateDurationTotalMember(instance, 3600),
-                nameof(TimeSpan.TotalMinutes) => TranslateDurationTotalMember(instance, 60),
-                nameof(TimeSpan.TotalSeconds) => DatePart("epoch", instance),
-                nameof(TimeSpan.TotalMilliseconds) => TranslateDurationTotalMember(instance, 0.001),
+        return memberName switch
+        {
+            // The components are truncated towards zero, and all have the value's sign.
+            // An interval's day and time fields are kept separate (e.g. '1 day -02:00:00'), and the time field can exceed a day.
+            // justify_hours() moves whole days into the day field and gives both fields the same sign,
+            // so that each date_part() returns the corresponding component
+            nameof(TimeSpan.Days) when isTime => sqlExpressionFactory.Divide(
+                ToInt(DatePart("hour", interval)), sqlExpressionFactory.Constant(24)),
+            nameof(TimeSpan.Hours) when isTime => sqlExpressionFactory.Modulo(
+                ToInt(DatePart("hour", interval)), sqlExpressionFactory.Constant(24)),
+            nameof(TimeSpan.Days) => ToInt(DatePart("day", JustifyHours())),
+            nameof(TimeSpan.Hours) => ToInt(DatePart("hour", JustifyHours())),
+            nameof(TimeSpan.Minutes) => ToInt(DatePart("minute", JustifyHours())),
+            // date_part('second') includes the fractional seconds
+            nameof(TimeSpan.Seconds) => ToInt(Truncate(DatePart("second", JustifyHours()))),
+            // date_part('millisecond') includes the seconds and PostgreSQL's integer % truncates
+            nameof(TimeSpan.Milliseconds) => sqlExpressionFactory.Modulo(
+                ToInt(Truncate(DatePart("millisecond", JustifyHours()))),
+                sqlExpressionFactory.Constant(1000)),
 
-                _ => null
-            };
-        }
+            nameof(TimeSpan.TotalDays) => TotalMember(86400),
+            nameof(TimeSpan.TotalHours) => TotalMember(3600),
+            nameof(TimeSpan.TotalMinutes) => TotalMember(60),
+            nameof(TimeSpan.TotalSeconds) => DatePart("epoch", interval),
+            nameof(TimeSpan.TotalMilliseconds) => TotalMember(0.001),
 
-        return null;
+            _ => null
+        };
 
-        SqlExpression Floor(SqlExpression value)
-            => _sqlExpressionFactory.Convert(
-                _sqlExpressionFactory.Function(
-                    "floor",
-                    [value],
+        SqlExpression JustifyHours()
+            => isTime
+                ? interval
+                : sqlExpressionFactory.Function(
+                    "justify_hours",
+                    [interval],
                     nullable: true,
                     argumentsPropagateNullability: TrueArrays[1],
-                    typeof(double)),
-                typeof(int));
+                    interval.Type,
+                    interval.TypeMapping);
 
         SqlExpression DatePart(string part, SqlExpression value)
-            => _sqlExpressionFactory.Function(
-                "date_part", [_sqlExpressionFactory.Constant(part), value],
+            => sqlExpressionFactory.Function(
+                "date_part",
+                [sqlExpressionFactory.Constant(part), value],
                 nullable: true,
                 argumentsPropagateNullability: FalseTrueArray,
-                returnType);
+                typeof(double));
+
+        SqlExpression Truncate(SqlExpression value)
+            => sqlExpressionFactory.Function(
+                "trunc",
+                [value],
+                nullable: true,
+                argumentsPropagateNullability: TrueArrays[1],
+                typeof(double));
+
+        SqlExpression ToInt(SqlExpression value)
+            => sqlExpressionFactory.Convert(value, typeof(int));
 
-        SqlExpression TranslateDurationTotalMember(SqlExpression instance, double divisor)
-            => _sqlExpressionFactory.Divide(DatePart("epoch", instance), _sqlExpressionFactory.Constant(divisor));
+        SqlExpression TotalMember(double divisor)
+            => sqlExpressionFactory.Divide(DatePart("epoch", interval), sqlExpressionFactory.Constant(divisor));
     }
 }

src/EFCore.PG/Query/Internal/NpgsqlQuerySqlGenerator.cs

@@ -599,6 +599,32 @@ protected virtual Expression VisitArrayIndex(SqlBinaryExpression expression)
         return expression;
     }
 
+    /// <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 Expression VisitAtTimeZone(AtTimeZoneExpression atTimeZoneExpression)
+    {
+        // Simplify: "(x AT TIME ZONE 'UTC') AT TIME ZONE 'UTC'"
+        // This is equivalent to "x", as converting to and from UTC is exact (UTC has no daylight saving time).
+        // This happens e.g. when timestamptz arithmetic is performed in UTC, and the result is then converted to UTC again.
+        if (atTimeZoneExpression is
+            {
+                TimeZone: SqlConstantExpression { Value: "UTC" },
+                Operand: AtTimeZoneExpression { TimeZone: SqlConstantExpression { Value: "UTC" }, Operand: var operand }
+            })
+        {
+            Sql.Append("(");
+            Visit(operand);
+            Sql.Append(")");
+            return atTimeZoneExpression;
+        }
+
+        return base.VisitAtTimeZone(atTimeZoneExpression);
+    }
+
     /// <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