SEPURI-SAI-KRISHNA commented on PR #12909: URL: https://github.com/apache/seatunnel/pull/12909#issuecomment-6348626007
Thanks for the review, @DanielLeens. Both items are addressed and the branch is synced. **Sync.** You asked me to wait for #12875. It never landed: hutiefang76 closed it unmerged today, conflicting. The same repair reached `dev` via #12938 (`cc54bb433`), which also carried the `NOT_SINK_OPTIONS` exclusion in `JdbcOptionRuleCompletenessTest` for exactly the #12874 break. I ran the formerly failing test locally rather than trust the diff: `LLMBooleanOutputTest` passes 42 tests. **Issue 1.** You noted you had not run `enforcePrecisionScale`, so I measured it against the jars this module resolves, `orc-core` 1.5.6-nohive and `hive-storage-api` 2.6.0: ``` target decimal(10,2) unless noted 1.239 -> 1.24 123456789.99 -> NULL (rejected) 1.235 -> 1.24 12345678901.23 -> NULL (rejected) 1.234 -> 1.23 99999999.99 -> 99999999.99 -1.239 -> -1.24 1e39 (38,0) -> HiveDecimal.create() = null ``` Your reading is exactly right: fractional digits beyond the declared scale are rounded HALF_UP and away from zero, silently, and only an integer part that does not fit, or a value beyond ORC's maximum precision of 38, produces null. Both documents now carry a bullet saying so. **Issue 2.** Done, but the halves differ in kind and I would rather say so than imply two bugs. The missing `close()` was a real leak: it sat after `write()`, so the two failing cases threw past it and left the write strategy open. It now runs in a `finally`, with a close failure logged so it cannot replace the `FileConnectorException` under assertion. The fixed `/tmp` path I could not make fail. I reverted the helper, cleared `/tmp/seatunnel/orc` and ran twice: both passed. `getFileNamesByPath` globs `tmp_path`, and on the passing case `finishAndCloseFile` moves the file out to the final path, so the globbed directory is empty again next run; the two failing cases leave files but throw before the read back. The unique directory is still worth having, since it removes any dependence on what an aborted run or unrelated process left in `/tmp`. I just cannot claim it cures an observed failure. **Validation**, JDK 17: `OrcWriteStrategyTest` 5 tests 0 failures, and 5 again on an immediate second run with the first run's output on disk; full `connector-file-base` suite 356 tests 0 failures 0 errors; `spotless:check` and `-DskipTests verify -am` both clean. CI is in flight; I will report what it says rather than assume 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]
