Skip to content
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

Decoder for numeric / decimal SQL Data type #7

Merged
merged 3 commits into from Jul 13, 2021

Conversation

Gustl22
Copy link
Contributor

@Gustl22 Gustl22 commented Jul 12, 2021

Closes stablekernel/postgresql-dart#50,
closes stablekernel/postgresql-dart#108,
closes stablekernel/postgresql-dart#112,
closes stablekernel/postgresql-dart#162,
Contributes to stablekernel/postgresql-dart#164

Encoder may follows, if I've time.
At least I wanted to give a decoding option to the users who need it.

I decided to use String as exact representation of BigDecimal. An alternative would be to use decimal package , but I don't know if its a good idea, to use third party package as datatype. If wanted the users can use the package and convert the string themselves with the Decimal.parse method, or use double.parse(), if precision is not that important.

I think BigInt isn't an option, as need to save the fractional digits and therefore would again need a new wrapper class.

I also implemented a less precise numericToDouble method, but I think we should stick to the String representation:

/// Decode numeric / decimal to double which may has loss in precision.
double decodeNumericToDouble(List<int> bytes) {
  final uint8List = Uint8List.fromList(bytes);
  final buffer = ByteData.sublistView(uint8List);
  final nDigits = buffer.getInt16(0); // non-zero digits, data buffer length = 2 * nDigits
  var weight = buffer.getInt16(2); // weight of first digit
  final signByte = buffer.getInt16(4); // NUMERIC_POS, NEG, NAN, PINF, or NINF
  final dScale = buffer.getInt16(6); // display scale
  if (signByte == 0xc000) return double.nan;
  final sign = signByte == 0x4000 ? -1 : 1;
  final prependBytes = 8;
  var decimal = 0.0;
  for (var i = prependBytes; i < (nDigits * 2 + prependBytes); i += 2) {
    decimal += buffer.getUint16(i) * pow(10000, weight);
    weight--;
  }
  return decimal * sign;
}

Copy link
Owner

@isoos isoos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this looks good, I have a small nit in the implementation. I don't think we need the lossy conversion to double, because the reason to use decimal/numeric is the need to preserve the precise number, without loosing precision.

Could you also make a note in the changelog about the new feature?

lib/src/binary_codec.dart Outdated Show resolved Hide resolved
@Gustl22
Copy link
Contributor Author

Gustl22 commented Jul 12, 2021

I made a note in the Changelog. If want to wait for the encoder, you can leave the PR open. Otherwise merging is fine, too :)

@isoos isoos merged commit d9c4ef6 into isoos:master Jul 13, 2021
@isoos
Copy link
Owner

isoos commented Jul 13, 2021

Thank you! Let me know if you want this to get released soon.

@Gustl22 Gustl22 deleted the 50-numeric-datatype branch July 13, 2021 06:15
@Gustl22
Copy link
Contributor Author

Gustl22 commented Jul 13, 2021

Thx! No hurry :) also can wait for the other PR

@MarkOSullivan94
Copy link

MarkOSullivan94 commented Jul 14, 2021

Thanks for adding support for numeric ❤️

What postgres release on pub will have this available in it?

Also off-topic but would it not be better having another repo for this package so people can report issues? Or is it recommended to report issues on the StableKernal repo @isoos?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants