lidavidm commented on code in PR #3787:
URL: https://github.com/apache/arrow-adbc/pull/3787#discussion_r2650043757
##########
c/driver/postgresql/copy/writer.h:
##########
@@ -234,48 +235,116 @@ class PostgresCopyNumericFieldWriter : public
PostgresCopyFieldWriter {
// Number of decimal digits per Postgres digit
constexpr int kDecDigits = 4;
std::vector<int16_t> pg_digits;
- int16_t weight = -(scale_ / kDecDigits);
- int16_t dscale = scale_;
- bool seen_decimal = scale_ == 0;
- bool truncating_trailing_zeros = true;
+ int16_t weight;
+ int16_t dscale;
Review Comment:
```suggestion
// "decimal scale". Number of digits after the decimal point (>=0)
// dscale may be more than the actual number of stored digits,
// implying there are significant zeroes that were not stored
int16_t dscale;
```
##########
c/driver/postgresql/copy/writer.h:
##########
@@ -241,17 +241,28 @@ class PostgresCopyNumericFieldWriter : public
PostgresCopyFieldWriter {
char decimal_string[max_decimal_digits_ + 1];
int total_digits = DecimalToString<bitwidth_>(&decimal, decimal_string);
- const int n_int_digits = total_digits > scale_ ? total_digits - scale_ : 0;
+ // Handle negative scale by appending zeros
+ int effective_scale = scale_;
+ if (scale_ < 0) {
+ int zeros_to_append = -scale_;
+ memset(decimal_string + total_digits, '0', zeros_to_append);
Review Comment:
```suggestion
std::memset(decimal_string + total_digits, '0', zeros_to_append);
```
nit
##########
c/driver/postgresql/copy/writer.h:
##########
@@ -234,48 +235,116 @@ class PostgresCopyNumericFieldWriter : public
PostgresCopyFieldWriter {
// Number of decimal digits per Postgres digit
constexpr int kDecDigits = 4;
std::vector<int16_t> pg_digits;
- int16_t weight = -(scale_ / kDecDigits);
- int16_t dscale = scale_;
- bool seen_decimal = scale_ == 0;
- bool truncating_trailing_zeros = true;
+ int16_t weight;
+ int16_t dscale;
char decimal_string[max_decimal_digits_ + 1];
- int digits_remaining = DecimalToString<bitwidth_>(&decimal,
decimal_string);
- do {
- const int start_pos =
- digits_remaining < kDecDigits ? 0 : digits_remaining - kDecDigits;
- const size_t len = digits_remaining < 4 ? digits_remaining : kDecDigits;
- const std::string_view substr{decimal_string + start_pos, len};
+ int total_digits = DecimalToString<bitwidth_>(&decimal, decimal_string);
+
+ // Handle negative scale by appending zeros
Review Comment:
Is there a way to handle scale without needing to append zeroes? The
Postgres decimal representation should store this separately, so I thought we
would only need to convert the integer part and then set dscale/weight
appropriately.
##########
c/driver/postgresql/copy/writer.h:
##########
@@ -234,48 +235,116 @@ class PostgresCopyNumericFieldWriter : public
PostgresCopyFieldWriter {
// Number of decimal digits per Postgres digit
constexpr int kDecDigits = 4;
std::vector<int16_t> pg_digits;
- int16_t weight = -(scale_ / kDecDigits);
- int16_t dscale = scale_;
- bool seen_decimal = scale_ == 0;
- bool truncating_trailing_zeros = true;
+ int16_t weight;
Review Comment:
```suggestion
// There are `weight + 1` base 10000 digits before the decimal point
// (may be negative)
int16_t weight;
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
