vlaurent1 commented on issue #4507:
URL: https://github.com/apache/arrow-adbc/issues/4507#issuecomment-4966583490

   > See [#3485](https://github.com/apache/arrow-adbc/issues/3485).
   > 
   > You(r agent) can test the nightly wheel to confirm?
   
   
   
   ## Tested nightly v1.12.0 on Windows — bug still present
   
   Tested `adbc-driver-postgresql` v1.12.0 (nightly from Gemfury) on Windows 
against PostgreSQL 18.4 with `decimal128(18, 6)` values. The bug is still there 
— same corruption pattern as the stable release.
   
   ### Results
   
   ```
   5000000000.000001  →  5000000100.000000  [CORRUPTED]
   1000000000.000001  →  1000000100.000000  [CORRUPTED]
   5000000000.100000  →  5010000000.0       [CORRUPTED]
   4999999999.000001  →  4999999999.000001  [OK]
   5000000001.000001  →  5000000001.000001  [OK]
   5000000000.000000  →  5000000000         [OK — display only]
   ```
   
   4 of 6 cases corrupted. Control values (integer part not a multiple of 10^9) 
round-trip correctly.
   
   ### Why it's still broken
   
   PR #3787 fixed the **fast path** (`WriteDecimal128Fast`) — it uses 128-bit 
integer arithmetic and produces correct results. But it's only enabled when 
`__SIZEOF_INT128__` is defined (GCC/Clang). Windows/MSVC falls back to the 
**slow path** (`WriteDecimal128Slow`), which builds the PostgreSQL binary 
format through string parsing and digit grouping.
   
   The bug is in `GroupIntegerDigits()` at 
`c/driver/postgresql/copy/writer.h:524-526`:
   
   ```cpp
   // Skip trailing zeros
   if (val != 0 || !digits.empty()) {
       digits.insert(digits.begin(), val);
   }
   ```
   
   It strips trailing zero base-10000 digit groups from the integer part. For 
`5000000000`, the correct digit groups are `[50, 0, 0]` (3 groups), but it 
returns `[50]` (1 group). This shifts the weight by 2 positions, moving 
fractional digits into the wrong place — so `5000000000.000001` becomes 
`5000000100.000000`.
   
   This means:
   
   - **Linux/macOS + DECIMAL128**: should pass (fast path)
   - **Windows + DECIMAL128**: fails (slow path, confirmed above)
   - **All platforms + DECIMAL256**: fails (always uses slow path)
   
   ### Suggested Fix 
   
   Remove the trailing-zero skip in `GroupIntegerDigits`. All base-10000 digit 
groups must be preserved — zero groups are needed to compute the correct weight 
for the fractional part. This is a small change and fixes the slow path for 
everyone.
   


-- 
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]

Reply via email to