crystal-mysql icon indicating copy to clipboard operation
crystal-mysql copied to clipboard

Add support for MySql::Type::Decimal

Open steveaffleck opened this issue 1 year ago • 3 comments

Added support for MySql::Type::Decimal as BigDecimal in crystal.

steveaffleck avatar Apr 06 '23 14:04 steveaffleck

Thanks for contributing!

I think that the downside of using BigDecimal is that we will force a dependency on libgmp. Not 100% sure. It might also be a bit too much to use BigDecimal for MySQL decimals, but it's like we have a smaller alternative.

Possible outcomes:

  1. Check/Accept that we will have a libgmp dependency for apps using crystal-mysql. The decl_type will cause an entry in types_by_code so BigDecimal will always be required even if not used I think.
  2. Put the Decimal support backed with BigDecimal behind a feature flag or an optional require "mysql/big"
  3. Create a MySql::Decimal Number type that would be lighter than BigDecimal. This type could offer from/to methods for decimal and other types (at the expense of loosing precision). If the to_decimal are not used, then there is no dependency to libgmp.

Let's wait for other's feedback

bcardiff avatar Apr 06 '23 18:04 bcardiff

For my use case, option 3 is probably best. I can imagine there are cases where precision could be more important. It isn't something I run into often.

steveaffleck avatar Apr 06 '23 19:04 steveaffleck

Yeah, crystal-mysql should not depend on big (1). Support for DECIMAL can be optional (2) or implemented w/o BigDecimal (3). I'm fine with either. (3) Requires some implementation effort, while (2) requires an extra include when you want to use decimals. But both are manageable.

straight-shoota avatar Apr 06 '23 20:04 straight-shoota