vapor / postgres-nio

🐘 Non-blocking, event-driven Swift client for PostgreSQL.
https://api.vapor.codes/postgresnio/documentation/postgresnio/
MIT License
317 stars 72 forks source link

Add support for decimal types other than Foundation.Decimal #377

Open ratranqu opened 1 year ago

ratranqu commented 1 year ago

Currently, the only numerical option for an exact representation of the Postgres NUMERIC type is to use Foundation.Decimal. However, Foundation.Decimal is not fully implemented, and in particular is missing some useful conformances such as LosslessStringConvertible or FloatingPoint. The current implementation makes it complex to use other Decimal implementations to map to the NUMERIC type.

This PR aims to provide a new protocol ExpressibleByFloatingPointString, to which Foundation.Decimal can trivially conform, and at the same time allow to easily make other Decimal types conform to (*), and be used as mapping to the Postgres NUMERIC type.

extension Decimal: ExpressibleByPostgresFloatingPointString {
    public init?(floatingPointString: String) {
        self.init(string: floatingPointString)
    }
}

The change aims to make no changes to the API. It has been tested with the Test suite from postgres-nio and that of postgres-kit as well as from use in a higher level Vapor app.

-- (*) for example BigDecimal from https://github.com/mgriebling/BigDecimal.git

import PostgresNIO
import BigDecimal

extension BigDecimal: ExpressibleByPostgresFloatingPointString {
    public init?(floatingPointString: String) {
        self.init(floatingPointString)
    } 
}
codecov-commenter commented 1 year ago

Codecov Report

Attention: Patch coverage is 70.00000% with 9 lines in your changes are missing coverage. Please review.

Project coverage is 61.72%. Comparing base (dc9caf8) to head (3eab477).

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #377 +/- ## ========================================== - Coverage 61.82% 61.72% -0.11% ========================================== Files 125 126 +1 Lines 10019 10024 +5 ========================================== - Hits 6194 6187 -7 - Misses 3825 3837 +12 ``` | [Files](https://app.codecov.io/gh/vapor/postgres-nio/pull/377?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=vapor) | Coverage Δ | | |---|---|---| | [...PostgresNIO/New/Data/Decimal+PostgresCodable.swift](https://app.codecov.io/gh/vapor/postgres-nio/pull/377?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=vapor#diff-U291cmNlcy9Qb3N0Z3Jlc05JTy9OZXcvRGF0YS9EZWNpbWFsK1Bvc3RncmVzQ29kYWJsZS5zd2lmdA==) | `100.00% <100.00%> (+25.00%)` | :arrow_up: | | [...ources/PostgresNIO/Data/PostgresData+Decimal.swift](https://app.codecov.io/gh/vapor/postgres-nio/pull/377?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=vapor#diff-U291cmNlcy9Qb3N0Z3Jlc05JTy9EYXRhL1Bvc3RncmVzRGF0YStEZWNpbWFsLnN3aWZ0) | `0.00% <0.00%> (ø)` | | | [...ources/PostgresNIO/Data/PostgresData+Numeric.swift](https://app.codecov.io/gh/vapor/postgres-nio/pull/377?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=vapor#diff-U291cmNlcy9Qb3N0Z3Jlc05JTy9EYXRhL1Bvc3RncmVzRGF0YStOdW1lcmljLnN3aWZ0) | `67.78% <0.00%> (-3.58%)` | :arrow_down: | | [...ata/ExpressibleByPostgresFloatingPointString.swift](https://app.codecov.io/gh/vapor/postgres-nio/pull/377?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=vapor#diff-U291cmNlcy9Qb3N0Z3Jlc05JTy9OZXcvRGF0YS9FeHByZXNzaWJsZUJ5UG9zdGdyZXNGbG9hdGluZ1BvaW50U3RyaW5nLnN3aWZ0) | `76.00% <76.00%> (ø)` | | ... and [2 files with indirect coverage changes](https://app.codecov.io/gh/vapor/postgres-nio/pull/377/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=vapor)
gwynne commented 1 year ago

It feels to me like this could be accomplished without a new protocol by making PostgresNumeric (which is already public) directly PostgresCodable; non-Foundation decimal types would then be able to more easily conform to PostgresCodable themselves using it. But I could easily be missing something. @fabianfett, any thoughts?

ratranqu commented 1 year ago

It feels to me like this could be accomplished without a new protocol by making PostgresNumeric (which is already public) directly PostgresCodable; non-Foundation decimal types would then be able to more easily conform to PostgresCodable themselves using it. But I could easily be missing something. @fabianfett, any thoughts?

My gut feel is that as long as PostgresNumeric (an exact type) allows itself to be represented on the Swift side by Floats or Doubles (non exact) as well as Decimal/exact types, it will be difficult to allow for that since Float/Double have their own PostgresCodable specificities?