paupino/rust-decimal

Support for sqlite: Here or in rusqlite?

Closed this issue · 4 comments

mamcx commented

I wanna add support for rust_decimal to sqlite (I use it for financial data) and contribute a pull request, and see that PG support is added here.

What could be best? Adding it on rusqlite or putting it in here?

I'd probably zoom out and ask where the most natural place to include the feature would be. Given that rusqlite is feature driven for date/time/json/url etc I'd probably suggest it makes the most sense as an additional feature within that project.

It's actually a common question I've been asking myself lately for some of the db stuff. When this library was younger and less stable it made sense to include the bridging logic here, however now that it has matured a bit it could make sense to migrate some of that logic into the consuming repos.

Happy to help put something together by the way. Let me know if you need any help.

mamcx commented

I send a pull request, in case you have any suggestions to make it more robust.

When this library was younger and less stable it made sense to include the bridging logic here, however now that it has matured a bit it could make sense to migrate some of that logic into the consuming repos.

This is a big part of why I'm opposed to adding it into rusqlite too, FWIW. I don't regret the existing library support that badly (although for a while chrono really worried me...), but have a lot of hesitation for new ones.

I think it's fine for users to just convert on their own though, rather than either library having native support. That way if they need to change libraries or formats or whatever, then they can do so with a migration (whereas if I put it in rusqlite, it will likely have to stay there for a long time, since I don't want to release an update where people can no longer read something they put into their database in the previous version).