MaxGekk commented on PR #56739: URL: https://github.com/apache/spark/pull/56739#issuecomment-4796473316
@uros-b could you take another look when you get a chance? Summary of what changed since your review: - **`p=7` value round-trip nit** (addressed): added a `p=7` round-trip case for both NTZ and LTZ in `ArrowWriterSuite`. The value path packs the full nanosecond value regardless of column precision (precision lives in the Arrow field metadata, not the value), so it's mechanically identical to `p=9` today but guards the value path against a future precision-enforcing change. Commit `9572e4c`. - **`isSupportedByArrow` coordination** (replied, not changed here): agreed it's a coordination item rather than something to fix in this PR. Flagged that adding the two nanos cases to `isSupportedByArrow` alone isn't enough — in #56334 the Arrow-cache reader dispatch and the column-stats/min-max paths also omit the nanos types, so flipping only the gate would turn today's silent fallback into a hard failure. Since #56334 owns the method and the reader, the full nanos coverage is cleanest there (or a follow-up once both land); the types are also still feature-gated behind `spark.sql.timestampNanosTypes.enabled` and unreleased. - **Extra hardening**: also added an `ArrowUtilsSuite` case covering the `fromArrowField` fallback when the precision metadata key is present but invalid (out of `[7, 9]` or non-numeric) -> falls back to the canonical `p=9`, alongside the existing no-metadata case. Commit `2373e9c`. CI is green on the current head (`9572e4c`). Thanks for the careful review! -- 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]
