Skip to content

Commit

Permalink
Translations around DateOnly.DayNumber (#3333)
Browse files Browse the repository at this point in the history
Closes #3194
  • Loading branch information
roji authored Oct 28, 2024
1 parent 02190c1 commit c484080
Show file tree
Hide file tree
Showing 5 changed files with 125 additions and 19 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,11 @@ public NpgsqlDateTimeMemberTranslator(IRelationalTypeMappingSource typeMappingSo
return translated;
}

if (declaringType == typeof(DateOnly) && TranslateDateOnly(instance, member) is { } translated2)
{
return translated2;
}

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
Expand Down Expand Up @@ -190,13 +195,7 @@ SqlExpression LocalNow()
return _sqlExpressionFactory.Convert(result, typeof(int));
}

/// <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>
public virtual SqlExpression? TranslateDateTimeOffset(SqlExpression instance, MemberInfo member)
private SqlExpression? TranslateDateTimeOffset(SqlExpression instance, MemberInfo member)
=> member.Name switch
{
// We only support UTC DateTimeOffset, so DateTimeOffset.DateTime is just a matter of converting to timestamp without time zone
Expand Down Expand Up @@ -226,6 +225,17 @@ SqlExpression LocalNow()
_ => null
};

private SqlExpression? TranslateDateOnly(SqlExpression? instance, MemberInfo member)
=> member.Name switch
{
// We use fragment rather than a DateOnly constant, since 0001-01-01 gets rendered as -infinity by default.
// TODO: Set the right type/type mapping after https://github.com/dotnet/efcore/pull/34995 is merged
nameof(DateOnly.DayNumber) when instance is not null
=> _sqlExpressionFactory.Subtract(instance, _sqlExpressionFactory.Fragment("DATE '0001-01-01'")),

_ => null
};

// Various conversion functions translated here (date_part, ::time) exist only for timestamp without time zone, so if we pass in a
// timestamptz it gets implicitly converted to a local timestamp based on TimeZone; that's the wrong behavior (these conversions are not
// supposed to be sensitive to TimeZone).
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,10 @@ private static readonly MethodInfo DateOnly_AddMonths
private static readonly MethodInfo DateOnly_AddYears
= typeof(DateOnly).GetRuntimeMethod(nameof(DateOnly.AddYears), [typeof(int)])!;

private static readonly MethodInfo DateOnly_FromDayNumber
= typeof(DateOnly).GetRuntimeMethod(
nameof(DateOnly.FromDayNumber), [typeof(int)])!;

private static readonly MethodInfo TimeOnly_FromDateTime
= typeof(TimeOnly).GetRuntimeMethod(nameof(TimeOnly.FromDateTime), [typeof(DateTime)])!;

Expand Down Expand Up @@ -226,6 +230,18 @@ public NpgsqlDateTimeMethodTranslator(
{
return _sqlExpressionFactory.MakePostgresBinary(PgExpressionType.Distance, arguments[1], arguments[2]);
}

if (method == DateOnly_FromDayNumber)
{
// We use fragment rather than a DateOnly constant, since 0001-01-01 gets rendered as -infinity by default.
// TODO: Set the right type/type mapping after https://github.com/dotnet/efcore/pull/34995 is merged
return new SqlBinaryExpression(
ExpressionType.Add,
_sqlExpressionFactory.Fragment("DATE '0001-01-01'"),
arguments[0],
typeof(DateOnly),
_typeMappingSource.FindMapping(typeof(DateOnly)));
}
}
else
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -218,21 +218,39 @@ when binaryExpression.Left.Type.UnwrapNullableType().FullName == "NodaTime.Local

var translation = base.VisitBinary(binaryExpression);

