Additional work on temporal constraints

src/EFCore.PG/ChangeTracking/Internal/NpgsqlRangeCurrentValueComparer.cs

@@ -0,0 +1,112 @@
+using System.Collections;
+
+namespace Npgsql.EntityFrameworkCore.PostgreSQL.ChangeTracking.Internal;
+
+/// <summary>
+///     An <see cref="IComparer" /> for <see cref="NpgsqlRange{T}" /> values, providing an arbitrary but stable
+///     total ordering. This is needed because <see cref="NpgsqlRange{T}" /> does not implement
+///     <see cref="IComparable{T}" /> (ranges are only partially ordered), but EF Core requires an ordering for key properties
+///     in the update pipeline.
+/// </summary>
+public sealed class NpgsqlRangeCurrentValueComparer(Type rangeClrType) : IComparer
+{
+    private readonly PropertyInfo _isEmptyProperty = rangeClrType.GetProperty(nameof(NpgsqlRange<>.IsEmpty))
+        ?? throw new ArgumentException($"Type '{rangeClrType}' does not have an 'IsEmpty' property.");
+    private readonly PropertyInfo _lowerBoundProperty = rangeClrType.GetProperty(nameof(NpgsqlRange<>.LowerBound))
+        ?? throw new ArgumentException($"Type '{rangeClrType}' does not have a 'LowerBound' property.");
+    private readonly PropertyInfo _upperBoundProperty = rangeClrType.GetProperty(nameof(NpgsqlRange<>.UpperBound))
+        ?? throw new ArgumentException($"Type '{rangeClrType}' does not have an 'UpperBound' property.");
+    private readonly PropertyInfo _lowerBoundInfiniteProperty = rangeClrType.GetProperty(nameof(NpgsqlRange<>.LowerBoundInfinite))
+        ?? throw new ArgumentException($"Type '{rangeClrType}' does not have a 'LowerBoundInfinite' property.");
+    private readonly PropertyInfo _upperBoundInfiniteProperty = rangeClrType.GetProperty(nameof(NpgsqlRange<>.UpperBoundInfinite))
+        ?? throw new ArgumentException($"Type '{rangeClrType}' does not have an 'UpperBoundInfinite' property.");
+    private readonly PropertyInfo _lowerBoundIsInclusiveProperty = rangeClrType.GetProperty(nameof(NpgsqlRange<>.LowerBoundIsInclusive))
+        ?? throw new ArgumentException($"Type '{rangeClrType}' does not have a 'LowerBoundIsInclusive' property.");
+    private readonly PropertyInfo _upperBoundIsInclusiveProperty = rangeClrType.GetProperty(nameof(NpgsqlRange<>.UpperBoundIsInclusive))
+        ?? throw new ArgumentException($"Type '{rangeClrType}' does not have an 'UpperBoundIsInclusive' property.");
+
+    /// <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 int Compare(object? x, object? y)
+    {
+        if (x is null)
+        {
+            return y is null ? 0 : -1;
+        }
+
+        if (y is null)
+        {
+            return 1;
+        }
+
+        var xIsEmpty = (bool)_isEmptyProperty.GetValue(x)!;
+        var yIsEmpty = (bool)_isEmptyProperty.GetValue(y)!;
+
+        if (xIsEmpty && yIsEmpty)
+        {
+            return 0;
+        }
+
+        if (xIsEmpty)
+        {
+            return -1;
+        }
+
+        if (yIsEmpty)
+        {
+            return 1;
+        }
+
+        // Compare lower bounds
+        var cmp = CompareBound(
+            _lowerBoundProperty.GetValue(x),
+            (bool)_lowerBoundInfiniteProperty.GetValue(x)!,
+            _lowerBoundProperty.GetValue(y),
+            (bool)_lowerBoundInfiniteProperty.GetValue(y)!,
+            isLower: true);
+
+        if (cmp != 0)
+        {
+            return cmp;
+        }
+
+        // Compare lower bound inclusivity
+        cmp = ((bool)_lowerBoundIsInclusiveProperty.GetValue(x)!).CompareTo(
+            (bool)_lowerBoundIsInclusiveProperty.GetValue(y)!);
+
+        if (cmp != 0)
+        {
+            return cmp;
+        }
+
+        // Compare upper bounds
+        cmp = CompareBound(
+            _upperBoundProperty.GetValue(x),
+            (bool)_upperBoundInfiniteProperty.GetValue(x)!,
+            _upperBoundProperty.GetValue(y),
+            (bool)_upperBoundInfiniteProperty.GetValue(y)!,
+            isLower: false);
+
+        if (cmp != 0)
+        {
+            return cmp;
+        }
+
+        // Compare upper bound inclusivity
+        return ((bool)_upperBoundIsInclusiveProperty.GetValue(x)!).CompareTo(
+            (bool)_upperBoundIsInclusiveProperty.GetValue(y)!);
+    }
+
+    private static int CompareBound(object? x, bool xInfinite, object? y, bool yInfinite, bool isLower) =>
+        (xInfinite, yInfinite) switch
+        {
+            (true, true) => 0,
+            (true, false) => isLower ? -1 : 1,
+            (false, true) => isLower ? 1 : -1,
+            (false, false) => Comparer.Default.Compare(x, y),
+        };
+}

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

