[Decimal] Fix Encoding Problem #734
Conversation
| @@ -9,15 +9,20 @@ import ( | |||
|
|
|||
| // EncodeDecimal is used to encode a string representation of a number to `org.apache.kafka.connect.data.Decimal`. | |||
| func EncodeDecimal(value string, scale int) ([]byte, error) { | |||
There was a problem hiding this comment.
We should probably make scale an int64.
There was a problem hiding this comment.
Really? I think if we were to change it, it should be like uint16
There was a problem hiding this comment.
PostgreSQL permits the scale in a numeric type declaration to be any value in the range -1000 to 1000. However, the SQL standard requires the scale to be in the range 0 to precision. Using scales outside that range may not be portable to other database systems.
There was a problem hiding this comment.
I think other databases would be similar. I can add a TODO here, if you think it's helpful
There was a problem hiding this comment.
Oh, are you mentioning this because we then don't need to convert on L17?
big.NewInt(int64(scale))There was a problem hiding this comment.
I just think that when it comes to mathematical operations we should avoid int and always try to use an integer with an explicit amount of bits (int16, int32, int64, etc). I would be fine with int16 here.
There was a problem hiding this comment.
I'm gonna add a TODO since it'll require refactoring our decimal type too.
Our previous library was susceptible to precision loss when converting from a
big.Floatinto abig.Int. Repro: https://go.dev/play/p/qg8NODRlxi5