SEZ9 commented on PR #12364:
URL: https://github.com/apache/seatunnel/pull/12364#issuecomment-5946646566
Thanks @goutamadwant — the `22ea3a7e7152` update sounds like the right shape
for what was left open here.
Going through the previous points against your latest comment:
- **PR12364-F1 (mixed integral/DECIMAL falling back to the double
comparator)** — your summary says integral wrappers combined with `BigDecimal`
now compare exactly, with the new regression red before the fix and green
after. I still need to confirm this against the diff in `63f001b74b1`; could
you point me to the test(s) covering the mixed case, and confirm whether the
earlier `{1L, 1.00000000000000000001}` case now asserts the exact extremum? You
also mention the unknown-Number fallback is unchanged — if `BigInteger` still
goes through the double path, that is fine for this PR as long as the Javadoc
and `sql-functions.md` say so explicitly rather than implying all
exact-precision numerics are exact. Can you confirm the wording?
- **PR12364-F3 (first-element tie retention relying on `Stream.max/min`)** —
you mention explicit first-element tie retention and an original-element
identity test, which is exactly what I was after. I will verify against the
changed files; a pointer to the test name would speed that up.
- **PR12364-F2 (incompatible-changes entry)** — you note the EN/ZH scope and
migration notes are updated. Please confirm the entry now carries the same
`Affected component` / `Migration Guide` structure as the neighbouring `dev`
entries and names the transforms module explicitly.
- **PR12364-F4 ("For the Zeta SQL engine" wording)** — covered by the same
docs update if the sentence no longer reads as Zeta-execution-engine-only; a
quick confirmation is enough.
- **PR12364-F5 (inline comment duplicating the `numericComparator`
Javadoc)** — not mentioned in your summary. Was the duplicate inline comment
dropped in `63f001b74b1`, or is it still there? If so, a one-line removal is
all that's needed.
Once the above is confirmed and CI on `22ea3a7e7152` has reported, I'm happy
to move this forward.
<!-- streview-comment:1469 -->
--
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]
