-
Notifications
You must be signed in to change notification settings - Fork 12
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
added tests for decimal #790
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -563,15 +563,36 @@ var ( | |
}, | ||
"sum": { | ||
ValidateArgs: func(args []*types.DataType) (*types.DataType, error) { | ||
// per https://www.postgresql.org/docs/current/datatype-numeric.html#DATATYPE-NUMERIC-TABLE | ||
// the result of sum will be made a decimal(1000, 0) | ||
if len(args) != 1 { | ||
return nil, wrapErrArgumentNumber(1, len(args)) | ||
} | ||
|
||
if !args[0].EqualsStrict(types.IntType) { | ||
return nil, wrapErrArgumentType(types.IntType, args[0]) | ||
if !args[0].IsNumeric() { | ||
return nil, fmt.Errorf("expected argument to be numeric, got %s", args[0].String()) | ||
} | ||
|
||
return types.IntType, nil | ||
// we check if it is an unknown type before the switch, | ||
// as unknown will be true for all EqualsStrict checks | ||
if args[0] == types.UnknownType { | ||
return types.UnknownType, nil | ||
} | ||
|
||
var retType *types.DataType | ||
switch { | ||
case args[0].EqualsStrict(types.IntType): | ||
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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think so. In postgres, |
||
case args[0].EqualsStrict(types.Uint256Type): | ||
retType = decimal1000.Copy() | ||
default: | ||
panic(fmt.Sprintf("unexpected numeric type: %s", retType.String())) | ||
} | ||
|
||
return retType, nil | ||
}, | ||
IsAggregate: true, | ||
PGFormat: func(inputs []string, distinct bool, star bool) (string, error) { | ||
|
@@ -653,6 +674,17 @@ func defaultFormat(name string) FormatFunc { | |
} | ||
} | ||
|
||
// decimal1000 is a decimal type with a precision of 1000. | ||
var decimal1000 *types.DataType | ||
|
||
func init() { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I feel we should always put There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah I thought about it, but honestly I think it's more clear next to the decimal since it is what we are initializing. |
||
var err error | ||
decimal1000, err = types.NewDecimalType(1000, 0) | ||
if err != nil { | ||
panic(fmt.Sprintf("failed to create decimal type: 1000, 0: %v", err)) | ||
} | ||
} | ||
|
||
func errDistinct(funcName string) error { | ||
return fmt.Errorf(`%w: cannot use DISTINCT with function "%s"`, ErrFunctionSignature, funcName) | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess you're not using EqualsStrict because the metadata (precision and scale) isn't relevant?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Exactly. Regardless of what the precision and scale is, if it is any sort of decimal we want to set it to precision 1000