postgres-nio icon indicating copy to clipboard operation
postgres-nio copied to clipboard

Add support for decimal types other than Foundation.Decimal

Open ratranqu opened this issue 2 years ago • 3 comments

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)
    } 
}

ratranqu avatar Jul 27 '23 20:07 ratranqu

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
@@            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 Coverage Δ
...PostgresNIO/New/Data/Decimal+PostgresCodable.swift 100.00% <100.00%> (+25.00%) :arrow_up:
...ources/PostgresNIO/Data/PostgresData+Decimal.swift 0.00% <0.00%> (ø)
...ources/PostgresNIO/Data/PostgresData+Numeric.swift 67.78% <0.00%> (-3.58%) :arrow_down:
...ata/ExpressibleByPostgresFloatingPointString.swift 76.00% <76.00%> (ø)

... and 2 files with indirect coverage changes

codecov-commenter avatar Jul 27 '23 20:07 codecov-commenter

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?

gwynne avatar Jul 27 '23 20:07 gwynne

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?

ratranqu avatar Jul 27 '23 20:07 ratranqu