Fix EF Core schema mapping for IsRowVersion()/xmin (#290) and ComplexProperty/ComplexCollection ToJson (#291)

src/Weasel.Core/Migrator.cs

@@ -251,6 +251,21 @@ await WriteTemplatedFile(dropFile, (r, w) =>
     /// <param name="name"></param>
     public abstract void AssertValidIdentifier(string name);
 
+    /// <summary>
+    ///     True when <paramref name="columnName" /> refers to an engine-managed
+    ///     system / pseudo column that must never be emitted as a real column in
+    ///     generated DDL (the engine rejects <c>CREATE TABLE</c> with such a name).
+    ///     The default is that no name is reserved; provider migrators override for
+    ///     their engine's system columns.
+    ///     <para>
+    ///     Used by the EF Core integration to skip properties EF maps to a system
+    ///     column — e.g. Npgsql's <c>IsRowVersion()</c> maps a <c>uint</c>
+    ///     concurrency token to PostgreSQL's implicit <c>xmin</c> column rather than
+    ///     creating a new one (weasel#290).
+    ///     </para>
+    /// </summary>
+    public virtual bool IsSystemColumn(string columnName) => false;
+
     /// <summary>
     ///     Ensures that the database referenced by the supplied connection's connection string exists,
     ///     creating it if necessary. This is a lightweight operation that only creates the database --

src/Weasel.EntityFrameworkCore.Tests/Postgresql/ComplexJsonDbContext.cs

@@ -0,0 +1,68 @@
+#if NET10_0_OR_GREATER
+using Microsoft.EntityFrameworkCore;
+
+namespace Weasel.EntityFrameworkCore.Tests.Postgresql;
+
+/// <summary>
+///     EF Core 10 <c>ComplexProperty(...).ToJson()</c> and
+///     <c>ComplexCollection(...).ToJson()</c> mappings — value-object aggregates
+///     serialized into a single JSONB container column each. Unlike
+///     <c>OwnsOne(...).ToJson()</c>, complex properties are not navigations and have
+///     no separate entity type. Regression coverage for weasel#291.
+/// </summary>
+public class ComplexJsonDbContext: DbContext
+{
+    public const string ConnectionString =
+        "Host=localhost;Port=5432;Database=marten_testing;Username=postgres;Password=postgres";
+
+    public ComplexJsonDbContext(DbContextOptions<ComplexJsonDbContext> options): base(options)
+    {
+    }
+
+    public DbSet<Order> Orders => Set<Order>();
+    public DbSet<Invoice> Invoices => Set<Invoice>();
+
+    protected override void OnModelCreating(ModelBuilder modelBuilder)
+    {
+        modelBuilder.Entity<Order>(entity =>
+        {
+            entity.ToTable("orders", "ef_complex_test");
+            entity.HasKey(e => e.Id);
+            entity.Property(e => e.Id).HasColumnName("id");
+            entity.ComplexProperty(e => e.Shipping, c => c.ToJson("shipping"));
+        });
+
+        modelBuilder.Entity<Invoice>(entity =>
+        {
+            entity.ToTable("invoices", "ef_complex_test");
+            entity.HasKey(e => e.Id);
+            entity.Property(e => e.Id).HasColumnName("id");
+            entity.ComplexCollection(e => e.Lines, c => c.ToJson("lines"));
+        });
+    }
+}
+
+public class Order
+{
+    public Guid Id { get; set; }
+    public Address Shipping { get; set; } = new();
+}
+
+public class Address
+{
+    public string Street { get; set; } = "";
+    public string City { get; set; } = "";
+}
+
+public class Invoice
+{
+    public Guid Id { get; set; }
+    public List<InvoiceLine> Lines { get; init; } = [];
+}
+
+public class InvoiceLine
+{
+    public string Sku { get; set; } = "";
+    public decimal Amount { get; set; }
+}
+#endif

src/Weasel.EntityFrameworkCore.Tests/Postgresql/RowVersionDbContext.cs

@@ -0,0 +1,40 @@
+using Microsoft.EntityFrameworkCore;
+
+namespace Weasel.EntityFrameworkCore.Tests.Postgresql;
+
+/// <summary>
+///     Models the Npgsql-recommended optimistic-concurrency pattern: a <c>uint</c>
+///     property mapped via <c>IsRowVersion()</c>, which Npgsql maps to PostgreSQL's
+///     implicit <c>xmin</c> system column rather than creating a new column.
+///     Regression coverage for weasel#290.
+/// </summary>
+public class RowVersionDbContext: DbContext
+{
+    public const string ConnectionString =
+        "Host=localhost;Port=5432;Database=marten_testing;Username=postgres;Password=postgres";
+
+    public RowVersionDbContext(DbContextOptions<RowVersionDbContext> options): base(options)
+    {
+    }
+
+    public DbSet<VersionedRecord> Records => Set<VersionedRecord>();
+
+    protected override void OnModelCreating(ModelBuilder modelBuilder)
+    {
+        modelBuilder.Entity<VersionedRecord>(entity =>
+        {
+            entity.ToTable("versioned_records", "ef_rowversion_test");
+            entity.HasKey(e => e.Id);
+            entity.Property(e => e.Id).HasColumnName("id");
+            entity.Property(e => e.Name).HasColumnName("name").HasMaxLength(100);
+            entity.Property(e => e.Version).IsRowVersion(); // Npgsql → xmin system column
+        });
+    }
+}
+
+public class VersionedRecord
+{
+    public Guid Id { get; set; }
+    public string Name { get; set; } = "";
+    public uint Version { get; set; }
+}

src/Weasel.EntityFrameworkCore.Tests/Postgresql/complex_json_columns.cs

@@ -0,0 +1,65 @@
+#if NET10_0_OR_GREATER
+using Microsoft.EntityFrameworkCore;
+using Shouldly;
+using Weasel.Postgresql;
+using Xunit;
+
+namespace Weasel.EntityFrameworkCore.Tests.Postgresql;
+
+/// <summary>
+///     Regression coverage for weasel#291 — EF Core 10
+///     <c>ComplexProperty(...).ToJson()</c> / <c>ComplexCollection(...).ToJson()</c>
+///     columns were missing from the Weasel table mapping (only
+///     <c>OwnsOne(...).ToJson()</c> was handled). These are model-mapping tests —
+///     they build the EF model offline and exercise
+///     <see cref="DbContextExtensions.MapToTable" /> without a live database.
+/// </summary>
+public class complex_json_columns
+{
+    private static (ComplexJsonDbContext context, PostgresqlMigrator migrator) build()
+    {
+        var options = new DbContextOptionsBuilder<ComplexJsonDbContext>()
+            .UseNpgsql(ComplexJsonDbContext.ConnectionString)
+            .Options;
+        return (new ComplexJsonDbContext(options), new PostgresqlMigrator());
+    }
+
+    [Fact]
+    public void maps_complex_property_tojson_to_a_jsonb_container_column()
+    {
+        var (context, migrator) = build();
+        using var _ctx = context;
+
+        var entityType = context.Model.FindEntityType(typeof(Order));
+        entityType.ShouldNotBeNull();
+
+        var table = migrator.MapToTable(entityType);
+
+        table.HasColumn("id").ShouldBeTrue();
+        table.HasColumn("shipping")
+            .ShouldBeTrue("ComplexProperty(...).ToJson() should contribute a container column");
+
+        var pgTable = table.ShouldBeOfType<Weasel.Postgresql.Tables.Table>();
+        pgTable.ColumnFor("shipping")!.Type.ShouldBe("jsonb");
+    }
+
+    [Fact]
+    public void maps_complex_collection_tojson_to_a_jsonb_container_column()
+    {
+        var (context, migrator) = build();
+        using var _ctx = context;
+
+        var entityType = context.Model.FindEntityType(typeof(Invoice));
+        entityType.ShouldNotBeNull();
+
+        var table = migrator.MapToTable(entityType);
+
+        table.HasColumn("id").ShouldBeTrue();
+        table.HasColumn("lines")
+            .ShouldBeTrue("ComplexCollection(...).ToJson() should contribute a container column");
+
+        var pgTable = table.ShouldBeOfType<Weasel.Postgresql.Tables.Table>();
+        pgTable.ColumnFor("lines")!.Type.ShouldBe("jsonb");
+    }
+}
+#endif

src/Weasel.EntityFrameworkCore.Tests/Postgresql/row_version_system_column.cs

@@ -0,0 +1,63 @@
+using Microsoft.EntityFrameworkCore;
+using Microsoft.EntityFrameworkCore.Metadata;
+using Shouldly;
+using Weasel.Postgresql;
+using Xunit;
+
+namespace Weasel.EntityFrameworkCore.Tests.Postgresql;
+
+/// <summary>
+///     Regression coverage for weasel#290 — a property mapped via Npgsql
+///     <c>IsRowVersion()</c> resolves to PostgreSQL's implicit <c>xmin</c> system
+///     column and must NOT be emitted as a real column (PG rejects
+///     <c>CREATE TABLE</c> with "42701 column name 'xmin' conflicts with a system
+///     column name"). These are model-mapping tests — they build the EF model
+///     offline and exercise <see cref="DbContextExtensions.MapToTable" /> without a
+///     live database connection.
+/// </summary>
+public class row_version_system_column
+{
+    private static (RowVersionDbContext context, PostgresqlMigrator migrator) build()
+    {
+        var options = new DbContextOptionsBuilder<RowVersionDbContext>()
+            .UseNpgsql(RowVersionDbContext.ConnectionString)
+            .Options;
+        return (new RowVersionDbContext(options), new PostgresqlMigrator());
+    }
+
+    [Fact]
+    public void npgsql_maps_row_version_to_the_xmin_system_column()
+    {
+        // Sanity: confirm the provider still maps IsRowVersion() → xmin, so the
+        // skip below is exercising the real scenario.
+        var (context, _) = build();
+        using var _ctx = context;
+
+        var entityType = context.Model.FindEntityType(typeof(VersionedRecord));
+        entityType.ShouldNotBeNull();
+
+        var soi = StoreObjectIdentifier.Table("versioned_records", "ef_rowversion_test");
+        var versionColumn = entityType.GetProperties()
+            .Single(p => p.Name == nameof(VersionedRecord.Version))
+            .GetColumnName(soi);
+
+        versionColumn.ShouldBe("xmin");
+    }
+
+    [Fact]
+    public void xmin_is_not_emitted_as_a_table_column()
+    {
+        var (context, migrator) = build();
+        using var _ctx = context;
+
+        var entityType = context.Model.FindEntityType(typeof(VersionedRecord));
+        entityType.ShouldNotBeNull();
+
+        var table = migrator.MapToTable(entityType);
+
+        table.HasColumn("id").ShouldBeTrue();
+        table.HasColumn("name").ShouldBeTrue();
+        table.HasColumn("xmin")
+            .ShouldBeFalse("xmin is a PostgreSQL system column and must not be created as a real column");
+    }
+}

src/Weasel.EntityFrameworkCore/DbContextExtensions.cs

@@ -362,14 +362,26 @@ public static ITable MapToTable(this Migrator migrator, IEntityType entityType)
             foreach (var property in et.GetProperties())
             {
                 var columnName = property.GetColumnName(storeObjectIdentifier);
-                if (columnName != null && addedColumns.Add(columnName))
+
+                // Skip properties EF maps to an engine system column (e.g. Npgsql
+                // IsRowVersion() -> PostgreSQL's implicit "xmin"); emitting them as
+                // real columns fails with "42701 column name conflicts with a
+                // system column name" (weasel#290).
+                if (columnName != null && !migrator.IsSystemColumn(columnName) && addedColumns.Add(columnName))
                 {
                     mapColumn(property, storeObjectIdentifier, primaryKeyPropertyNames, table);
                 }
             }
 
             // Add JSON columns from owned entities mapped via OwnsOne().ToJson()
             mapJsonColumns(et, addedColumns, table);
+
+#if NET10_0_OR_GREATER
+            // Add JSON columns from complex properties / collections mapped via
+            // ComplexProperty(...).ToJson() / ComplexCollection(...).ToJson()
+            // (EF Core 10+; weasel#291).
+            mapComplexJsonColumns(et, addedColumns, table);
+#endif
         }
 
         // Set primary key constraint name from EF Core metadata.
@@ -502,6 +514,35 @@ private static void mapJsonColumns(IEntityType entityType, HashSet<string> added
         }
     }
 
+#if NET10_0_OR_GREATER
+    /// <summary>
+    ///     Map the JSON container columns produced by EF Core 10
+    ///     <c>ComplexProperty(...).ToJson()</c> / <c>ComplexCollection(...).ToJson()</c>.
+    ///     Unlike <c>OwnsOne(...).ToJson()</c> (handled in <see cref="mapJsonColumns" />
+    ///     via navigations to a JSON-mapped entity type), complex properties are not
+    ///     navigations and carry no separate entity type — their JSON container lives
+    ///     on the <see cref="IComplexType" />. Both the single
+    ///     (<c>ComplexProperty</c>) and collection (<c>ComplexCollection</c>) shapes
+    ///     serialize into a single container column, so each top-level JSON-mapped
+    ///     complex property contributes exactly one column (weasel#291).
+    /// </summary>
+    private static void mapComplexJsonColumns(IEntityType entityType, HashSet<string> addedColumns, ITable table)
+    {
+        foreach (var complexProperty in entityType.GetComplexProperties())
+        {
+            var complexType = complexProperty.ComplexType;
+            if (!complexType.IsMappedToJson()) continue;
+
+            var columnName = complexType.GetContainerColumnName();
+            if (columnName == null || !addedColumns.Add(columnName)) continue;
+
+            var columnType = complexType.GetContainerColumnType() ?? "jsonb";
+            var column = table.AddColumn(columnName, columnType);
+            column.AllowNulls = complexProperty.IsNullable;
+        }
+    }
+#endif
+
     private static CascadeAction mapDeleteBehavior(DeleteBehavior deleteBehavior)
     {
         return deleteBehavior switch

src/Weasel.Postgresql.Tests/PostgresqlMigratorSystemColumnTests.cs

@@ -0,0 +1,39 @@
+using Shouldly;
+using Weasel.Postgresql;
+using Xunit;
+
+namespace Weasel.Postgresql.Tests;
+
+/// <summary>
+///     Unit coverage for <see cref="PostgresqlMigrator.IsSystemColumn" />, the hook
+///     the EF Core integration uses to skip properties EF maps to a PostgreSQL system
+///     column (e.g. Npgsql <c>IsRowVersion()</c> → <c>xmin</c>) so they aren't emitted
+///     as real columns. weasel#290.
+/// </summary>
+public class PostgresqlMigratorSystemColumnTests
+{
+    private readonly PostgresqlMigrator theMigrator = new();
+
+    [Theory]
+    [InlineData("xmin")]
+    [InlineData("XMIN")] // case-insensitive — PG folds, EF metadata may be either case
+    [InlineData("xmax")]
+    [InlineData("cmin")]
+    [InlineData("cmax")]
+    [InlineData("ctid")]
+    [InlineData("tableoid")]
+    public void recognizes_postgres_system_columns(string columnName)
+    {
+        theMigrator.IsSystemColumn(columnName).ShouldBeTrue();
+    }
+
+    [Theory]
+    [InlineData("id")]
+    [InlineData("name")]
+    [InlineData("version")]
+    [InlineData("oid")] // oid is a valid user column on modern PG (12+), not reserved
+    public void allows_ordinary_user_columns(string columnName)
+    {
+        theMigrator.IsSystemColumn(columnName).ShouldBeFalse();
+    }
+}

src/Weasel.Postgresql/PostgresqlMigrator.cs

@@ -165,6 +165,19 @@ public static string CreateSchemaStatementFor(string schemaName)
         return $"create schema if not exists {PostgresqlProvider.Instance.ToQualifiedName(schemaName)};";
     }
 
+    /// <summary>
+    ///     PostgreSQL's implicit system columns. PostgreSQL rejects
+    ///     <c>CREATE TABLE</c> with a column named any of these
+    ///     ("42701: column name X conflicts with a system column name"), so they
+    ///     must be skipped when generating DDL. <c>oid</c> is intentionally
+    ///     excluded — it is no longer a system column on ordinary tables (PG 12+)
+    ///     and is a valid user column name.
+    /// </summary>
+    private static readonly HashSet<string> SystemColumns =
+        new(StringComparer.OrdinalIgnoreCase) { "tableoid", "xmin", "cmin", "xmax", "cmax", "ctid" };
+
+    public override bool IsSystemColumn(string columnName) => SystemColumns.Contains(columnName);
+
     public override void AssertValidIdentifier(string name)
     {
         if (string.IsNullOrWhiteSpace(name))