How to migrate from `PostgresDataConvertible`?

[snow helm]

I'm having some trouble upgrading to a newer version of postgres-kit. The PostgresDataConvertible protocol was deprecated in favor of PostgresDataEncodable and PostgresDataDecodable, but the latter aren't super straightforward to implement. When I do I get typeMismatch errors thrown during decoding that are decoded just fine when using PostgresDataConvertible. My main goal is to get the Tagged type from this repo (which is basically a concrete version of RawRepresentable with some bells and whistles) as something that is decodable from Postgres. Can anyone help out or let me know where I'm going wrong?

[whole portal]

Something like this doesn't work?
What is your current implementation?

extension Tagged: PostgresDecodable where RawValue: PostgresDecodable {
    public init(
        from byteBuffer: inout ByteBuffer,
        type: PostgresDataType,
        format: PostgresFormat,
        context: PostgresDecodingContext<some PostgresJSONDecoder>
    ) throws {
        self.init(
            try RawValue(
                from: &byteBuffer,
                type: type,
                format: format,
                context: context
            )
        )
    }
}

For PostgresEncodable:

extension Tagged: PostgresThrowingDynamicTypeEncodable where RawValue: PostgresThrowingDynamicTypeEncodable {
    public var psqlFormat: PostgresFormat {
        self.rawValue.psqlFormat
    }

    public var psqlType: PostgresDataType {
        self.rawValue.psqlType
    }

    public func encode(
        into byteBuffer: inout ByteBuffer,
        context: PostgresEncodingContext<some PostgresJSONEncoder>
    ) throws {
        try self.rawValue.encode(
            into: &byteBuffer,
            context: context
        )
    }
}

extension Tagged: PostgresEncodable where RawValue: PostgresEncodable {
    public static var psqlType: PostgresDataType {
        RawValue.psqlType
    }

    public static var psqlFormat: PostgresFormat {
        RawValue.psqlFormat
    }
}

[snow helm]

Nope, that causes decoding failures for me

It throws typeMismatch for some reason, and if I inspect the type decoded by the PostgresJSONDecoder, it's UNKNOWN 16440

[whole portal]

can you share your PostgresDataConvertible conformance?

and also how you're trying to decode now, vs you you would decode with PostgresDataConvertible

[snow helm]

It's the trivial conformance: ```
extension Tagged: PostgresDataConvertible where RawValue: PostgresDataConvertible {}

I'm calling SQLRow.decode for decoding

[whole portal]

O hmm so that could be problematic

let me do some digging

[snow helm]

Here are all the helpers we use: https://github.com/pointfreeco/pointfreeco/blob/63db1dd9a73c6e9f240aebe4a8072b1a725ee1f0/Sources/Database/Helpers.swift

[whole portal]

So @snow helm I think PostgresKit is still not compatible with the non-deprecated PostgresNIO APIs

Those depreciation warnings come from PostgresNIO, which is the underlying package that PostgresKit uses

[snow helm]

Hm, should I file an issue to remove those deprecations till they can be in lock-step with one another?

[whole portal]

Still not sure what exactly is happening in your code that results in that error, but I think what I said is true anyway

Cc @chilly raft

I'm already working on some stuff, but they're not ready to use/try:
#issues-and-prs message

Feel free to file an issue though

[snow helm]

Opened, thanks! https://github.com/vapor/postgres-nio/issues/437

[whole portal]

@snow helm I see @chilly raft is right about her having already updated the APIs with the newer PostgresNIO APIs, although the PR i have in works is still very much valid since it'll make better use of the new PostgresNIO APIs (Gwynne couldn't have done it in a non-breaking or pain-less way, i think ... my changes will might even require a new major version)
I'm not sure what exactly is happening then unless you can provide some sample code. I have guesses but my guesses so far have been wrong so prefer not to throw even more guesses.

[whole portal]

I also found out the better way to do this is to simply:
extension Tagged: PostgresDecodable where RawValue: PostgresDecodable, RawValue._DecodableType == RawValue { }
Though the conformance of Tagged to Decodable could cause problems. I was dealing with some of those problems a while ago where PostgresNIO would choose the Codable conformance instead of the raw PostgresDecodable conformance, and try to decode the value as a JSON blob instead of as a Postgres value, which would cause decoding errors.
I'm not sure if this case is possible at all in PostgresKit, but it does happen when using raw PostgresNIO.

