added tests for decimal #790
Conversation
| switch { | ||
| case args[0].EqualsStrict(types.IntType): | ||
| retType = decimal1000.Copy() | ||
| case args[0].Name == types.DecimalStr: |
There was a problem hiding this comment.
I guess you're not using EqualsStrict because the metadata (precision and scale) isn't relevant?
There was a problem hiding this comment.
Exactly. Regardless of what the precision and scale is, if it is any sort of decimal we want to set it to precision 1000
| retType = decimal1000.Copy() | ||
| case args[0].Name == types.DecimalStr: | ||
| retType = args[0].Copy() | ||
| retType.Metadata[0] = 1000 // max precision |
There was a problem hiding this comment.
Is there any need to modify scale? Error if not already 0? I'm just guessing about the assumptions here.
There was a problem hiding this comment.
I don't think so. In postgres, sum will always maintain the input's scale. If we were multiplying or dividing, I would be a bit more concerned about scale, but for sum it really seems like we should just give as much precision as possible and keep the scale the same
parse/functions.go
Outdated
| case args[0].EqualsStrict(types.UnknownType): | ||
| retType = types.UnknownType.Copy() |
There was a problem hiding this comment.
EqualsStrict has this:
// if unknown, return true. unknown is a special case used
// internally when type checking is disabled.
if c.Name == unknownStr || other.Name == unknownStr {
return true
}So this can be true with any type?
There was a problem hiding this comment.
Oh jeez, that's a good catch. Will fix this
parse/functions.go
Outdated
| retType.Metadata[0] = 1000 // max precision | ||
| case args[0].EqualsStrict(types.Uint256Type): | ||
| retType = decimal1000.Copy() | ||
| case args[0].EqualsStrict(types.UnknownType): |
There was a problem hiding this comment.
Can you give an example what's a unknown numeric type?
There was a problem hiding this comment.
unknown is a special case that can be used for any type. I refactored this part though, as noted by Jon's bug above.
| // decimal1000 is a decimal type with a precision of 1000. | ||
| var decimal1000 *types.DataType | ||
|
|
||
| func init() { |
There was a problem hiding this comment.
I feel we should always put init at the very top, but I couldn't find a guide for this. nbd
There was a problem hiding this comment.
Yeah I thought about it, but honestly I think it's more clear next to the decimal since it is what we are initializing.
This adds more tests for decimal and properly handles return types from
sum, fixing a bug noted by Truflation trufnetwork/node#295.It also fixes a variety of other small bugs, such as failure to catch runtime errors for calculations done on numeric array types, and not enforcing precision and scale on final results returned to the client.