@@ -1,3 +1,6 @@
+using Microsoft.EntityFrameworkCore.ChangeTracking.Internal;
+using Microsoft.EntityFrameworkCore.Metadata.Internal;
+using Npgsql.EntityFrameworkCore.PostgreSQL.ChangeTracking.Internal;
 using Npgsql.EntityFrameworkCore.PostgreSQL.Infrastructure.Internal;
 using Npgsql.EntityFrameworkCore.PostgreSQL.Storage.Internal;
 using Npgsql.EntityFrameworkCore.PostgreSQL.Storage.Internal.Mapping;
@@ -28,6 +31,7 @@ public virtual void ProcessModelFinalizing(IConventionModelBuilder modelBuilder,
                 {
                     DiscoverPostgresExtensions(property, typeMapping, modelBuilder);
                     ProcessRowVersionProperty(property, typeMapping);
+                    SetRangeCurrentValueComparer(property, typeMapping);
                 }
             }
 
@@ -108,4 +112,21 @@ protected virtual void ProcessRowVersionProperty(IConventionProperty property, R
             property.Builder.HasColumnName("xmin");
         }
     }
+
+    /// <summary>
+    ///     Pre-sets the current value comparer for range key/FK properties, since <see cref="NpgsqlRange{T}" /> doesn't
+    ///     implement <see cref="IComparable" /> and the default <see cref="CurrentValueComparerFactory" /> would reject it.
+    /// </summary>
+    protected virtual void SetRangeCurrentValueComparer(IConventionProperty property, RelationalTypeMapping typeMapping)
+    {
+        if ((property.IsKey() || property.IsForeignKey())
+            && typeMapping is NpgsqlRangeTypeMapping
+            && property is PropertyBase propertyBase)
+        {
+#pragma warning disable EF1001 // Internal EF Core API usage.
+            propertyBase.SetCurrentValueComparer(
+                new EntryCurrentValueComparer((IProperty)property, new NpgsqlRangeCurrentValueComparer(property.ClrType)));
+#pragma warning restore EF1001 // Internal EF Core API usage.
+        }
+    }
 }

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

@@ -1,24 +1,20 @@
+using Microsoft.EntityFrameworkCore.ChangeTracking.Internal;
+using Npgsql.EntityFrameworkCore.PostgreSQL.ChangeTracking.Internal;
 using Npgsql.EntityFrameworkCore.PostgreSQL.Metadata.Internal;
+using Npgsql.EntityFrameworkCore.PostgreSQL.Storage.Internal.Mapping;
 
 namespace Npgsql.EntityFrameworkCore.PostgreSQL.Metadata.Conventions;
 
 /// <summary>
 ///     A convention that creates an optimized copy of the mutable model.
 /// </summary>