// A somewhat hacky workaround for #2942.
// When an optional owned JSON entity is compared to null, we get WHERE (x -> y) IS NULL.
// The -> operator (returning jsonb) is used rather than ->> (returning text), since an entity type is being extracted, and further
// JSON operations may need to be composed. However, when the value extracted is a JSON null, a non-NULL jsonb value is returned,
// and comparing that to relational NULL returns false.
// Pattern-match this and force the use of ->> by changing the mapping to be a scalar rather than an entity type.
if (translation is SqlUnaryExpression
switch (translation)
{
// Optimize (x - c) - (y - c) to x - y.
// This is particularly useful for DateOnly.DayNumber - DateOnly.DayNumber, which is the way to express DateOnly subtraction
// (the subtraction operator isn't defined over DateOnly in .NET). The translation of x.DayNumber is x - DATE '0001-01-01',
// so the below is a useful simplification.
// TODO: As this is a generic mathematical simplification, we should move it to a generic optimization phase in EF Core.
case SqlBinaryExpression
{
OperatorType: ExpressionType.Subtract,
Left: SqlBinaryExpression { OperatorType: ExpressionType.Subtract, Left: var left1, Right: var right1 },
Right: SqlBinaryExpression { OperatorType: ExpressionType.Subtract, Left: var left2, Right: var right2 }
} originalBinary when right1.Equals(right2):
{
return new SqlBinaryExpression(ExpressionType.Subtract, left1, left2, originalBinary.Type, originalBinary.TypeMapping);
}

// A somewhat hacky workaround for #2942.
// When an optional owned JSON entity is compared to null, we get WHERE (x -> y) IS NULL.
// The -> operator (returning jsonb) is used rather than ->> (returning text), since an entity type is being extracted, and
// further JSON operations may need to be composed. However, when the value extracted is a JSON null, a non-NULL jsonb value is
// returned, and comparing that to relational NULL returns false.
// Pattern-match this and force the use of ->> by changing the mapping to be a scalar rather than an entity type.
case SqlUnaryExpression
{
OperatorType: ExpressionType.Equal or ExpressionType.NotEqual,
Operand: JsonScalarExpression { TypeMapping: NpgsqlOwnedJsonTypeMapping } operand
} unary)
{
return unary.Update(
new JsonScalarExpression(
operand.Json, operand.Path, operand.Type, _typeMappingSource.FindMapping("text"), operand.IsNullable));
} unary:
{
return unary.Update(
new JsonScalarExpression(
operand.Json, operand.Path, operand.Type, _typeMappingSource.FindMapping("text"), operand.IsNullable));
}
}

return translation;
Expand Down
12 changes: 12 additions & 0 deletions src/EFCore.PG/Query/NpgsqlSqlExpressionFactory.cs
Original file line number Diff line number Diff line change
Expand Up @@ -476,6 +476,18 @@ private SqlBinaryExpression ApplyTypeMappingOnSqlBinary(SqlBinaryExpression bina
binary.Type,
typeMapping ?? _typeMappingSource.FindMapping(binary.Type, "interval"));
}

// TODO: This is a hack until https://github.com/dotnet/efcore/pull/34995 is done; the translation of DateOnly.DayNumber
// generates a substraction with a fragment, but for now we can't assign a type/type mapping to a fragment.
case ExpressionType.Subtract when left.Type == typeof(DateOnly) && right is SqlFragmentExpression:
{
return new SqlBinaryExpression(
ExpressionType.Subtract,
ApplyDefaultTypeMapping(left),
right,
typeof(int),
_typeMappingSource.FindMapping(typeof(int)));
}
}

// If this is a row value comparison (e.g. (a, b) > (5, 6)), doing type mapping inference on each corresponding pair.
Expand Down
50 changes: 50 additions & 0 deletions test/EFCore.PG.FunctionalTests/Query/TimestampQueryTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -744,6 +744,56 @@ WHERE CAST(e."TimestamptzDateTime" AT TIME ZONE 'UTC' AS date) + TIME '15:26:38'
""");
}

[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual async Task DateOnly_DayNumber(bool async)
{
await AssertQuery(
async,
ss => ss.Set<Entity>().Where(e => DateOnly.FromDateTime(e.TimestamptzDateTime).DayNumber == 729490));

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 CAST(e."TimestamptzDateTime" AT TIME ZONE 'UTC' AS date) - DATE '0001-01-01' = 729490
""");
}

[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual async Task DateOnly_DayNumber_subtraction(bool async)
{
await AssertQuery(
async,
ss => ss.Set<Entity>().Where(
e => DateOnly.FromDateTime(e.TimestamptzDateTime).DayNumber -
DateOnly.FromDateTime(e.TimestamptzDateTime - TimeSpan.FromDays(3)).DayNumber == 3));

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 CAST(e."TimestamptzDateTime" AT TIME ZONE 'UTC' AS date) - CAST((e."TimestamptzDateTime" - INTERVAL '3 00:00:00') AT TIME ZONE 'UTC' AS date) = 3
""");
}

[ConditionalTheory]
[MemberData(nameof(IsAsyncData))]
public virtual async Task DateOnly_FromDayNumber(bool async)
{
await AssertQuery(
async,
ss => ss.Set<Entity>().Where(e => DateOnly.FromDayNumber(e.Id) == new DateOnly(0001, 01, 03)));

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 '0001-01-01' + e."Id" = DATE '0001-01-03'
""");
}

#endregion DateOnly

#region TimeOnly
Expand Down

0 comments on commit c484080

Please sign in to comment.