Skip to content

Commit 86c777b

Browse files
authored
Additional work on temporal constraints (#3746)
Fix lack of NpgsqlRange comparer Closes #3739 Part of #2097
1 parent 83627a4 commit 86c777b

File tree

10 files changed

+775
-32
lines changed

10 files changed

+775
-32
lines changed
Lines changed: 112 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,112 @@
1+
using System.Collections;
2+
3+
namespace Npgsql.EntityFrameworkCore.PostgreSQL.ChangeTracking.Internal;
4+
5+
/// <summary>
6+
/// An <see cref="IComparer" /> for <see cref="NpgsqlRange{T}" /> values, providing an arbitrary but stable
7+
/// total ordering. This is needed because <see cref="NpgsqlRange{T}" /> does not implement
8+
/// <see cref="IComparable{T}" /> (ranges are only partially ordered), but EF Core requires an ordering for key properties
9+
/// in the update pipeline.
10+
/// </summary>
11+
public sealed class NpgsqlRangeCurrentValueComparer(Type rangeClrType) : IComparer
12+
{
13+
private readonly PropertyInfo _isEmptyProperty = rangeClrType.GetProperty(nameof(NpgsqlRange<>.IsEmpty))
14+
?? throw new ArgumentException($"Type '{rangeClrType}' does not have an 'IsEmpty' property.");
15+
private readonly PropertyInfo _lowerBoundProperty = rangeClrType.GetProperty(nameof(NpgsqlRange<>.LowerBound))
16+
?? throw new ArgumentException($"Type '{rangeClrType}' does not have a 'LowerBound' property.");
17+
private readonly PropertyInfo _upperBoundProperty = rangeClrType.GetProperty(nameof(NpgsqlRange<>.UpperBound))
18+
?? throw new ArgumentException($"Type '{rangeClrType}' does not have an 'UpperBound' property.");
19+
private readonly PropertyInfo _lowerBoundInfiniteProperty = rangeClrType.GetProperty(nameof(NpgsqlRange<>.LowerBoundInfinite))
20+
?? throw new ArgumentException($"Type '{rangeClrType}' does not have a 'LowerBoundInfinite' property.");
21+
private readonly PropertyInfo _upperBoundInfiniteProperty = rangeClrType.GetProperty(nameof(NpgsqlRange<>.UpperBoundInfinite))
22+
?? throw new ArgumentException($"Type '{rangeClrType}' does not have an 'UpperBoundInfinite' property.");
23+
private readonly PropertyInfo _lowerBoundIsInclusiveProperty = rangeClrType.GetProperty(nameof(NpgsqlRange<>.LowerBoundIsInclusive))
24+
?? throw new ArgumentException($"Type '{rangeClrType}' does not have a 'LowerBoundIsInclusive' property.");
25+
private readonly PropertyInfo _upperBoundIsInclusiveProperty = rangeClrType.GetProperty(nameof(NpgsqlRange<>.UpperBoundIsInclusive))
26+
?? throw new ArgumentException($"Type '{rangeClrType}' does not have an 'UpperBoundIsInclusive' property.");
27+
28+
/// <summary>
29+
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
30+
/// the same compatibility standards as public APIs. It may be changed or removed without notice in
31+
/// any release. You should only use it directly in your code with extreme caution and knowing that
32+
/// doing so can result in application failures when updating to a new Entity Framework Core release.
33+
/// </summary>
34+
public int Compare(object? x, object? y)
35+
{
36+
if (x is null)
37+
{
38+
return y is null ? 0 : -1;
39+
}
40+
41+
if (y is null)
42+
{
43+
return 1;
44+
}
45+
46+
var xIsEmpty = (bool)_isEmptyProperty.GetValue(x)!;
47+
var yIsEmpty = (bool)_isEmptyProperty.GetValue(y)!;
48+
49+
if (xIsEmpty && yIsEmpty)
50+
{
51+
return 0;
52+
}
53+
54+
if (xIsEmpty)
55+
{
56+
return -1;
57+
}
58+
59+
if (yIsEmpty)
60+
{
61+
return 1;
62+
}
63+
64+
// Compare lower bounds
65+
var cmp = CompareBound(
66+
_lowerBoundProperty.GetValue(x),
67+
(bool)_lowerBoundInfiniteProperty.GetValue(x)!,
68+
_lowerBoundProperty.GetValue(y),
69+
(bool)_lowerBoundInfiniteProperty.GetValue(y)!,
70+
isLower: true);
71+
72+
if (cmp != 0)
73+
{
74+
return cmp;
75+
}
76+
77+
// Compare lower bound inclusivity
78+
cmp = ((bool)_lowerBoundIsInclusiveProperty.GetValue(x)!).CompareTo(
79+
(bool)_lowerBoundIsInclusiveProperty.GetValue(y)!);
80+
81+
if (cmp != 0)
82+
{
83+
return cmp;
84+
}
85+
86+
// Compare upper bounds
87+
cmp = CompareBound(
88+
_upperBoundProperty.GetValue(x),
89+
(bool)_upperBoundInfiniteProperty.GetValue(x)!,
90+
_upperBoundProperty.GetValue(y),
91+
(bool)_upperBoundInfiniteProperty.GetValue(y)!,
92+
isLower: false);
93+
94+
if (cmp != 0)
95+
{
96+
return cmp;
97+
}
98+
99+
// Compare upper bound inclusivity
100+
return ((bool)_upperBoundIsInclusiveProperty.GetValue(x)!).CompareTo(
101+
(bool)_upperBoundIsInclusiveProperty.GetValue(y)!);
102+
}
103+
104+
private static int CompareBound(object? x, bool xInfinite, object? y, bool yInfinite, bool isLower) =>
105+
(xInfinite, yInfinite) switch
106+
{
107+
(true, true) => 0,
108+
(true, false) => isLower ? -1 : 1,
109+
(false, true) => isLower ? 1 : -1,
110+
(false, false) => Comparer.Default.Compare(x, y),
111+
};
112+
}

src/EFCore.PG/Metadata/Conventions/NpgsqlPostgresModelFinalizingConvention.cs

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,6 @@
1+
using Microsoft.EntityFrameworkCore.ChangeTracking.Internal;
2+
using Microsoft.EntityFrameworkCore.Metadata.Internal;
3+
using Npgsql.EntityFrameworkCore.PostgreSQL.ChangeTracking.Internal;
14
using Npgsql.EntityFrameworkCore.PostgreSQL.Infrastructure.Internal;
25
using Npgsql.EntityFrameworkCore.PostgreSQL.Storage.Internal;
36
using Npgsql.EntityFrameworkCore.PostgreSQL.Storage.Internal.Mapping;
@@ -28,6 +31,7 @@ public virtual void ProcessModelFinalizing(IConventionModelBuilder modelBuilder,
2831
{
2932
DiscoverPostgresExtensions(property, typeMapping, modelBuilder);
3033
ProcessRowVersionProperty(property, typeMapping);
34+
SetRangeCurrentValueComparer(property, typeMapping);
3135
}
3236
}
3337

@@ -108,4 +112,21 @@ protected virtual void ProcessRowVersionProperty(IConventionProperty property, R
108112
property.Builder.HasColumnName("xmin");
109113
}
110114
}
115+
116+
/// <summary>
117+
/// Pre-sets the current value comparer for range key/FK properties, since <see cref="NpgsqlRange{T}" /> doesn't
118+
/// implement <see cref="IComparable" /> and the default <see cref="CurrentValueComparerFactory" /> would reject it.
119+
/// </summary>
120+
protected virtual void SetRangeCurrentValueComparer(IConventionProperty property, RelationalTypeMapping typeMapping)
121+
{
122+
if ((property.IsKey() || property.IsForeignKey())
123+
&& typeMapping is NpgsqlRangeTypeMapping
124+
&& property is PropertyBase propertyBase)
125+
{
126+
#pragma warning disable EF1001 // Internal EF Core API usage.
127+
propertyBase.SetCurrentValueComparer(
128+
new EntryCurrentValueComparer((IProperty)property, new NpgsqlRangeCurrentValueComparer(property.ClrType)));
129+
#pragma warning restore EF1001 // Internal EF Core API usage.
130+
}
131+
}
111132
}

