julianpeeters / avrohugger

Generate Scala case class definitions from Avro schemas
Apache License 2.0
201 stars 120 forks source link

Adds support for precision scale in decimals #102

Closed fedefernandez closed 5 years ago

fedefernandez commented 5 years ago

This PR adds support for typing the precision and scale in the decimals.

It uses shapeless for tagging (@@) and typing (Nat._2, ...) the numbers.

So, we go from something like this:

protocol MyProtocol {
  record MyRecord {
    decimal(9, 2) dec;
  }
}

to this:

case class MyRecord(dec: BigDecimal @@ (Nat._9, Nat._2))

All tests pass, but probably it deserves some kind of integration testing.

julianpeeters commented 5 years ago

Lookin good @fedefernandez! 2 Things: 1) let's make the shapeless dependency configurable (since it's a serde library concern, and since shapeless is a big dependency when some libraries won't need it). We can add ScalaBigDecimalWithPrecision (or ScalaShapelessBigDecimal or whatever you prefer) here, and tweak this method to check for a non-default type mapping. Then we can use the avroScalaCustomTypes setting in sbt-avrohugger to configure custom types, as usual.

Along the same lines, your work on SpecificRecord format is extremely valued, but if it doesn't require the use of type tags, then I wouldn't be opposed to punting on Specific, since (if I'm not mistaken) non-tagged BigDecimals seem to work fine.

2) lets add a serialization integration test to confirm what we generate can serde as we expect: here's where we test default decimal type for Standard, and I suppose there'd need to be a separate scripted project for the new custom decimal with precision, like this one for custom union: https://github.com/julianpeeters/sbt-avrohugger/blob/master/src/sbt-test/avrohugger/GenericSerializationTests/src/main/avro/logical.avdl https://github.com/julianpeeters/sbt-avrohugger/blob/master/src/sbt-test/avrohugger/GenericSerializationTests/src/main/avro/logical.avsc https://github.com/julianpeeters/sbt-avrohugger/blob/master/src/sbt-test/avrohugger/GenericSerializationTests/src/test/scala/test/standard/StandardPrimitivesSpec.scala#L103-L121

fedefernandez commented 5 years ago

Sounds like a plan, I'll work on it. Thanks

fedefernandez commented 5 years ago

Updated and created this other PR https://github.com/julianpeeters/sbt-avrohugger/pull/63

julianpeeters commented 5 years ago

Looking great @fedefernandez. This is now published as 1.0.0-RC14 up at sonatype, which should be syncing soon in maven central.