-public class NpgsqlRuntimeModelConvention : RelationalRuntimeModelConvention
+/// <param name="dependencies">Parameter object containing dependencies for this convention.</param>
+/// <param name="relationalDependencies">Parameter object containing relational dependencies for this convention.</param>
+public class NpgsqlRuntimeModelConvention(
+    ProviderConventionSetBuilderDependencies dependencies,
+    RelationalConventionSetBuilderDependencies relationalDependencies)
+    : RelationalRuntimeModelConvention(dependencies, relationalDependencies)
 {
-    /// <summary>
-    ///     Creates a new instance of <see cref="NpgsqlRuntimeModelConvention" />.
-    /// </summary>
-    /// <param name="dependencies">Parameter object containing dependencies for this convention.</param>
-    /// <param name="relationalDependencies">Parameter object containing relational dependencies for this convention.</param>
-    public NpgsqlRuntimeModelConvention(
-        ProviderConventionSetBuilderDependencies dependencies,
-        RelationalConventionSetBuilderDependencies relationalDependencies)
-        : base(dependencies, relationalDependencies)
-    {
-    }
-
     /// <inheritdoc />
     protected override void ProcessModelAnnotations(
         Dictionary<string, object?> annotations,
@@ -79,6 +75,18 @@ protected override void ProcessPropertyAnnotations(
     {
         base.ProcessPropertyAnnotations(annotations, property, runtimeProperty, runtime);
 
+        // NpgsqlRange<T> doesn't implement IComparable (ranges are only partially ordered), so we must
+        // provide a custom CurrentValueComparer for the runtime model. Without this, the update pipeline's
+        // ModificationCommandComparer would fail when trying to sort commands by key values.
+        if ((property.IsKey() || property.IsForeignKey())
+            && property.FindTypeMapping() is NpgsqlRangeTypeMapping)
+        {
+#pragma warning disable EF1001 // Internal EF Core API usage.
+            runtimeProperty.SetCurrentValueComparer(
+                new EntryCurrentValueComparer(runtimeProperty, new NpgsqlRangeCurrentValueComparer(property.ClrType)));
+#pragma warning restore EF1001 // Internal EF Core API usage.
+        }
+
         if (!runtime)
         {
             annotations.Remove(NpgsqlAnnotationNames.IdentityOptions);

src/EFCore.PG/Properties/NpgsqlStrings.resx

@@ -250,6 +250,9 @@
   <data name="TwoDataSourcesInSameServiceProvider" xml:space="preserve">
     <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>
   </data>
+  <data name="PeriodForeignKeyTrackingNotSupported" xml:space="preserve">
+    <value>Queries that join entities via a PERIOD foreign key (temporal constraint) cannot use change tracking. Use 'AsNoTracking()' to execute this query.</value>
+  </data>
   <data name="PeriodRequiresPostgres18" xml:space="preserve">
     <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>
   </data>

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

@@ -0,0 +1,146 @@
+using System.Diagnostics.CodeAnalysis;
+using Npgsql.EntityFrameworkCore.PostgreSQL.Internal;
+using Npgsql.EntityFrameworkCore.PostgreSQL.Query.Expressions;
+using Npgsql.EntityFrameworkCore.PostgreSQL.Query.Expressions.Internal;
+
+namespace Npgsql.EntityFrameworkCore.PostgreSQL.Query.Internal;
+
+/// <summary>
+///     A postprocessor that rewrites join predicates for PERIOD foreign keys. For PERIOD FKs, the range column join
+///     condition must use PostgreSQL range containment (<c>@&gt;</c>) rather than equality (<c>=</c>), since the principal's
+///     range must contain the dependent's range.
+/// </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 class NpgsqlPeriodForeignKeyPostprocessor(QueryTrackingBehavior queryTrackingBehavior) : ExpressionVisitor
+{
+    /// <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 VisitExtension(Expression extensionExpression)
+        => extensionExpression switch
+        {
+            ShapedQueryExpression shapedQueryExpression
+                => shapedQueryExpression.Update(
+                    Visit(shapedQueryExpression.QueryExpression),
+                    Visit(shapedQueryExpression.ShaperExpression)),
+
+            // For equality predicates between columns, check if they correspond to a PERIOD FK range column
+            // and replace with range containment (@>).
+            // Note that EF's change tracker assumes equality, but the temporal foreign key (PERIOD) uses containment;
+            // the change tracker is therefore currently incompatible with it. We check and throw, directing the user to use
+            // a no-tracking query instead.
+            SqlBinaryExpression { OperatorType: ExpressionType.Equal } eqExpression
+                when eqExpression.Left is ColumnExpression leftCol
+                    && eqExpression.Right is ColumnExpression rightCol
+                    && TryGetPeriodFkInfo(leftCol, rightCol, out var principalColumn, out var dependentColumn)
+                => queryTrackingBehavior is QueryTrackingBehavior.TrackAll
+                    ? throw new InvalidOperationException(NpgsqlStrings.PeriodForeignKeyTrackingNotSupported)
+                    : new PgBinaryExpression(
+                        PgExpressionType.Contains,
+                        principalColumn,
+                        dependentColumn,
+                        typeof(bool),
+                        eqExpression.TypeMapping),
+
+            _ => base.VisitExtension(extensionExpression)
+        };
+
+    /// <summary>
+    ///     Determines whether two columns in an equality predicate correspond to the range column of a PERIOD FK,
+    ///     and if so, identifies which is the principal column and which is the dependent column.
+    /// </summary>
+    private static bool TryGetPeriodFkInfo(
+        ColumnExpression leftCol,
+        ColumnExpression rightCol,
+        [NotNullWhen(true)] out ColumnExpression? principalColumn,
+        [NotNullWhen(true)] out ColumnExpression? dependentColumn)
+    {
+        principalColumn = null;
+        dependentColumn = null;
+
+        // We need column metadata to identify the FK
+        if (leftCol.Column is not { } leftColumnBase || rightCol.Column is not { } rightColumnBase)
+        {
+            return false;
+        }
+
+        // Check all properties mapped to the left column for PERIOD FK participation.
+        // We check both GetContainingForeignKeys() (property is on the dependent/FK side) and
+        // GetContainingKeys() -> GetReferencingForeignKeys() (property is on the principal/PK side).
+        foreach (var leftMapping in leftColumnBase.PropertyMappings)
+        {
+            var leftProperty = leftMapping.Property;
+
+            foreach (var fk in GetPeriodForeignKeys(leftProperty))
+            {
+                // The range property is the last one in the FK
+                var fkRangeProperty = fk.Properties[^1];
+                var principalRangeProperty = fk.PrincipalKey.Properties[^1];
+
+                // Determine if the left column is the dependent or principal range property,
+                // and look for the counterpart on the right column.
+                IProperty expectedRight;
+                ColumnExpression candidatePrincipal, candidateDependent;
+
+                if (leftProperty == fkRangeProperty)
+                {
+                    expectedRight = principalRangeProperty;
+                    candidatePrincipal = rightCol;
+                    candidateDependent = leftCol;
+                }
+                else if (leftProperty == principalRangeProperty)
+                {
+                    expectedRight = fkRangeProperty;
+                    candidatePrincipal = leftCol;
+                    candidateDependent = rightCol;
+                }
+                else
+                {
+                    continue;
+                }
+
+                foreach (var rightMapping in rightColumnBase.PropertyMappings)
+                {
+                    if (rightMapping.Property == expectedRight)
+                    {
+                        principalColumn = candidatePrincipal;
+                        dependentColumn = candidateDependent;
+                        return true;
+                    }
+                }
+            }
+        }
+
+        return false;
+
+        static IEnumerable<IForeignKey> GetPeriodForeignKeys(IProperty property)
+        {
+            foreach (var fk in property.GetContainingForeignKeys())
+            {
+                if (fk.GetPeriod() == true)
+                {
+                    yield return fk;
+                }
+            }
+
+            foreach (var key in property.GetContainingKeys())
+            {
+                foreach (var fk in key.GetReferencingForeignKeys())
+                {
+                    if (fk.GetPeriod() == true)
+                    {
+                        yield return fk;
+                    }
+                }
+            }
+        }
+    }
+}