Allow unmapped enums to be written

Steps to reproduce

open System

open Npgsql
open Npgsql.Logging
open NpgsqlTypes


type DbPeriodStatus =
    | [<PgName("active")>] Active = 0
    | [<PgName("deleted")>] Deleted = 1

let readFirstStatus (connection: NpgsqlConnection) =
    use command = connection.CreateCommand()
    command.CommandText <- "SELECT * FROM sch_accounting.accounting_periods;"
    use reader = command.ExecuteReader()
    reader.Read() |> ignore
    let status = reader.GetFieldValue<DbPeriodStatus>(3)
    printfn "%A" status

let addParameter
    ((name: string), (maybeDbType: NpgsqlDbType option), (value: obj)) (command: NpgsqlCommand) =
    let parameter = command.CreateParameter()
    parameter.SourceColumn <- name
    match maybeDbType with
    | Some dbType -> parameter.NpgsqlDbType <- dbType
    | None -> ()
    parameter.Value <- value
    command.Parameters.Add(parameter) |> ignore

let writeStuff (connection: NpgsqlConnection) =
    use command = connection.CreateCommand()
    command.CommandText <-
        "INSERT INTO sch_accounting.accounting_periods
        (id, start_date, end_date, status, created_by, created_on, updated_by, updated_on)
        VALUES
        (@id, @startDate, @endDate, @status, @createdBy, @createdOn, @updatedBy, @updatedOn)"
    [ ("id", Some NpgsqlDbType.Uuid, Guid.NewGuid() :> obj)
      ("start_date", Some NpgsqlDbType.TimestampTz, DateTimeOffset.UtcNow :> obj )
      ("end_date", Some NpgsqlDbType.TimestampTz, DateTimeOffset.UtcNow :> obj )
      ("status", None, DbPeriodStatus.Active :> obj )
      ("created_by", Some NpgsqlDbType.Text, "Meow" :> obj )
      ("created_on", Some NpgsqlDbType.TimestampTz, DateTimeOffset.UtcNow :> obj )
      ("updated_by", Some NpgsqlDbType.Text, "Meow" :> obj )
      ("updated_on", Some NpgsqlDbType.TimestampTz, DateTimeOffset.UtcNow :> obj ) ]
    |> List.iter(fun stuff -> addParameter stuff command)
    command.ExecuteNonQuery() |> ignore


[<EntryPoint>]
let main _ =
    NpgsqlLogManager.Provider <- ConsoleLoggingProvider(NpgsqlLogLevel.Trace, true, true)
    let connectionString = "Host=localhost;Port=5432;Database=read_model;User ID=postgres;Password=postgres"
    use connection = new NpgsqlConnection(connectionString)
    connection.Open()
    readFirstStatus connection
    readFirstStatus connection
    writeStuff connection
    0

For unmapped enums:

Reading stuff works
Writing stuff fails

The issue

Atm, the attribute to decorate our enums only supports the value:

type DbPeriodStatus =
    | [<PgName("active")>] Active = 0
    | [<PgName("deleted")>] Deleted = 1

It would be nice to instead of having to rely systematically on the mapper: NpgsqlConnection.GlobalTypeMapper.MapEnum("sch_accounting.accounting_period_status")

We could just put either:

[<PgName("sch_accounting.accounting_period_status")>]
type DbPeriodStatus =
    | [<PgName("active")>] Active = 0
    | [<PgName("deleted")>] Deleted = 1

or

type DbPeriodStatus =
    | [<PgName("sch_accounting.accounting_period_status", "active")>] Active = 0
    | [<PgName("sch_accounting.accounting_period_status", "deleted")>] Deleted = 1

or maybe to make the read and write works with:

type DbPeriodStatus =
    | [<PgName("sch_accounting.accounting_period_status.active")>] Active = 0
    | [<PgName("sch_accounting.accounting_period_status.deleted")>] Deleted = 1

There is already a few existing issues opened about unmapped and related stuff

So I know that my take on unmapped enums might be considered as controversial by some others.

The exception we're getting atm:

System.NotSupportedException: The CLR enum type DbPeriodStatus must be registered with Npgsql before usage, please refer to the documentation.
   at Npgsql.TypeMapping.ConnectorTypeMapper.GetByClrType(Type type)
   at Npgsql.NpgsqlParameter.ResolveHandler(ConnectorTypeMapper typeMapper)
   at Npgsql.NpgsqlParameter.Bind(ConnectorTypeMapper typeMapper)
   at Npgsql.NpgsqlCommand.ValidateParameters(ConnectorTypeMapper typeMapper)
   at Npgsql.NpgsqlCommand.ExecuteReader(CommandBehavior behavior, Boolean async, CancellationToken cancellationToken)
   at Npgsql.NpgsqlCommand.ExecuteReader(CommandBehavior behavior, Boolean async, CancellationToken cancellationToken)
   at Npgsql.NpgsqlCommand.ExecuteNonQuery(Boolean async, CancellationToken cancellationToken)
   at Npgsql.NpgsqlCommand.ExecuteNonQuery()
   at Program.writeStuff(NpgsqlConnection connection) in C:\Users\Michelle\Desktop\Personal\Repos\fsharp-playground\FSharp.Playground\FSharp.Playground\Program.fs:line 102
   at Program.main(String[] _arg1) in C:\Users\Michelle\Desktop\Personal\Repos\fsharp-playground\FSharp.Playground\FSharp.Playground\Program.fs:line 113

Further technical details

Npgsql version: 5.0.0
PostgreSQL version: PostgreSQL 12.4 on x86_64-pc-linux-musl, compiled by gcc (Alpine 9.3.0) 9.3.0, 64-bit
Operating system: Windows 10 Professional + Docker

Other details about my project setup:

I think this could be done even in a patch version, would be really useful for users. Thanks for reporting!

@YohDeadfall

I think this could be done even in a patch version, would be really useful for users. Thanks for reporting!

@YohDeadfall
I thought about adding the support in our F# library, but I think it would be a nice addition to npgsql itself, but yea a patch could just do as well.

Any pointers about existing patches / addons that can hint me in the right direction?

For now you should map the enum since for mapped it's supported already:

npgsql/src/Npgsql/TypeMapping/TypeMapperBase.cs

Lines 53 to 54 in 4aad4c0

    
           if (pgName == null) 
        
               pgName = GetPgName(typeof(TEnum), nameTranslator);

npgsql/src/Npgsql/TypeMapping/TypeMapperBase.cs

Lines 127 to 129 in 4aad4c0

    
           static string GetPgName(Type clrType, INpgsqlNameTranslator nameTranslator) 
        
               => clrType.GetCustomAttribute<PgNameAttribute>()?.PgName 
        
                  ?? nameTranslator.TranslateTypeName(clrType.Name);

Otherwise, specify the type name instead of NpgsqlDbType. Or use discriminating unions to pass the exact name just for enums.

By the way, you can avoid specifying the type in case when it can be determined by the driver from the CLR type. See our documnetation for details.

For now you should map the enum since for mapped it's supported already:

npgsql/src/Npgsql/TypeMapping/TypeMapperBase.cs

Lines 53 to 54 in 4aad4c0

if (pgName == null)

pgName = GetPgName(typeof(TEnum), nameTranslator);

npgsql/src/Npgsql/TypeMapping/TypeMapperBase.cs

Lines 127 to 129 in 4aad4c0

static string GetPgName(Type clrType, INpgsqlNameTranslator nameTranslator)

=> clrType.GetCustomAttribute<PgNameAttribute>()?.PgName

?? nameTranslator.TranslateTypeName(clrType.Name);

Otherwise, specify the type name instead of NpgsqlDbType. Or use discriminating unions to pass the exact name just for enums.

Already checked, this is what we did our with F# lib at work

By the way, you can avoid specifying the type in case when it can be determined by the driver from the CLR type. See our documnetation for details.

Also what we did that with our F# lib at work:

/// PostgreSQL unmapped through the npgsql mapper
type UnmappedEnum =
    /// As a .NET Enum marked with the PgName attribute, only works when reading data
    | AsEnum of Enum
    /// As a string
    | AsString of string

