Lack a default Type Mapping for short array in composite to PostgreSQL smallint[] · Issue #5591 · npgsql/npgsql · GitHub
Skip to content

Lack a default Type Mapping for short array in composite to PostgreSQL smallint[]  #5591

Open

Description

@guderianfyzz

Hi,
First, thanks for providing such a great project!

I've encountered a situation where Npgsql seems to lack a default type mapping for short arrays to Postgre smallint[] fields within composite types. Below are the details:

Steps to reproduce

using Npgsql;

// 1. Assuming a PostgreSQL setup with the following type and table:
CREATE TYPE public.inventory_item AS
(
    name text,
    supplier_id integer,
    price numeric,
    shortlist smallint[]
);

CREATE TABLE some_table (
  id serial PRIMARY KEY,
  my_composite inventory_item
);

// 2. C# code that demonstrates the issue:
public class InventoryItem
{
    public string Name { get; set; } = "";
    public int SupplierId { get; set; }
    public decimal Price { get; set; }
    public short[] ShortList { get; set; }
}

// (Code for connection setup using NpgsqlDataSourceBuilder)

await using var conn = dataSource.CreateConnection();
await conn.OpenAsync();

await using var cmdWriting = new NpgsqlCommand("INSERT INTO public.some_table (my_composite) VALUES (@composite)", conn);

var inventoryItem = new InventoryItem
{
  Name = "InventoryItemName",
  SupplierId = 4323,
  Price = 300.0m,
  ShortList = new short[] { 1, 2, 3 } 
};

cmdWriting.Parameters.AddWithValue("@composite", inventoryItem);
await cmdWriting.ExecuteNonQueryAsync(); 

The issue

The code above produces the following errors:

  • System.InvalidCastException: 'Writing values of 'npg_composite_mapping_object_issue.InventoryItem' is not supported for parameters having no NpgsqlDbType or DataTypeName. Try setting one of these values to the expected database type..'

  • InvalidOperationException: Cannot find property or field for composite field 'shortlist'.

The issue is the lack of a default type handler within Npgsql to map a C# short[] property to a PostgreSQL smallint[] field in a composite type.

Further technical details

  • Npgsql version: 8.0.2
  • Operating system: Windows and macOS

