fatmanverse commented on PR #12802: URL: https://github.com/apache/seatunnel/pull/12802#issuecomment-6620532501
Thanks @SEZ9. Here is the status of each item on the current head 27340ef916 (all were first answered in https://github.com/apache/seatunnel/pull/12802#issuecomment-6557065213; line numbers are at this head): 1. **Bounding `new BigDecimal(cellText)` / `toPlainString()`**: fixed on the sink side, read side deferred. `ExcelGenerator#toDecimalText` (ExcelGenerator.java:252) calls `toPlainString()` only when `plainTextLength(value)` (:260), computed from precision and scale without building the text, fits Excel's 32767-character cell; otherwise it writes `BigDecimal#toString()`. `1E+2147483647` never reaches `toPlainString()` (`ExcelGeneratorTest#testDecimalWithLargeExponentIsWrittenAsScientificText`, and `#testDecimalPlainTextLengthLimit` pins both sides of the limit). On the read side, `toDeclaredScale` checks the declared precision before it rescales, so it never expands such a value either. I did not add a rejection, because the CSV/Text/JSON/XML readers parse DECIMAL the same way. 2. **EasyExcel `.xls`**: no change needed. In EasyExcel 4.0.3 the `.xls` `NumberRecordHandler` and `FormulaRecordHandler` also set `originalNumberValue` (via `ReadCellData.newInstanceOriginal` / `setOriginalNumberValue`), and POI converts RK/MulRK records to `NumberRecord` before EasyExcel sees them. `ExcelReadStrategyTest#testXlsReadKeepsNumericPrecision` (:829) reads an `.xls` file with both engines. 3. **Upgrade note vs POI FORMULA / EasyExcel**: it matches the code now. `case FORMULA` (ExcelReadStrategy.java:464-470) goes through the same `ExcelCellUtils#numberText(double, SqlType)` as `case NUMERIC`, which matches how EasyExcel reads the cached result. The note no longer says "keep their full stored value". It now describes each column type and spells out the formula case, e.g. `=0.1+0.2` reading as `0.30000000000000004` into a DECIMAL, with `ROUND(col, 2)` as the way to get the value the formula shows. 4. **Mixed number/text cells in a DECIMAL column**: documented. No new option for now. The sink doc (`docs/{en,zh}/connectors/sink/LocalFile.md`, after the Excel compression tip) explains when a DECIMAL becomes a text cell and gives the uniform layouts: `CAST(col AS DOUBLE)` for number cells, `CAST(col AS STRING)` for text cells. I'd rather add `excel_decimal_mode` in a follow-up if users ask for it, since an option is a permanent contract. 5. **Declared scale**: fixed. `ExcelCellUtils#toDeclaredScale` (:156) pads to the declared scale when that is lossless and fits the precision; it never rounds. `testSinkOutputRoundTripsDoubleAndDecimal` now uses `assertEquals` on the exact `BigDecimal` (`5000.0000`), and `testDecimalReadsWithDeclaredScaleWithoutRounding` (:685) covers padding, zero, the no-rounding case and huge exponents. 6. **Hard `(BigDecimal) value` cast**: fixed. A non-`BigDecimal` value goes through `new BigDecimal(value.toString())` (ExcelGenerator.java:180-182). 7. **Test comment in `testPoiReadKeepsNumericPrecision`**: reworded to "A text cell holding a decimal that a double cannot hold is parsed in full" (ExcelReadStrategyTest.java:635). 8. **Connector docs**: done in en and zh. The sink `LocalFile.md` covers number vs text cells, the source `LocalFile.md` (under `excel_engine`) covers the reading rules per column type. The latest fork Build failed only in `ActivemqIT.testSourceStreamingJob` (a timeout in an unrelated module); I've rerun it. -- 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]
