HyukjinKwon commented on PR #56720: URL: https://github.com/apache/spark/pull/56720#issuecomment-4786340819
**Self-review (code-review skill, high effort) — posted as a comment, not an approval.** Scope: 1 file, +16/-0 (golden test resource only). Findings below. ### Correctness ✅ - The two appended blocks are **byte-identical** to the corresponding `TIME`-type blocks in the default `datetime-formatting.sql.out` (confirmed via `diff`). This is expected: `HH:mm:ss`/`HH:mm:ss.SSSSSS` formatting uses only numeric time fields, which are not locale/CLDR/JDK-sensitive, so the JDK 21 output equals the default. - Block count restored to **36** (matches the default golden and the input's query count), which is exactly what resolves `Expected 109, but got 103 blocks`. - The pre-existing JDK-specific divergence (the `3` vs `5` block earlier in the file) is untouched; the new blocks are appended after it, so they don't interact. - Verified green on JDK 21: `SQLQueryTestSuite -z datetime-formatting` → `Tests: succeeded 6, failed 0` (linked in the PR description). ### Minor note (process / `CLAUDE.md`) - The repo `CLAUDE.md` says: *"DO NOT edit the generated golden files (`.sql.out`) directly. Always regenerate them when needed."* This change hand-appends the two blocks rather than running `SPARK_GENERATE_GOLDEN_FILES=1`. Mitigation: the result is **verified equivalent** to a regeneration — the appended output is identical to the default golden and the suite passes on JDK 21. Happy to regenerate via the script instead if a reviewer prefers the canonical path. No correctness or behavioral concerns. Test-only change. -- 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] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
