Skip to content

Commit

Permalink
Fix Date translations (#2336)
Browse files Browse the repository at this point in the history
* Avoid time zone conversion when translating DateTime.Date with
  timestamptz.
* Return PG date from NodaTime {Local,Zoned}DateTime.Date

Fixes #2333
Fixes #2335

(cherry picked from commit da284d6)
  • Loading branch information
roji committed Apr 19, 2022
1 parent 99fe0f9 commit 84e0e03
Show file tree
Hide file tree
Showing 4 changed files with 79 additions and 19 deletions.
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
using System;
using System;
using System.Collections.Generic;
using System.Reflection;
using Microsoft.EntityFrameworkCore;
Expand All @@ -8,6 +8,7 @@
using Microsoft.EntityFrameworkCore.Storage;
using NodaTime;
using Npgsql.EntityFrameworkCore.PostgreSQL.Query;
using Npgsql.EntityFrameworkCore.PostgreSQL.Storage.Internal;

namespace Npgsql.EntityFrameworkCore.PostgreSQL.NodaTime.Query.Internal;

Expand Down Expand Up @@ -320,13 +321,11 @@ SqlExpression Upper()
getValueExpression
);

case "Date":
return _sqlExpressionFactory.Function(
"DATE_TRUNC",
new[] { _sqlExpressionFactory.Constant("day"), instance },
nullable: true,
argumentsPropagateNullability: TrueArrays[2],
returnType);
// PG allows converting a timestamp directly to date, truncating the time; but given a timestamptz, it performs a time zone
// conversion (based on TimeZone), which we don't want (so avoid translating except on timestamp).
// The translation for ZonedDateTime.Date converts to timestamp before ending up here.
case "Date" when instance.TypeMapping is TimestampLocalDateTimeMapping:
return _sqlExpressionFactory.Convert(instance, typeof(LocalDate), _typeMappingSource.FindMapping(typeof(LocalDate))!);

case "TimeOfDay":
// TODO: Technically possible simply via casting to PG time,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
using System;
using System;
using System.Reflection;
using Microsoft.EntityFrameworkCore;
using Microsoft.EntityFrameworkCore.Diagnostics;
Expand All @@ -7,6 +7,7 @@
using Microsoft.EntityFrameworkCore.Storage;
using Npgsql.EntityFrameworkCore.PostgreSQL.Storage.Internal;
using NpgsqlTypes;
using Npgsql.EntityFrameworkCore.PostgreSQL.Storage.Internal.Mapping;
using static Npgsql.EntityFrameworkCore.PostgreSQL.Utilities.Statics;

namespace Npgsql.EntityFrameworkCore.PostgreSQL.Query.ExpressionTranslators.Internal;
Expand Down Expand Up @@ -60,6 +61,39 @@ public NpgsqlDateTimeMemberTranslator(
return translated;
}

if (member.Name == nameof(DateTime.Date))
{
// Note that DateTime.Date returns a DateTime, not a DateOnly (introduced later); so we convert using date_trunc (which returns
// a PG timestamp/timestamptz) rather than a conversion to PG date (compare with NodaTime where we want a LocalDate).

// When given a timestamptz, date_trunc performs the truncation with respect to TimeZone; to avoid that, we use the overload
// accepting a time zone, and pass UTC. For regular timestamp (or in legacy timestamp mode), we use the simpler overload without
// a time zone.
if (NpgsqlTypeMappingSource.LegacyTimestampBehavior || instance?.TypeMapping is NpgsqlTimestampTypeMapping)
{
return _sqlExpressionFactory.Function(
"date_trunc",
new[] { _sqlExpressionFactory.Constant("day"), instance! },
nullable: true,
argumentsPropagateNullability: TrueArrays[2],
returnType,
instance!.TypeMapping);
}

if (instance?.TypeMapping is NpgsqlTimestampTzTypeMapping)
{
return _sqlExpressionFactory.Function(
"date_trunc",
new[] { _sqlExpressionFactory.Constant("day"), instance, _sqlExpressionFactory.Constant("UTC") },
nullable: true,
argumentsPropagateNullability: TrueArrays[3],
returnType,
instance.TypeMapping);
}

return null;
}

return member.Name switch
{
// Legacy behavior
Expand Down Expand Up @@ -95,14 +129,6 @@ public NpgsqlDateTimeMemberTranslator(
// .NET's DayOfWeek is an enum, but its int values happen to correspond to PostgreSQL
nameof(DateTime.DayOfWeek) => GetDatePartExpression(instance!, "dow", floor: true),

nameof(DateTime.Date) => _sqlExpressionFactory.Function(
"date_trunc",
new[] { _sqlExpressionFactory.Constant("day"), instance! },
nullable: true,
argumentsPropagateNullability: TrueArrays[2],
returnType,
instance!.TypeMapping),

// TODO: Technically possible simply via casting to PG time, should be better in EF Core 3.0
// but ExplicitCastExpression only allows casting to PG types that
// are default-mapped from CLR types (timespan maps to interval,
Expand Down
35 changes: 35 additions & 0 deletions test/EFCore.PG.FunctionalTests/Query/TimestampQueryTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -346,6 +346,41 @@ await AssertQuery(

#endregion Now

#region Date member

[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
[MinimumPostgresVersion(12, 0)]
public virtual async Task Where_datetime_date_on_timestamptz(bool async)
{
await AssertQuery(
async,
ss => ss.Set<Entity>().Where(e => e.TimestamptzDateTime.Date == new DateTime(1998, 4, 12, 0, 0, 0, DateTimeKind.Utc)),
entryCount: 1);

AssertSql(
@"SELECT e.""Id"", e.""TimestampDateTime"", e.""TimestampDateTimeArray"", e.""TimestampDateTimeOffset"", e.""TimestampDateTimeOffsetArray"", e.""TimestampDateTimeRange"", e.""TimestamptzDateTime"", e.""TimestamptzDateTimeArray"", e.""TimestamptzDateTimeRange""
FROM ""Entities"" AS e
WHERE date_trunc('day', e.""TimestamptzDateTime"", 'UTC') = TIMESTAMPTZ '1998-04-12 00:00:00Z'");
}

[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual async Task Where_datetime_date_on_timestamp(bool async)
{
await AssertQuery(
async,
ss => ss.Set<Entity>().Where(e => e.TimestampDateTime.Date == new DateTime(1998, 4, 12, 0, 0, 0, DateTimeKind.Local)),
entryCount: 1);

AssertSql(
@"SELECT e.""Id"", e.""TimestampDateTime"", e.""TimestampDateTimeArray"", e.""TimestampDateTimeOffset"", e.""TimestampDateTimeOffsetArray"", e.""TimestampDateTimeRange"", e.""TimestamptzDateTime"", e.""TimestamptzDateTimeArray"", e.""TimestamptzDateTimeRange""
FROM ""Entities"" AS e
WHERE date_trunc('day', e.""TimestampDateTime"") = TIMESTAMP '1998-04-12 00:00:00'");
}

#endregion

#region DateTimeOffset

[ConditionalTheory]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -286,7 +286,7 @@ await AssertQuery(
AssertSql(
@"SELECT n.""Id"", n.""DateInterval"", n.""Duration"", n.""Instant"", n.""InstantRange"", n.""Interval"", n.""LocalDate"", n.""LocalDate2"", n.""LocalDateRange"", n.""LocalDateTime"", n.""LocalTime"", n.""Long"", n.""OffsetTime"", n.""Period"", n.""TimeZoneId"", n.""ZonedDateTime""
FROM ""NodaTimeTypes"" AS n
WHERE DATE_TRUNC('day', n.""LocalDateTime"") = DATE '2018-04-20'");
WHERE n.""LocalDateTime""::date = DATE '2018-04-20'");
}

[ConditionalTheory]
Expand Down Expand Up @@ -1354,7 +1354,7 @@ await AssertQuery(
AssertSql(
@"SELECT n.""Id"", n.""DateInterval"", n.""Duration"", n.""Instant"", n.""InstantRange"", n.""Interval"", n.""LocalDate"", n.""LocalDate2"", n.""LocalDateRange"", n.""LocalDateTime"", n.""LocalTime"", n.""Long"", n.""OffsetTime"", n.""Period"", n.""TimeZoneId"", n.""ZonedDateTime""
FROM ""NodaTimeTypes"" AS n
WHERE DATE_TRUNC('day', n.""ZonedDateTime"" AT TIME ZONE 'UTC') = DATE '2018-04-20'");
WHERE CAST(n.""ZonedDateTime"" AT TIME ZONE 'UTC' AS date) = DATE '2018-04-20'");
}

[ConditionalTheory]
Expand Down

0 comments on commit 84e0e03

Please sign in to comment.