-
Notifications
You must be signed in to change notification settings - Fork 37
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
Fix decoding numeric values #129
Conversation
lib/src/binary_codec.dart
Outdated
@@ -449,7 +453,7 @@ class PostgresBinaryDecoder<T> extends Converter<Uint8List?, T?> { | |||
return null; | |||
} | |||
|
|||
final dataType = typeMap[typeCode]; | |||
final dataType = type ?? typeMap[typeCode]; |
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.
Could this be merged with type
in the constructor?
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.
We could move the map lookup into the constructor, but not without dropping const
. I didn't want to do that since that might be breaking. If you think it's worth it, I'm happy to unify that in the constructor.
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 think it is worth it. The decoder is not part of the public API, so dropping const
seems to be fine...
Codecov ReportPatch coverage:
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## master #129 +/- ##
==========================================
+ Coverage 86.43% 86.51% +0.08%
==========================================
Files 29 29
Lines 2492 2492
==========================================
+ Hits 2154 2156 +2
+ Misses 338 336 -2
☔ View full report in Codecov by Sentry. |
PgDataType.numeric
is instantiated with theList<int>
type parameter. I think this is wrong, the parameter must have been taken from the comment by mistake.Looking at the implementation for
numeric
in the binary encoder and decoder, it seems like we support encoding these values from ints, doubles and strings. But we're always decoding them as strings. Since these types share no common interface, the type need to be declared asObject
to avoid theas T
in the decoder failing.I've also added a constructor directly taking a type to avoid the double lookup from and to oids.