npgsql / efcore.pg

Entity Framework Core provider for PostgreSQL
PostgreSQL License
1.54k stars 226 forks source link

GenerateNonNullSqlLiteral not supported on NpgsqlRowValueTypeMapping after ef core 8 upgrade #3133

Closed DizzyDeveloper closed 7 months ago

DizzyDeveloper commented 7 months ago

We are in the process of upgrading from ef core 6 to ef core 8, and consequently the efcore.pg libraries (from now on when I refer to ef core X, I am also refering to the EF Core Npgsql packages) . However, since upgrading to ef core 8 we have started encountering the following exception during our migration tests:

System.InvalidOperationException : GenerateNonNullSqlLiteral not supported on NpgsqlRowValueTypeMapping
   at Npgsql.EntityFrameworkCore.PostgreSQL.Storage.Internal.Mapping.NpgsqlRowValueTypeMapping.GenerateNonNullSqlLiteral(Object value)
   at Microsoft.EntityFrameworkCore.Storage.RelationalTypeMapping.GenerateSqlLiteral(Object value)
   at Npgsql.EntityFrameworkCore.PostgreSQL.Migrations.NpgsqlMigrationsSqlGenerator.ColumnDefinition(String schema, String table, String name, ColumnOperation operation, IModel model, MigrationCommandListBuilder builder)
   at Microsoft.EntityFrameworkCore.Migrations.MigrationsSqlGenerator.Generate(AddColumnOperation operation, IModel model, MigrationCommandListBuilder builder, Boolean terminate)
   at Npgsql.EntityFrameworkCore.PostgreSQL.Migrations.NpgsqlMigrationsSqlGenerator.Generate(AddColumnOperation operation, IModel model, MigrationCommandListBuilder builder, Boolean terminate)
   at Microsoft.EntityFrameworkCore.Migrations.MigrationsSqlGenerator.Generate(IReadOnlyList`1 operations, IModel model, MigrationsSqlGenerationOptions options)
   at Npgsql.EntityFrameworkCore.PostgreSQL.Migrations.NpgsqlMigrationsSqlGenerator.Generate(IReadOnlyList`1 operations, IModel model, MigrationsSqlGenerationOptions options)
   at Microsoft.EntityFrameworkCore.Migrations.Internal.Migrator.GenerateUpSql(Migration migration, MigrationsSqlGenerationOptions options)
   at Microsoft.EntityFrameworkCore.Migrations.Internal.Migrator.Migrate(String targetMigration)
   at Npgsql.EntityFrameworkCore.PostgreSQL.Migrations.Internal.NpgsqlMigrator.Migrate(String targetMigration)

Our migration tests where running fine on ef core 6, and as an additional test the same migration test are fine on ef core 7 as well. We have only encountered this issue since upgrading to ef core 8. After debugging the migrations a tad more I have worked out that the offending column that is leading to the above exception is:

using System.Net;
using Microsoft.EntityFrameworkCore;
using Microsoft.EntityFrameworkCore.Metadata.Builders;

public class TableA: ICreatedTrackingDateTime, ILastModifiedTrackingDateTime
{  
    private IPNetwork2 _address = null!;

        / * Rest of properties and constructors removed for brevity */

    public IPNetwork2 Address
    {
        get { return _address; }
        set
        {
            _address = Guard.AssertNotNull(value, nameof(value));
        }
    }
}

internal class TableAEntityTypeConfiguration : EntityTypeConfiguration<TableA>
{
    public override void Configure(EntityTypeBuilder<TableA> builder)
    {
        builder
            .ToTable("table-a"))
            .HasKey(e => e.Id);

        builder
            .Property(e => e.Address)
            .HasColumnType("cidr")
            .HasConversion(DatabaseContext.IpNetworkToNpgsqlInetConverter)
            .IsRequired()
            .HasDefaultValue(new IPNetwork2(new IPAddress(0), 0))
            .ValueGeneratedNever();

           / * Rest of columns definitions removed for brevity */

    }
}

/ * IPNetworkToNpgsqlInetConverter defintion - yip name misleading */
public class IPNetworkToNpgsqlInetConverter : ValueConverter<IPNetwork2, ValueTuple<IPAddress, int>>
{
    public IPNetworkToNpgsqlInetConverter()
        : base(v => ConvertToNpgsqlInetExpression(v), v => ConvertToIPNetworkExpression(v))
    { }

    public static ValueTuple<IPAddress, int> ConvertToNpgsqlInetExpression(IPNetwork2 network)
    {
        return new ValueTuple<IPAddress, int>(network.Network, network.Cidr);
    }

    // ReSharper disable once InconsistentNaming
    public static IPNetwork2 ConvertToIPNetworkExpression(ValueTuple<IPAddress, int> value)
    {
        return new IPNetwork2(value.Item1, Convert.ToByte(value.Item2));
    }
}

This error occurs on the migration that adds the 'Address` column to the table, i.e. when its creating the ALTER TABLE statement for that column.

The corresponding Migration has this snippet in it:

migrationBuilder.AddColumn<ValueTuple<IPAddress, int>>(
            name: "address",
            table: "subnet",
            type: "cidr",
            nullable: true,
            defaultValue: new System.ValueTuple<System.Net.IPAddress, int>(System.Net.IPAddress.Parse("0.0.0.0"), 0));

Ef Core 8 does not seem to like the default value.

FYI Addendum:

For the sake of trying something I manually updated the snippet above to include the defaultValueSql as the code appears to take that if the it exists. So added defaultValueSql: "'0.0.0.0/0'", so above becomes:

migrationBuilder.AddColumn<ValueTuple<IPAddress, int>>(
            name: "address",
            table: "subnet",
            type: "cidr",
            nullable: true,
            defaultValueSql: "'0.0.0.0/0'",
            defaultValue: new System.ValueTuple<System.Net.IPAddress, int>(System.Net.IPAddress.Parse("0.0.0.0"), 0));

And then the migration tests all appear to happily start run again.

roji commented 7 months ago

Am I right in assuming that the migration in question was generated before the upgrade to EF 8.0, and started failing after upgrading to 8.0?

If so, this is probably due to us removing the option to map ValueTuple<IPAddress, int> to PG cidr in Npgsql 8.0 (breaking change note). As a result, EF just sees a general value tuple, which gets mapped to a PG "row value" via NpgsqlRowValueTypeMapping - a row value in SQL is e.g. WHERE (x, y) > (1, 2). That type mapping is (currently) very restricted in what it can do - it only supports some very specific scenarios. But in any case, that's not what you want - you're looking to have a cidr in the database.

To summarize, this is a result of the same breaking change in 8.0. The best solution here is probably for you to go over your migrations and replace new System.ValueTuple<System.Net.IPAddress, int> (the old mapping type) with new NpgsqlCidr() (the new mapping type).

DizzyDeveloper commented 7 months ago

Heya @roji,

Thanks mate that was spot on. Fixed my issues :)