Also @chilly raft @sinful nimbus I have no idea why these work:

extension Tagged: PostgresThrowingDynamicTypeEncodable where RawValue == String { }

extension Tagged: PostgresEncodable where RawValue == String { }

enum T: String, PostgresEncodable { case a }

But this doesn't:

extension Tagged: PostgresThrowingDynamicTypeEncodable where RawValue: PostgresThrowingDynamicTypeEncodable { }

extension Tagged: PostgresEncodable where RawValue: PostgresEncodable { }

I have a few guesses but long story short, i still wasn't able to make it work without manually providing conformances, although enum T: String, PostgresEncodable { case a } still works.
It's ... very weird. I assume i'm just missing the proper/needed conformance instead of RawValue: PostgresThrowingDynamicTypeEncodable, but i can't find it.

[chilly raft]

What is the RawValue type being decoded when the error shows up?

Type mismatch with an UNKNOWN OID greater than 16384 means you're dealing with the enum string conversion problem

There is a workaround in place for that: https://github.com/vapor/postgres-kit/blob/main/Sources/PostgresKit/PostgresDataTranslation.swift#L41-L42

It looks like the problem is that you're not trying to decode it as a string.

The old version works because its passthrough logic is less strict (dangerously so, which is why I didn't keep it that way for the new code)

Hm, this is an unusual case.

Let me dig a bit further.

Okay, the basic problem is that conforming Tagged to PostgresCodable is not enough for compatibility. To be fully compatible, you have to keep the conformance to the deprecated protocol as well (a problem I encountered in PostgresKit itself - if you look at the logs, you'll see there's a deprecation warning coming from PostgresKit that I can't eliminate entirely without a new major version; see https://github.com/vapor/postgres-kit/blob/main/Sources/PostgresKit/PostgresDataTranslation.swift#L18-L21)

[sinful nimbus]

@snow helm can you describe for which type the conversion works using the old pattern and doesn’t for the new pattern? Also what is the type in the db?

[chilly raft]

@sinful nimbus It's not a PostgresNIO issue, it's at a higher layer.

[snow helm]

Hey all, I linked to our web site that is having the issue, which is also open source. Specifically the helpers in the file that call to SQLRow.decode fail to decode Tagged values with the newer conformance

I'm not much of a Discord user, so I may miss updates here. Any reason why the discussion couldn't have remained open on GitHub since there does appear to be an issue in the library?

I guess my main question is should this API be hard deprecated at all yet if it's causing noise in downstream ways that cannot be silenced?

Hard deprecations should ideally signal something that a person can fix in their code base to remain current

I think a soft deprecation could be appropriate for now while things are in limbo

[chilly raft]

@sinful nimbus 😏 I get to say I told you so 😛

@snow helm If you're using the PostgresKit layer (and presumably also FluentPostgresDriver via Fluent), why are you conforming to PostgresDataConvertible at all?

[snow helm]

@chilly raft It's possible I only need to use PostgresNIO. Our use is pretty lightweight. We simply interact with a SQLDatabase and decode SQLRows

It sounds like there's no way to make Tagged decodable for those APIs with the newer PostgresDecodable protocol, though?

[chilly raft]

Okay, that's the PostgresKit layer.

It's entirely possible to make Tagged decodable - the problem isn't with Tagged, it's with whatever the underlying type that's involved is.

[snow helm]

I think the typeMismatch decoding error I was getting was for a Tagged<_, String>

All the decoding works fine through the older PostgresDataConvertible protocol

[chilly raft]

Ah ha. That makes sense, sadly.

Let me check something here...

The underlying problem is that by the time you reach the decode implementation in Tagged, it's too late; you're already below the level where it's currently possible to reinvoke all the workaround logic in the PostgresKit layer.

I need to do something about that - my apologies; the problem is indeed in PostgresKit, although in an extremely subtle way.

Ooof, this is gonna take some non-trivial work to fix. I'm actually a bit surprised no one's hit it before.

@snow helm I will reopen the issue you filed and keep you updated therein