Activity

  1. changed the title [-]Lack a default Type Mapping for C# short[] to PostgreSQL smallint[] Mapping[/-] [+]Lack a default Type Mapping for C# short[] to PostgreSQL smallint[] [/+] on Feb 24, 2024
  2. changed the title [-]Lack a default Type Mapping for C# short[] to PostgreSQL smallint[] [/-] [+]Lack a default Type Mapping for short array in composite to PostgreSQL smallint[] [/+] on Feb 24, 2024
  3. NinoFloris commented on Feb 24, 2024

    @NinoFloris
    Member

    Glad to hear you like it.

    The outer exception does not mention the short[] type being the issue but the composite type instead
    npg_composite_mapping_object_issue.InventoryItem.

    The underlying cause is stated in the inner exception message: Cannot find property or field for composite field 'shortlist'.

    The composite info factory tries to match the postgres fields to CLR fields/props the first time you read or write the composite type. We have to delay validation until that point because during MapComposite we don't actually know what the composite type looks like yet. We haven't yet queried the database for its types, so all we know is the composite name at that point.

    Try renaming your shortlist column to short_list, which would be the name after translation by the datasourcebuilder.DefaultNameTranslator (assuming you haven't explicitly passed a translator to MapComposite).

    Alternatively - if your schema is already final - you can add an attribute to your property like so:

    public class InventoryItem
    {
        public string Name { get; set; } = "";
        public int SupplierId { get; set; }
        public decimal Price { get; set; }
        [PgNameAttribute("shortlist")]
        public short[] ShortList { get; set; }
    }
  4. guderianfyzz commented on Feb 25, 2024

    @guderianfyzz
    Author

    Hi, @NinoFloris
    Thanks a lot for your answer. It's truly encouraging to see such a supportive and responsive community. Your suggestion to add the PgNameAttribute does work, thank you very much. Nonetheless, we're currently facing an additional challenge in our production environment that requires further assistance. Allow me to elaborate on the issue:

    We have a custom type mapping in our system, where our enums inherit from short, not the typical int, so the mapping in PostgreSQL is smallint. We were previously using version 5 with our own custom type mapper based on a handler, which worked fine. Now, we are trying to upgrade to version 8. Since version 8, the handlers were completely dropped and switched to converters and resolvers. So, we had to rewrite it using the new converter.

    Below is a simplified version of our production code to illustrate the core of our issue more clearly. This distilled example captures the essence of our current setup and the challenges we're facing:

    **Type.cs**:
    ```csharp
    public class InventoryItem4
    {
        public string Name { get; set; } = "";
        public int SupplierId { get; set; }
        public decimal Price { get; set; }
        
        public Mood mood { get; set; }
        
        [PgName("moodlist")]
        public Mood[] MoodList  { get; init; } = Array.Empty<Mood>();
    }
    
    public enum Mood : short
    {
        Sad,
        Ok,
        Happy
    }

    Resolver:

    public sealed class CustomTypeInfoResolverFactory : PgTypeInfoResolverFactory
    {
        public override IPgTypeInfoResolver CreateResolver() => new CustomEnumTypeResolver(); 
        public override IPgTypeInfoResolver? CreateArrayResolver() => new CustomTypeArrayResolver();
    
        class CustomEnumTypeResolver : IPgTypeInfoResolver
        {
            private TypeInfoMappingCollection? _mappings;
            protected TypeInfoMappingCollection Mappings => _mappings ??= AddMappings(new());
            
            public PgTypeInfo? GetTypeInfo(Type? type, DataTypeName? dataTypeName, PgSerializerOptions options)
            {
                return Mappings.Find(type, dataTypeName, options);
            }
    
            public TypeInfoMappingCollection AddMappings(TypeInfoMappingCollection mappings)
            {
                mappings.AddStructType<Mood>("pg_catalog.int2",
                     static (options, mapping, _) => mapping.CreateInfo(options, new MyEnumToSmallIntConverter<Mood>()), isDefault: false);
                return mappings;
            }
        }
        
        class CustomTypeArrayResolver : CustomEnumTypeResolver, IPgTypeInfoResolver
        {
            TypeInfoMappingCollection? _mappings;
            new TypeInfoMappingCollection Mappings => _mappings ??= AddMappings(new(base.Mappings));
            public PgTypeInfo? GetTypeInfo(Type? type, DataTypeName? dataTypeName, PgSerializerOptions options)
            {
                return Mappings.Find(type, dataTypeName, options);
            }
                
            public TypeInfoMappingCollection AddMappings(TypeInfoMappingCollection mappings)
            {
                mappings.AddStructArrayType<Mood>("pg_catalog.int2");
    
                return mappings;
            }
        }
    }

    Converter:

    public class MyEnumToSmallIntConverter<T> : PgBufferedConverter<T>
    {
        public MyEnumToSmallIntConverter() : base(customDbNullPredicate: false) {}
    
        protected override T ReadCore(PgReader reader)
        {
            if (!typeof(T).IsEnum)
            {
                throw new InvalidOperationException($"The generic type {typeof(T).Name} must be an enum.");
            }
    
            short value = reader.ReadInt16();
            return (T)Enum.ToObject(typeof(T), value);
        }
    
        protected override void WriteCore(PgWriter writer, T value)
        {
            if (typeof(T).IsEnum)
            {
                short enumAsShort = (short)(object)value;
                writer.WriteInt16(enumAsShort);
            }
            else
            {
                throw new InvalidCastException($"Invalid cast from {typeof(T).Name} to smallint. T must be an enum type.");
            }
        }
    
        public override bool CanConvert(DataFormat format, out BufferRequirements bufferRequirements)
        {
            if (format == DataFormat.Binary)
            {
                bufferRequirements = BufferRequirements.CreateFixedSize(2); 
                return true;
            }
            else
            {
                bufferRequirements = default;
                return false;
            }
        }
    }

    Main:

    var connectionString = "User ID=root;Host=localhost;Port=xxx;Database=xxx;Password=xxx";
    var dataSourceBuilder = new NpgsqlDataSourceBuilder(connectionString);
    var suffix = "4";
    var pgName = $"public.inventory_item_{suffix}";
    var tableName = $"some_table_{suffix}";
    dataSourceBuilder.MapComposite<InventoryItem4>(pgName);
    var customTypeInfoResolverFactory = new CustomTypeInfoResolverFactory();
    dataSourceBuilder.AddTypeInfoResolverFactory(customTypeInfoResolverFactory);
    
    await using var dataSource = dataSourceBuilder.Build();
    await using var conn = dataSource.CreateConnection();
    await conn.OpenAsync();
    
    await using var cmdWriting = new NpgsqlCommand($"INSERT INTO public.{tableName} (my_enum, my_composite) VALUES (@enum, @composite)", conn);
    
    
    var inventoryItem4 = new InventoryItem4
    {
        Name = "InventoryItemName",
        SupplierId = 4323,
        Price = 300.0m,
        mood = Mood.Happy,
        MoodList = new Mood[] {Mood.Happy, Mood.Ok, Mood.Sad}
    };
    
    cmdWriting.Parameters.AddWithValue("@enum", Mood.Sad);
    cmdWriting.Parameters.AddWithValue("@composite", inventoryItem4);
    
    // Execute the command
    await cmdWriting.ExecuteNonQueryAsync();

    It works fine in this case, but in this example, I was specifically injecting the type Mood in the code:

    mappings.AddStructType<Mood>("pg_catalog.int2", static (options, mapping, _) => mapping.CreateInfo(options, new MyEnumToSmallIntConverter<Mood>()), isDefault: false);

    and

    mappings.AddStructArrayType<Mood>("pg_catalog.int2");

    Our production code follows a similar idea to implement. However, because there are many types in our production code, we cannot specifically inject the type. We want our code to get the type at runtime and dynamically inject it into the mapper, so we use reflection to capture the runtime type.

    For non-array resolver, like this:

    private void MapEnumPropertiesRecursive(TypeInfoMappingCollection typeMappingCollection, Type type, PgTypeId smallIntTypeId)
    {
        PgConverter smallIntConverterInstance = CreateSmallIntConverterInstance(propertyType);
        TypeInfoFactory pgFactory = CreatePgFactory(propertyType, smallIntConverterInstance, smallIntTypeId);
        Type[] parameterTypes = { typeof(string), typeof(TypeInfoFactory), typeof(bool) };
        MethodInfo addStructTypeMethod = typeMappingCollection.GetType().GetMethod("AddStructType", parameterTypes).MakeGenericMethod(propertyType);
        addStructTypeMethod.Invoke(typeMappingCollection, new object[] { "pg_catalog.int2", pgFactory, false });
    }

    And for array resolver:

    MethodInfo addStructArrayTypeMethod = typeMappingCollection.GetType().GetMethod(nameof(TypeInfoMappingCollection.AddStructArrayType), parameterTypes).MakeGenericMethod(propertyType);
    
    // Invoke the generic method with the runtime type
    addStructArrayTypeMethod.Invoke(typeMappingCollection, new object[] { "pg_catalog.int2" });

    However, this does not work in production anymore, we get the error:

    System.InvalidOperationException: Could not find mapping for ProductionNameSpace.productionEnumTypeA <-> pg_catalog.int2
    

    Our production type is composite, like this:

    public record ProductionRecord {
        int integerA,
        string stringA,
        .........
        productionEnumTypeA enumTypeA
    };

    The strange thing is that, if I make the CreateArrayResolver point to null, this case will work. And What I understand from the Npgsql code is that CreateResolver function in TypeInfoResolverFactory deals with C# type to PostgreSQL type and CreateArrayResolver deals with the array of type to PostgreSQL type.

    public override IPgTypeInfoResolver CreateArrayResolver() => null;

    However, when we set CreateArrayResolver to null, the productionEnumTypeA case functions correctly. Yet, this adjustment leads to a problem with another case, specifically with arrays, which does not work as intended.

    [System.NotSupportedException
    No resolution could be found for ('Production.ProductionEnumTypeB[]', 'pg_catalog.smallint[]'). Mapping: CLR property 'ProductionEnumTypeB[].enumTypeBName' <-> Composite field 'postgres_table_type.enum_typeb_name'
    

    This composite type is like this:

    public record ProductionTypeB : productionTypeC
    {
        public ProductionEnumTypeB[] enumTypeBName;
    }

    It's very strange that the error becomes a combination of TypeName plus fieldName like ProductionEnumTypeB[].enumTypeBName. At this point, we are really stuck and would really appreciate if you could help us.

    1. Could you help us look at the implementation in the simple case, point out if that follows the current best practice to implement? If not, could you help us point out where we could find the best practice to follow? There is very limited resource I could find on the internet, partly because this new resolver and converter system just came out last August.
    2. Could you help us point out what is the issue with our production example, what we could do or what way we could try to fix it? Greatly appreciate your help!
  5. NinoFloris commented on Feb 25, 2024

    @NinoFloris
    Member

    After taking a look at your reflection code it looks like you're passing in the full property type to AddStructArrayType. Make sure it's the element type like you passed in your manual setup.

    It needs the element type to make multiple collection mappings over it - without reflection - like T[], T?[] (including their multi-dim mappings) and IList<T>, IList<T?>.

    The next step of why you're getting System.InvalidOperationException: Could not find mapping for ProductionNameSpace.productionEnumTypeA <-> pg_catalog.int2 is that before calling AddStructArrayType you'll have to make sure the collection contains the base type (the one that has the factory etc) via AddStructType.

    AddStructArrayType is basically a compositional helper method as it's not very easy to create all those mappings correctly manually. It looks up the base type mapping which is used to read and write the element data.

    All of this array handling used to be done automatically in the previous handler system and required reflection and MakeGenericType, which meant it was completely incompatible with trimming and NativeAOT.

    Take a look at the UnmappedTypeInfoResolverFactory to see how we're dealing with reflection based mappings. Instead of implementing IPgTypeInfoResolver we inherit from DynamicTypeInfoResolver to help write resolvers when types are statically unknown.

    If you do know all the types you need at app startup (but just don't during compilation) you could keep using what code you have and just fill a collection of mappings via reflection.

    The DynamicTypeInfoResolver is really meant for those cases where you want to resolve against a *pattern* like ex. "all type.IsEnum types when their pg type is 'hstore'" while you have no way of preloading the set of types that would fit that pattern.

    Regarding

    It's very strange that the error becomes a combination of TypeName plus fieldName like ProductionEnumTypeB[].enumTypeBName. At this point, we are really stuck and would really appreciate if you could help us.

    That's a good catch, the exception message is using the wrong data, that will be fixed in 8.0.3 :)

    Just as a heads up, we're like to make a small api change to public override bool CanConvert(DataFormat format, out BufferRequirements bufferRequirements) so we better support DataFormat.Text - which wasn't fully fleshed out before release - but it will be easy to adapt, most likely it's just an extra argument.

    Overall your converter code looks fine! All I might say is that you can remove the empty constructor, and you could also remove your typeof(T).IsEnum checks by adding where T : Enum to the class instead.

    Good luck with the implementation!

  6. guderianfyzz commented on Feb 26, 2024

    @guderianfyzz
    Author

    Thanks! @NinoFloris, for your great insights. After revisiting the implementation, it's unsurprising that we've encountered numerous questions, prompting me to inquire about one particular aspect. Specifically, I'm interested in knowing if enums in C# can seamlessly work with PostgreSQL integers by default. This question arises from the understanding that utilizing short would unequivocally necessitate custom mapping, inevitably leading to the implementation of custom type mappings by ourselves and thereby heightening maintenance costs.

    Given these considerations, we are contemplating reverting from inheriting from short to default usage of enums in C#. This approach appears to be viable for our backend. However, the challenge arises when interfacing with the database. From what I've gleaned from examples, employing a structure like:

    dataSourceBuilder.MapEnum<Mood>();

    mandates the creation of a corresponding type in the database, exemplified by:

    CREATE TYPE mood AS ENUM ('sad', 'ok', 'happy');

    This requirement significantly increases our workload for several reasons:

    1. We would need to define enum types comprehensively, potentially amounting to hundreds.
    2. All PostgreSQL functions and tables utilizing these enums would have to adopt the new type.
    3. Future development efforts would necessitate the creation of corresponding database types for every new enum in the backend. Similarly, any modification to an enum would require an update. This could pose coordination challenges, given that database development and backend development are handled by distinct teams.

    In light of these considerations, if a default mapping between standard enums and integers in PostgreSQL exists, it could substantially reduce both upgrade and maintenance costs. Moreover, if no such default mapping currently exists, is there any plan to support this feature in the foreseeable future?

    Example: Tried with default Enum and postgres int, but not work

    public enum Mood 
    {
        Sad,
        Ok,
        Happy
    }
    
    public class InventoryItem5
    {
        public string Name { get; set; } = "";
        public int SupplierId { get; set; }
        public decimal Price { get; set; }
        
        [PgName("mood")]
        public Mood mood { get; set; }
        
        [PgName("moodlist")]
        public Mood[] MoodList  { get; init; } = Array.Empty<Mood>();
    }
    
    dataSourceBuilder.MapComposite<InventoryItem5>(pgName);
    dataSourceBuilder.MapEnum<Mood>();
    await using var dataSource = dataSourceBuilder.Build();
    await using var conn = dataSource.CreateConnection();
    await conn.OpenAsync();
    
    await using var cmdWriting = new NpgsqlCommand($"INSERT INTO public.{tableName} (my_composite) VALUES (@composite)", conn);
    
    var inventoryItem5 = new InventoryItem5
    {
        Name = "InventoryItemName",
        SupplierId = 23,
        Price = 30.0m,
        mood = Mood.Sad,
        MoodList = new Mood[] {Mood.Happy, Mood.Ok}
    };
    
    cmdWriting.Parameters.AddWithValue("@composite", inventoryItem5);
    
    await cmdWriting.ExecuteNonQueryAsync();

    With the error of:

    Unhandled exception. System.InvalidCastException: Writing values of 'npg_composite_mapping_object_issue.InventoryItem5' is not supported for parameters having no NpgsqlDbType or DataTypeName. Try setting one of these values to the expected database type..
     ---> System.NotSupportedException: No resolution could be found for ('npg_composite_mapping_object_issue.Mood', 'pg_catalog.integer'). Mapping: CLR property 'Mood.mood' <-> Composite field 'inventory_item_5.mood'
       at Npgsql.Internal.Composites.ReflectionCompositeInfoFactory.CreateCompositeInfo[T](PostgresCompositeType pgType, INpgsqlNameTranslator nameTranslator, PgSerializerOptions options)
       at Npgsql.TypeMapping.UserTypeMapper.CompositeMapping`1.<AddMapping>b__2_0(PgSerializerOptions options, TypeInfoMapping mapping, Boolean _)
       at Npgsql.Internal.TypeInfoMappingCollection.Find(Type type, Nullable`1 dataTypeName, PgSerializerOptions options)
       at Npgsql.TypeMapping.UserTypeMapper.Resolver.Npgsql.Internal.IPgTypeInfoResolver.GetTypeInfo(Type type, Nullable`1 dataTypeName, PgSerializerOptions options)
       at Npgsql.Internal.ChainTypeInfoResolver.GetTypeInfo(Type type, Nullable`1 dataTypeName, PgSerializerOptions options)
       at Npgsql.Internal.TypeInfoCache`1.<GetOrAddInfo>g__CreateInfo|6_3(Type type, Nullable`1 typeId, PgSerializerOptions options, Boolean defaultTypeFallback, Boolean validatePgTypeIds)
       at Npgsql.Internal.TypeInfoCache`1.<GetOrAddInfo>g__AddByType|6_1(Type type)
       at Npgsql.Internal.TypeInfoCache`1.GetOrAddInfo(Type type, Nullable`1 pgTypeId, Boolean defaultTypeFallback)
       at Npgsql.Internal.PgSerializerOptions.GetTypeInfoCore(Type type, Nullable`1 pgTypeId, Boolean defaultTypeFallback)
       at Npgsql.Internal.PgSerializerOptions.GetTypeInfo(Type type, Nullable`1 pgTypeId)
       at Npgsql.Internal.AdoSerializerHelpers.GetTypeInfoForWriting(Type type, Nullable`1 pgTypeId, PgSerializerOptions options, Nullable`1 npgsqlDbType)
       --- End of inner exception stack trace ---
       at Npgsql.Internal.AdoSerializerHelpers.<GetTypeInfoForWriting>g__ThrowWritingNotSupported|1_0(Type type, PgSerializerOptions options, Nullable`1 pgTypeId, Nullable`1 npgsqlDbType, Exception inner)
       at Npgsql.Internal.AdoSerializerHelpers.GetTypeInfoForWriting(Type type, Nullable`1 pgTypeId, PgSerializerOptions options, Nullable`1 npgsqlDbType)
       at Npgsql.NpgsqlParameter.ResolveTypeInfo(PgSerializerOptions options)
       at Npgsql.NpgsqlParameterCollection.ProcessParameters(PgSerializerOptions options, Boolean validateValues, CommandType commandType)
       at Npgsql.NpgsqlCommand.ExecuteReader(Boolean async, CommandBehavior behavior, CancellationToken cancellationToken)
       at Npgsql.NpgsqlCommand.ExecuteReader(Boolean async, CommandBehavior behavior, CancellationToken cancellationToken)
       at Npgsql.NpgsqlCommand.ExecuteNonQuery(Boolean async, CancellationToken cancellationToken)
       at Program.<Main>$(String[] args) in /Users/zz/RiderProjects/npg-composite-mapping-object-issue/npg-composite-mapping-object-issue/Program.cs:line 87
       at Program.<Main>$(String[] args) in /Users/zz/RiderProjects/npg-composite-mapping-object-issue/npg-composite-mapping-object-issue/Program.cs:line 87
       at Program.<Main>$(String[] args) in /Users/zz/RiderProjects/npg-composite-mapping-object-issue/npg-composite-mapping-object-issue/Program.cs:line 87
       at Program.<Main>$(String[] args) in /Users/zz/RiderProjects/npg-composite-mapping-object-issue/npg-composite-mapping-object-issue/Program.cs:line 87
       at Program.<Main>(String[] args)
    
  7. NinoFloris commented on Feb 27, 2024

    @NinoFloris
    Member

    There is no builtin support for it yet, but the general idea is tracked in #3303. Support for int2 and int8 seem like good additions to it and would be very easy to add once we have the int4 support.

    There are some tricky bits to do this without boxing and being NativeAOT compatible too but it's on our radar.

    We'll have to see whether its reasonable to backport this to 8.x though.

    EDIT:
    Just to be clear, it will require a DbType/NpgsqlDbType/DataTypeName to be set on the parameter for the driver to understand the representation you want to write the enum type as. We could potentially default to string or the underlying type of the CLR enum type, but there will always be cases where it will be the wrong default for different people.

  8. waqar-islam commented on Sep 9, 2024

    @waqar-islam

    @NinoFloris IN case of composite types what will be DbType/NpgsqlDbType/DataTypeName?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions