Skip to content

Commit

Permalink
Workaround for comparing owned JSON entity type to null
Browse files Browse the repository at this point in the history
Fixes #2942
  • Loading branch information
roji committed Nov 15, 2023
1 parent b30700b commit 0156462
Show file tree
Hide file tree
Showing 2 changed files with 37 additions and 18 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -177,10 +177,11 @@ protected override Expression VisitNewArray(NewArrayExpression newArrayExpressio
/// </summary>
protected override Expression VisitBinary(BinaryExpression binaryExpression)
{
if (binaryExpression.NodeType == ExpressionType.Subtract)
switch (binaryExpression.NodeType)
{
if (binaryExpression.Left.Type.UnwrapNullableType().FullName == "NodaTime.LocalDate"
&& binaryExpression.Right.Type.UnwrapNullableType().FullName == "NodaTime.LocalDate")
case ExpressionType.Subtract
when binaryExpression.Left.Type.UnwrapNullableType().FullName == "NodaTime.LocalDate"
&& binaryExpression.Right.Type.UnwrapNullableType().FullName == "NodaTime.LocalDate":
{
if (TranslationFailed(binaryExpression.Left, Visit(TryRemoveImplicitConvert(binaryExpression.Left)), out var sqlLeft)
|| TranslationFailed(binaryExpression.Right, Visit(TryRemoveImplicitConvert(binaryExpression.Right)), out var sqlRight))
Expand All @@ -200,20 +201,40 @@ protected override Expression VisitBinary(BinaryExpression binaryExpression)
builtIn: true,
_nodaTimePeriodType ??= binaryExpression.Left.Type.Assembly.GetType("NodaTime.Period")!,
typeMapping: null);

// Note: many other date/time arithmetic operators are fully supported as-is by PostgreSQL - see NpgsqlSqlExpressionFactory
}

// Note: many other date/time arithmetic operators are fully supported as-is by PostgreSQL - see NpgsqlSqlExpressionFactory
case ExpressionType.ArrayIndex:
{
// During preprocessing, ArrayIndex and List[] get normalized to ElementAt; see NpgsqlArrayTranslator
Check.DebugFail(
"During preprocessing, ArrayIndex and List[] get normalized to ElementAt; see NpgsqlArrayTranslator. "
+ "Should never see ArrayIndex.");
break;
}
}

if (binaryExpression.NodeType == ExpressionType.ArrayIndex)
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
{
OperatorType: ExpressionType.Equal or ExpressionType.NotEqual,
Operand: JsonScalarExpression { TypeMapping: NpgsqlOwnedJsonTypeMapping } operand
} unary)
{
// During preprocessing, ArrayIndex and List[] get normalized to ElementAt; see NpgsqlArrayTranslator
Check.DebugFail(
"During preprocessing, ArrayIndex and List[] get normalized to ElementAt; see NpgsqlArrayTranslator. "
+ "Should never see ArrayIndex.");
return unary.Update(
new JsonScalarExpression(
operand.Json, operand.Path, operand.Type, _typeMappingSource.FindMapping("text"), operand.IsNullable));
}

return base.VisitBinary(binaryExpression);
return translation;
}

/// <summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -85,27 +85,25 @@ public override async Task Filter_optional_dependent_with_some_required_compared

public override async Task Filter_nested_optional_dependent_with_all_optional_compared_to_null(bool async)
{
// #2942
await Assert.ThrowsAsync<EqualException>(() => base.Filter_nested_optional_dependent_with_all_optional_compared_to_not_null(async));
await base.Filter_nested_optional_dependent_with_all_optional_compared_to_null(async);

AssertSql(
"""
SELECT e."Id", e."Name", e."Json"
FROM "EntitiesAllOptional" AS e
WHERE (e."Json" -> 'OpNav2') IS NOT NULL
WHERE (e."Json" ->> 'OpNav1') IS NULL
""");
}

public override async Task Filter_nested_optional_dependent_with_all_optional_compared_to_not_null(bool async)
{
// #2942
await Assert.ThrowsAsync<EqualException>(() => base.Filter_nested_optional_dependent_with_all_optional_compared_to_not_null(async));
await base.Filter_nested_optional_dependent_with_all_optional_compared_to_not_null(async);

AssertSql(
"""
SELECT e."Id", e."Name", e."Json"
FROM "EntitiesAllOptional" AS e
WHERE (e."Json" -> 'OpNav2') IS NOT NULL
WHERE (e."Json" ->> 'OpNav2') IS NOT NULL
""");
}

Expand All @@ -117,7 +115,7 @@ public override async Task Filter_nested_optional_dependent_with_some_required_c
"""
SELECT e."Id", e."Name", e."Json"
FROM "EntitiesSomeRequired" AS e
WHERE (e."Json" -> 'ReqNav1') IS NULL
WHERE (e."Json" ->> 'ReqNav1') IS NULL
""");
}

Expand All @@ -129,7 +127,7 @@ public override async Task Filter_nested_optional_dependent_with_some_required_c
"""
SELECT e."Id", e."Name", e."Json"
FROM "EntitiesSomeRequired" AS e
WHERE (e."Json" -> 'ReqNav2') IS NOT NULL
WHERE (e."Json" ->> 'ReqNav2') IS NOT NULL
""");
}

Expand Down

0 comments on commit 0156462

Please sign in to comment.