src/EFCore.PG/Metadata/Conventions/NpgsqlRuntimeModelConvention.cs

Lines changed: 21 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,24 +1,20 @@
1+
using Microsoft.EntityFrameworkCore.ChangeTracking.Internal;
2+
using Npgsql.EntityFrameworkCore.PostgreSQL.ChangeTracking.Internal;
13
using Npgsql.EntityFrameworkCore.PostgreSQL.Metadata.Internal;
4+
using Npgsql.EntityFrameworkCore.PostgreSQL.Storage.Internal.Mapping;
25

36
namespace Npgsql.EntityFrameworkCore.PostgreSQL.Metadata.Conventions;
47

58
/// <summary>
69
/// A convention that creates an optimized copy of the mutable model.
710
/// </summary>
8-
public class NpgsqlRuntimeModelConvention : RelationalRuntimeModelConvention
11+
/// <param name="dependencies">Parameter object containing dependencies for this convention.</param>
12+
/// <param name="relationalDependencies">Parameter object containing relational dependencies for this convention.</param>
13+
public class NpgsqlRuntimeModelConvention(
14+
ProviderConventionSetBuilderDependencies dependencies,
15+
RelationalConventionSetBuilderDependencies relationalDependencies)
16+
: RelationalRuntimeModelConvention(dependencies, relationalDependencies)
917
{
10-
/// <summary>
11-
/// Creates a new instance of <see cref="NpgsqlRuntimeModelConvention" />.
12-
/// </summary>
13-
/// <param name="dependencies">Parameter object containing dependencies for this convention.</param>
14-
/// <param name="relationalDependencies">Parameter object containing relational dependencies for this convention.</param>
15-
public NpgsqlRuntimeModelConvention(
16-
ProviderConventionSetBuilderDependencies dependencies,
17-
RelationalConventionSetBuilderDependencies relationalDependencies)
18-
: base(dependencies, relationalDependencies)
19-
{
20-
}
21-
2218
/// <inheritdoc />
2319
protected override void ProcessModelAnnotations(
2420
Dictionary<string, object?> annotations,
@@ -79,6 +75,18 @@ protected override void ProcessPropertyAnnotations(
7975
{
8076
base.ProcessPropertyAnnotations(annotations, property, runtimeProperty, runtime);
8177

78+
// NpgsqlRange<T> doesn't implement IComparable (ranges are only partially ordered), so we must
79+
// provide a custom CurrentValueComparer for the runtime model. Without this, the update pipeline's
80+
// ModificationCommandComparer would fail when trying to sort commands by key values.
81+
if ((property.IsKey() || property.IsForeignKey())
82+
&& property.FindTypeMapping() is NpgsqlRangeTypeMapping)
83+
{
84+
#pragma warning disable EF1001 // Internal EF Core API usage.
85+
runtimeProperty.SetCurrentValueComparer(
86+
new EntryCurrentValueComparer(runtimeProperty, new NpgsqlRangeCurrentValueComparer(property.ClrType)));
87+
#pragma warning restore EF1001 // Internal EF Core API usage.
88+
}
89+
8290
if (!runtime)
8391
{
8492
annotations.Remove(NpgsqlAnnotationNames.IdentityOptions);

src/EFCore.PG/Properties/NpgsqlStrings.Designer.cs

Lines changed: 6 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

src/EFCore.PG/Properties/NpgsqlStrings.resx

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -250,6 +250,9 @@
250250
<data name="TwoDataSourcesInSameServiceProvider" xml:space="preserve">
251251
<value>Using two distinct data sources within a service provider is not supported, and Entity Framework is not building its own internal service provider. Either allow Entity Framework to build the service provider by removing the call to '{useInternalServiceProvider}', or ensure that the same data source is used for all uses of a given service provider passed to '{useInternalServiceProvider}'.</value>
252252
</data>
253+
<data name="PeriodForeignKeyTrackingNotSupported" xml:space="preserve">
254+
<value>Queries that join entities via a PERIOD foreign key (temporal constraint) cannot use change tracking. Use 'AsNoTracking()' to execute this query.</value>
255+
</data>
253256
<data name="PeriodRequiresPostgres18" xml:space="preserve">
254257
<value>PERIOD on foreign key '{foreignKeyName}' in entity type '{entityType}' requires PostgreSQL 18.0 or later. If you're targeting an older version, remove the `WithPeriod()` configuration call. Otherwise, set PostgreSQL compatibility mode by calling 'optionsBuilder.UseNpgsql(..., o =&gt; o.SetPostgresVersion(18, 0))' in your model's OnConfiguring.</value>
255258
</data>
Lines changed: 146 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,146 @@
1+
using System.Diagnostics.CodeAnalysis;
2+
using Npgsql.EntityFrameworkCore.PostgreSQL.Internal;
3+
using Npgsql.EntityFrameworkCore.PostgreSQL.Query.Expressions;
4+
using Npgsql.EntityFrameworkCore.PostgreSQL.Query.Expressions.Internal;
5+
6+
namespace Npgsql.EntityFrameworkCore.PostgreSQL.Query.Internal;
7+
8+
/// <summary>
9+
/// A postprocessor that rewrites join predicates for PERIOD foreign keys. For PERIOD FKs, the range column join
10+
/// condition must use PostgreSQL range containment (<c>@&gt;</c>) rather than equality (<c>=</c>), since the principal's
11+
/// range must contain the dependent's range.
12+
/// </summary>
13+
/// <remarks>
14+
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
15+
/// the same compatibility standards as public APIs. It may be changed or removed without notice in
16+
/// any release. You should only use it directly in your code with extreme caution and knowing that
17+
/// doing so can result in application failures when updating to a new Entity Framework Core release.
18+
/// </remarks>
19+
public class NpgsqlPeriodForeignKeyPostprocessor(QueryTrackingBehavior queryTrackingBehavior) : ExpressionVisitor
20+
{
21+
/// <summary>
22+
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
23+
/// the same compatibility standards as public APIs. It may be changed or removed without notice in
24+
/// any release. You should only use it directly in your code with extreme caution and knowing that
25+
/// doing so can result in application failures when updating to a new Entity Framework Core release.
26+
/// </summary>
27+
protected override Expression VisitExtension(Expression extensionExpression)
28+
=> extensionExpression switch
29+
{
30+
ShapedQueryExpression shapedQueryExpression
31+
=> shapedQueryExpression.Update(
32+
Visit(shapedQueryExpression.QueryExpression),
33+
Visit(shapedQueryExpression.ShaperExpression)),
34+
35+
// For equality predicates between columns, check if they correspond to a PERIOD FK range column
36+
// and replace with range containment (@>).
37+
// Note that EF's change tracker assumes equality, but the temporal foreign key (PERIOD) uses containment;
38+
// the change tracker is therefore currently incompatible with it. We check and throw, directing the user to use
39+
// a no-tracking query instead.
40+
SqlBinaryExpression { OperatorType: ExpressionType.Equal } eqExpression
41+
when eqExpression.Left is ColumnExpression leftCol
42+
&& eqExpression.Right is ColumnExpression rightCol
43+
&& TryGetPeriodFkInfo(leftCol, rightCol, out var principalColumn, out var dependentColumn)
44+
=> queryTrackingBehavior is QueryTrackingBehavior.TrackAll
45+
? throw new InvalidOperationException(NpgsqlStrings.PeriodForeignKeyTrackingNotSupported)
46+
: new PgBinaryExpression(
47+
PgExpressionType.Contains,
48+
principalColumn,
49+
dependentColumn,
50+
typeof(bool),
51+
eqExpression.TypeMapping),
52+
53+
_ => base.VisitExtension(extensionExpression)
54+
};
55+
56+
/// <summary>
57+
/// Determines whether two columns in an equality predicate correspond to the range column of a PERIOD FK,
58+
/// and if so, identifies which is the principal column and which is the dependent column.
59+
/// </summary>
60+
private static bool TryGetPeriodFkInfo(
61+
ColumnExpression leftCol,
62+
ColumnExpression rightCol,
63+
[NotNullWhen(true)] out ColumnExpression? principalColumn,
64+
[NotNullWhen(true)] out ColumnExpression? dependentColumn)
65+
{
66+
principalColumn = null;
67+
dependentColumn = null;
68+
69+
// We need column metadata to identify the FK
70+
if (leftCol.Column is not { } leftColumnBase || rightCol.Column is not { } rightColumnBase)
71+
{
72+
return false;
73+
}
74+
75+
// Check all properties mapped to the left column for PERIOD FK participation.
76+
// We check both GetContainingForeignKeys() (property is on the dependent/FK side) and
77+
// GetContainingKeys() -> GetReferencingForeignKeys() (property is on the principal/PK side).
78+
foreach (var leftMapping in leftColumnBase.PropertyMappings)
79+
{
80+
var leftProperty = leftMapping.Property;
81+
82+
foreach (var fk in GetPeriodForeignKeys(leftProperty))
83+
{
84+
// The range property is the last one in the FK
85+
var fkRangeProperty = fk.Properties[^1];
86+
var principalRangeProperty = fk.PrincipalKey.Properties[^1];
87+
88+
// Determine if the left column is the dependent or principal range property,
89+
// and look for the counterpart on the right column.
90+
IProperty expectedRight;
91+
ColumnExpression candidatePrincipal, candidateDependent;
92+
93+
if (leftProperty == fkRangeProperty)
94+
{
95+
expectedRight = principalRangeProperty;
96+
candidatePrincipal = rightCol;
97+
candidateDependent = leftCol;
98+
}
99+
else if (leftProperty == principalRangeProperty)
100+
{
101+
expectedRight = fkRangeProperty;
102+
candidatePrincipal = leftCol;
103+
candidateDependent = rightCol;
104+
}
105+
else
106+
{
107+
continue;
108+
}
109+
110+
foreach (var rightMapping in rightColumnBase.PropertyMappings)
111+
{
112+
if (rightMapping.Property == expectedRight)
113+
{
114+
principalColumn = candidatePrincipal;
115+
dependentColumn = candidateDependent;
116+
return true;
117+
}
118+
}
119+
}
120+
}
121+
122+
return false;
123+
124+
static IEnumerable<IForeignKey> GetPeriodForeignKeys(IProperty property)
125+
{
126+
foreach (var fk in property.GetContainingForeignKeys())
127+
{
128+
if (fk.GetPeriod() == true)
129+
{
130+
yield return fk;
131+
}
132+
}
133+
134+
foreach (var key in property.GetContainingKeys())
135+
{
136+
foreach (var fk in key.GetReferencingForeignKeys())
137+
{
138+
if (fk.GetPeriod() == true)
139+
{
140+
yield return fk;
141+
}
142+
}
143+
}
144+
}
145+
}
146+
}

0 commit comments

Comments
 (0)