Add proper support for Decimal

[madsodgaard]

Fixes an issue when trying to decode/encode numeric columns as Decimal, that would result in an error such as: server: column "balance" is of type numeric but expression is of type text (transformAssignedExpr).

[fabianfett]

Hi @madsodgaard,

generally I'm open to accept this change. However there are a number of things that I would like to see, if we want to continue:

1. Also add conformance of Decimal to PSQLEncodable and PSQLDecodable
2. Unit tests (locally confirming that encoding and decoding to the raw value works)
3. Integration tests (sending a bind payload with the value and retrieving it)

I'm happy to help at every step of the way, if you need help :)

[madsodgaard]

@fabianfett I have added conformance to PSQLCodable and some unit tests and a integration test, let me know if anything is missing or can be improved

[fabianfett]

LGTM. Generally we should probably rework PostgresNumeric in the future... But let's ask @gwynne for a review as well!

[gwynne]

I'm concerned about the change in the public API behavior - will this break code at runtime for people who are currently storing Decimals in TEXT columns? Can we add a test or two which explicitly demonstrates this scenario, for both reading (select) and writing (insert/update)? (Ideally we'd have the test both here at the raw Postgres level operating directly on these API and also in FluentBenchmarker ensuring the behavior is stable for the common usage.)

[madsodgaard]

@gwynne It should not. I added a integration test to this library for testing string serialization. FluentBenchmark actually already contains a test for this in: PerformanceTests. That is not the most explicit place for asserting this behaviour though.

[codecov-commenter]

Codecov Report

Merging #194 (1e8523f) into main (f91f23d) will increase coverage by 6.08%.
The diff coverage is 69.69%.

@@            Coverage Diff             @@
##             main     #194      +/-   ##
==========================================
+ Coverage   36.13%   42.21%   +6.08%     
==========================================
  Files         115      116       +1     
  Lines        7788     7826      +38     
==========================================
+ Hits         2814     3304     +490     
+ Misses       4974     4522     -452     

Flag	Coverage Δ
unittests	42.21% <69.69%> (+6.08%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
...ources/PostgresNIO/Data/PostgresData+Decimal.swift	0.00% <0.00%> (ø)
...ces/PostgresNIO/New/Data/Decimal+PSQLCodable.swift	74.19% <74.19%> (ø)
...es/PostgresNIO/New/PSQLBackendMessageDecoder.swift	94.07% <0.00%> (+0.74%)	⬆️
...urces/PostgresNIO/New/Data/Array+PSQLCodable.swift	94.94% <0.00%> (+1.01%)	⬆️
...rces/PostgresNIO/New/Extensions/Logging+PSQL.swift	19.60% <0.00%> (+1.42%)	⬆️
Sources/PostgresNIO/New/PSQLConnection.swift	31.55% <0.00%> (+1.94%)	⬆️
...s/PostgresNIO/Message/PostgresMessageDecoder.swift	95.45% <0.00%> (+2.27%)	⬆️
Sources/PostgresNIO/New/Data/Int+PSQLCodable.swift	20.26% <0.00%> (+2.61%)	⬆️
...urces/PostgresNIO/New/Messages/ErrorResponse.swift	32.30% <0.00%> (+3.07%)	⬆️
...urces/PostgresNIO/New/Data/Float+PSQLCodable.swift	76.66% <0.00%> (+3.33%)	⬆️
... and 17 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f91f23d...1e8523f. Read the comment docs.

[gwynne]

Okay, I'm sufficiently satisfied that this is backwards-compatible - I'm not 100%, but I'm mindful of being a bit paranoid, and given that text-column round-tripping still works, I can't think of any actual objection that warrants holding back this long-overdue missing functionality 🙂

[gwynne]

Merging as semver-minor - though there are no new public APIs, just in case my paranoia isn't completely pointless it gives just a tiny bit of extra wiggle room 🤷‍♀️

[gwynne]

Thanks again for the excellent work on this, @madsodgaard!

[VaporBot]

These changes are now available in 1.7.0