// https://www.npgsql.org/doc/types/basic.html
// https://stackoverflow.com/a/845472/4636721
type PostgreSqlDbValue =
    | Null
    | Bit of bool
    | Boolean of bool
    | SmallInt of int16
    | Integer of int32
    | Oid of uint32
    | Xid of uint32
    | Cid of uint32
    | BigInt of int64

    | Real of single
    | Double of double

    | Money of decimal
    | Numeric of decimal

    | ByteA of uint8 array
    | OidVector of uint32 array

    | Uuid of Guid

    | INet of IPAddress
    | MacAddr of PhysicalAddress

    | TsQuery of NpgsqlTsQuery
    | TsVector of NpgsqlTsVector

    | Point of NpgsqlPoint
    | LSeg of NpgsqlLSeg
    | Path of NpgsqlPath
    | Polygon of NpgsqlPolygon
    | Line of NpgsqlLine
    | Circle of NpgsqlCircle
    | Box of NpgsqlBox

    | HStore of Dictionary<string, string>

    | Date of DateTime
    | Interval of TimeSpan
    | Time of DateTime
    | TimeTz of DateTime
    | Timestamp of DateTime
    | TimestampTz of DateTimeOffset

    | InternalChar of uint8

    | Char of string
    | VarChar of string

    | Name of string
    | CiText of string
    | Text of string
    | Xml of string
    | Json of string
    | Jsonb of string
    /// Only if the relevant Npgsql mapping for the Enum has been set up beforehand.
    /// See: https://www.npgsql.org/doc/types/enums_and_composites.html
    | MappedEnum of Enum
    /// Only if there is no relevant Npgsql mapping that has been set up beforehand.
    | UnmappedEnum of value: UnmappedEnum * typeName: string

@roji

@roji, the next PR with tests I told you about is for that issue, and I think it's a good non-breaking improvement for 5.0.1 which gave me some thoughts on mappings for future changes.

@YohDeadfall

@YohDeadfall approved this, thanks! I'm not quite sure it's a good fit for 5.0.1 though, as this is really a new feature.

I know we merged #3391 for 5.0.1 even though it could be considered a feature, though it also seems to have been an omission (i.e. almost a bug). This case (writing unmapped enums) is less clear.

I guess I'm fine with it, it's very low-risk.

Supported unmapped enum writing (#3398)

@roji

@roji @YohDeadfall

I know we merged #3391 for 5.0.1 even though it could be considered a feature, though it also seems to have been an omission (i.e. almost a bug). This case (writing unmapped enums) is less clear.

I guess I'm fine with it, it's very low-risk.

Based on my understanding, this should work fine starting with version 5.0.1?

Yes, it's published already.

@YohDeadfall

@YohDeadfall cool, that's pretty awesome!
(Need to adjust our lib then)

Sunbelt Computer Software

PL/B Language Development and Support

Navigation Menu

Repository navigation

Allow unmapped enums to be written #3389

Steps to reproduce

The issue

Further technical details

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

	if (pgName == null)
	pgName = GetPgName(typeof(TEnum), nameTranslator);

	static string GetPgName(Type clrType, INpgsqlNameTranslator nameTranslator)
	=> clrType.GetCustomAttribute<PgNameAttribute>()?.PgName
	?? nameTranslator.TranslateTypeName(clrType.Name);

Sunbelt Computer Software

PL/B Language Development and Support

Allow unmapped enums to be written #3389

Description

Steps to reproduce

The issue

Further technical details

Activity

YohDeadfall commented on Dec 4, 2020

natalie-o-perret commented on Dec 4, 2020

YohDeadfall commented on Dec 4, 2020

natalie-o-perret commented on Dec 4, 2020

YohDeadfall commented on Dec 8, 2020

roji commented on Dec 10, 2020

roji commented on Dec 10, 2020

natalie-o-perret commented on Dec 18, 2020

YohDeadfall commented on Dec 18, 2020

natalie-o-perret commented on Dec 18, 2020

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions