npgsql / efcore.pg

Entity Framework Core provider for PostgreSQL
PostgreSQL License
1.49k stars 215 forks source link

NpgsqlArrayConverter.ArrayConversionExpression throws IndexOutOfRangeException when TInput is string #3074

Open Rudomitori opened 5 months ago

Rudomitori commented 5 months ago

Npgsql.EntityFrameworkCore.PostgreSQL: 8.0.0 Npgsql: 8.0.1

System.IndexOutOfRangeException
Index was outside the bounds of the array.
   at Npgsql.EntityFrameworkCore.PostgreSQL.Storage.ValueConversion.NpgsqlArrayConverter`3.ArrayConversionExpression[TInput,TOutput,TConcreteOutput](LambdaExpression elementConversionExpression)
   at Npgsql.EntityFrameworkCore.PostgreSQL.Storage.ValueConversion.NpgsqlArrayConverter`3..ctor(ValueConverter elementConverter)
   at System.RuntimeMethodHandle.InvokeMethod(Object target, Void** arguments, Signature sig, Boolean isConstructor)
   at System.Reflection.MethodBaseInvoker.InvokeDirectByRefWithFewArgs(Object obj, Span`1 copyOfArgs, BindingFlags invokeAttr)

The problem is in the line https://github.com/npgsql/efcore.pg/blob/35f0fd5a75771382b97a3ee49825fdfdae932907/src/EFCore.PG/Storage/ValueConversion/NpgsqlArrayConverter.cs#L142

I think, inputElementType variable must be used here instead of typeof(TInput).GetGenericArguments()[0]

roji commented 5 months ago

TInput is supposed to be an array/list type - it isn't the element type contained within the array/list; and handling string as an array is not a scenario that's supported.

Can you submit a code sample showing what you're trying to accomplish? If you're trying to map string as an array, can you provide some context on why you're doing that?

Rudomitori commented 5 months ago

After short debug session I found that the exception is throwed when one of the old migrations is applyed.

In the migration I have:

migrationBuilder.AddColumn<List<int>>(
    name: "RepeatableSurveyRuleIds",
    table: "Patients",
    type: "integer[]",
    nullable: false,
    defaultValue: "{}"
);

It works fine with

Npgsql.EntityFrameworkCore.PostgreSQL: 7.0.4
Npgsql: 7.0.4
Microsoft.EntityFrameworkCore: 7.0.5

and produces the correct SQL:

ALTER TABLE "Patients" ADD "RepeatableSurveyRuleIds" integer[] NOT NULL DEFAULT '{}';

But new versions of the packages introduced some breaking change and the exception.

In my case, the problem can be easely fixed via replacing defaultValue: "{}" with defaultValueSql: "'{}'::integer[]", but it will be more helpful, if the exception will be meaningful or, at least, won't have a cut stack trace

roji commented 4 months ago

@Rudomitori I can't reproduce this; in addition, the original exception stack trace you've posted is from NpgsqlArrayConverter, which has nothing to do with the default value specified in the migrations - the two aren't connected.

Can you please try to put together a clear set of steps and/or a a minimal, runnable project that can show the error happening?

Rudomitori commented 4 months ago

Here is the reproduction: https://github.com/Rudomitori/npgsql_3074

If you checkout to the dotnet_7 branch and launch the app, it works as expected. Or if you try to execute the dotnet ef migrations script command on this branch you get a valid SQL script.

But if you try to do these on the dotnet_8 branch, you get the following exception:

System.Reflection.TargetInvocationException: Exception has been thrown by the target of an invocation.
 ---> System.Reflection.TargetInvocationException: Exception has been thrown by the target of an invocation.
 ---> System.IndexOutOfRangeException: Index was outside the bounds of the array.
   at Npgsql.EntityFrameworkCore.PostgreSQL.Storage.ValueConversion.NpgsqlArrayConverter`3.ArrayConversionExpression[TInput,TOutput,TConcreteOutput](LambdaExpression elementConversionExpression)
   at Npgsql.EntityFrameworkCore.PostgreSQL.Storage.ValueConversion.NpgsqlArrayConverter`3..ctor(ValueConverter elementConverter)
   at System.RuntimeMethodHandle.InvokeMethod(Object target, Void** arguments, Signature sig, Boolean isConstructor)
   at System.Reflection.MethodBaseInvoker.InvokeDirectByRefWithFewArgs(Object obj, Span`1 copyOfArgs, BindingFlags invokeAttr)
   --- End of inner exception stack trace ---
   at System.Reflection.MethodBaseInvoker.InvokeDirectByRefWithFewArgs(Object obj, Span`1 copyOfArgs, BindingFlags invokeAttr)
   at System.Reflection.MethodBaseInvoker.InvokeWithOneArg(Object obj, BindingFlags invokeAttr, Binder binder, Object[] parameters, CultureInfo culture)
   at System.Reflection.RuntimeConstructorInfo.Invoke(BindingFlags invokeAttr, Binder binder, Object[] parameters, CultureInfo culture)
   at System.RuntimeType.CreateInstanceImpl(BindingFlags bindingAttr, Binder binder, Object[] args, CultureInfo culture)
   at System.Activator.CreateInstance(Type type, Object[] args)
   at Npgsql.EntityFrameworkCore.PostgreSQL.Storage.Internal.Mapping.NpgsqlArrayTypeMapping`3.CreateParameters(String storeType, RelationalTypeMapping elementMapping)
   at Npgsql.EntityFrameworkCore.PostgreSQL.Storage.Internal.Mapping.NpgsqlArrayTypeMapping`3..ctor(String storeType, RelationalTypeMapping elementTypeMapping)
   at System.RuntimeMethodHandle.InvokeMethod(Object target, Void** arguments, Signature sig, Boolean isConstructor)
   at System.Reflection.MethodBaseInvoker.InvokeDirectByRefWithFewArgs(Object obj, Span`1 copyOfArgs, BindingFlags invokeAttr)
   --- End of inner exception stack trace ---
   at System.Reflection.MethodBaseInvoker.InvokeDirectByRefWithFewArgs(Object obj, Span`1 copyOfArgs, BindingFlags invokeAttr)
   at System.Reflection.MethodBaseInvoker.InvokeWithFewArgs(Object obj, BindingFlags invokeAttr, Binder binder, Object[] parameters, CultureInfo culture)
   at System.Reflection.RuntimeConstructorInfo.Invoke(BindingFlags invokeAttr, Binder binder, Object[] parameters, CultureInfo culture)
   at System.RuntimeType.CreateInstanceImpl(BindingFlags bindingAttr, Binder binder, Object[] args, CultureInfo culture)
   at System.Activator.CreateInstance(Type type, Object[] args)
   at Npgsql.EntityFrameworkCore.PostgreSQL.Storage.Internal.NpgsqlTypeMappingSource.FindCollectionMapping(RelationalTypeMappingInfo info, Type modelType, Type providerType, CoreTypeMapping elementMapping)
   at Npgsql.EntityFrameworkCore.PostgreSQL.Storage.Internal.NpgsqlTypeMappingSource.FindBaseMapping(RelationalTypeMappingInfo& mappingInfo)
   at Npgsql.EntityFrameworkCore.PostgreSQL.Storage.Internal.NpgsqlTypeMappingSource.FindMapping(RelationalTypeMappingInfo& mappingInfo)
   at Microsoft.EntityFrameworkCore.Storage.RelationalTypeMappingSource.<>c.<FindMappingWithConversion>b__8_0(ValueTuple`4 k, RelationalTypeMappingSource self)
   at System.Collections.Concurrent.ConcurrentDictionary`2.GetOrAdd[TArg](TKey key, Func`3 valueFactory, TArg factoryArgument)
   at Microsoft.EntityFrameworkCore.Storage.RelationalTypeMappingSource.FindMappingWithConversion(RelationalTypeMappingInfo mappingInfo, Type providerClrType, ValueConverter customConverter)
   at Microsoft.EntityFrameworkCore.Storage.RelationalTypeMappingSource.FindMappingWithConversion(RelationalTypeMappingInfo mappingInfo, IReadOnlyList`1 principals)
   at Microsoft.EntityFrameworkCore.Storage.RelationalTypeMappingSource.FindMapping(Type type, String storeTypeName, Boolean keyOrIndex, Nullable`1 unicode, Nullable`1 size, Nullable`1 rowVersion, Nullable`1 fixedLength, Nullable`1 precision, Nullable`1 scale)
   at Microsoft.EntityFrameworkCore.Migrations.MigrationsSqlGenerator.DefaultValue(Object defaultValue, String defaultValueSql, String columnType, MigrationCommandListBuilder builder)
   at Npgsql.EntityFrameworkCore.PostgreSQL.Migrations.NpgsqlMigrationsSqlGenerator.DefaultValue(Object defaultValue, String defaultValueSql, String columnType, MigrationCommandListBuilder builder)
   at Npgsql.EntityFrameworkCore.PostgreSQL.Migrations.NpgsqlMigrationsSqlGenerator.ColumnDefinition(String schema, String table, String name, ColumnOperation operation, IModel model, MigrationCommandListBuilder builder)
   at Microsoft.EntityFrameworkCore.Migrations.MigrationsSqlGenerator.ColumnDefinition(AddColumnOperation 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.<>c.<.cctor>b__82_0(MigrationsSqlGenerator g, MigrationOperation o, IModel m, MigrationCommandListBuilder b)
   at Microsoft.EntityFrameworkCore.Migrations.MigrationsSqlGenerator.Generate(MigrationOperation operation, IModel model, MigrationCommandListBuilder builder)
   at Npgsql.EntityFrameworkCore.PostgreSQL.Migrations.NpgsqlMigrationsSqlGenerator.Generate(MigrationOperation operation, IModel model, MigrationCommandListBuilder builder)
   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.<>c__DisplayClass16_2.<GetMigrationCommandLists>b__2()
   at Microsoft.EntityFrameworkCore.Migrations.Internal.Migrator.Migrate(String targetMigration)
   at Npgsql.EntityFrameworkCore.PostgreSQL.Migrations.Internal.NpgsqlMigrator.Migrate(String targetMigration)
   at Microsoft.EntityFrameworkCore.RelationalDatabaseFacadeExtensions.Migrate(DatabaseFacade databaseFacade)
   at Program.<Main>$(String[] args) in C:\Projects\Npgsql_3074\Program.cs:line 15
rafaelkallis commented 3 months ago

I am facing the exact same problem, only difference compared to @Rudomitori 's example is that we have a text[] column instead of integer[].

We have a historic migration (from net6.0 times) with the following content:

    protected override void Up(MigrationBuilder migrationBuilder)
    {
        migrationBuilder.AddColumn<List<string>>(
            name: "MyListColumn",
            table: "MyTable",
            type: "text[]",
            nullable: false,
            defaultValue: "{}");
    }

After upgrading to 8.0.2 I get the exact same stacktrace as posted above.

Also note that after removing the defaultValue parameter the migration works.

SArmanFatemi commented 3 weeks ago

I encountered the same exception, but upon further investigation, I discovered the error within the *.Desiner.cs files. I am currently working on a legacy software project that originated from an earlier version of dotnet, and it appears that this code was generated in a manner unknown to me.

Here is the section of problematic code:

b.Property<string>("ColumnName")
    .ValueGeneratedOnAdd()
    .HasColumnName("column_name")
    .HasColumnType("character varying(255)[]")
    .HasDefaultValueSql("'{}'::character varying[]");

Upon closer examination, I realized that using the string type for the property was incorrect, and it should actually be string[]. What intrigues me is that this code is functional on dotnet 7.