SEZ9 commented on PR #12802: URL: https://github.com/apache/seatunnel/pull/12802#issuecomment-6567516225
Thanks @fatmanverse for checking this against the EasyExcel and POI behaviour before changing anything, and for pushing 8972a5024f. **On point 1 (unbounded `BigDecimal` / `toPlainString()`):** the approach you describe — `toDecimalText` writing the plain text only when its length fits an Excel text cell and otherwise falling back to `BigDecimal#toString()`, with `ExcelGeneratorTest#testDecimalWithLargeExponentIsWrittenAsScientificText` covering `1E+2147483647` — sounds like the right shape. I have not yet gone through the diff, so I will keep the finding open until I have. Two things I will look for, and it would help if you could confirm them: 1. The length guard is computed from precision/scale and never calls `toPlainString()` itself, so the guard cannot materialise the plain string. 2. The test covers the boundary: a value whose plain text is just under the limit still goes out as plain text, and one just over falls back to `toString()`. I accept the argument for not adding a read-side precision/scale rejection. If `new BigDecimal(text)` is what the CSV, Text, JSON and XML readers already do, and the Excel reader already accepted values such as `123456789012345678` into `decimal(20, 4)` before this PR, then a hard failure here would be a behaviour change for Excel alone. **One thing I need from you:** your comment appears to be cut off at "The new scale", so I cannot see the rest of your reply, including anything you wrote for the remaining points (docs placement for the new numeric semantics, the `.xls` / EasyExcel `getOriginalNumberValue()` path, mixed number/text cells in a DECIMAL column, the declared scale on read, the upgrade-note wording, the `(BigDecimal) value` cast, and the test comment in `testPoiReadKeepsNumericPrecision`). Could you repost the missing part? Once I can read it I will review 8972a5024f against those points. <!-- streview-comment:2205 --> -- 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]
