andygrove opened a new pull request, #6686:
URL: https://github.com/apache/datafusion-comet/pull/6686
## Which issue does this PR close?
No issue closes. Part of #6335.
## Rationale for this change
#6337 added `timezones.md` before the fixes from the same audit landed. Four
of them changed behavior the page describes as current: #6347 (`CASE` and
`COALESCE` no longer panic on a timestamp branch with another label), #6351
(session timezone IDs are normalized before they reach native code), #6353 (a
warning when the native and JVM timezone databases differ), and #6349 (the
native CSV scan falls back for timestamps outside UTC). The page still says the
`CASE` casts panic, describes a `timeZoneId.getOrElse("UTC")` fallback that no
longer exists on `main`, and says native code cannot parse `GMT+8`, `Z` or
`PST`.
## What changes are included in this PR?
- "The invariant": `CASE`, `COALESCE` and `IF` now relabel a mismatched
timestamp branch (`coerce_branch` in `case_when.rs`), so they move from the
list of things that depend on the label to the places where a wrong label hides.
- "How the session timezone reaches native code": serdes go through
`CometTimeZone.nativeId`, an expression with no timezone gets `"UTC"`, and
native code reports an empty timezone as an error (`require_timezone`) instead
of asserting.
- "Parsing timezone IDs": which forms `nativeId` rewrites, and what happens
to an offset with seconds such as `+05:45:30`.
- "Timezone rules": the warning `NativeBase` logs when the tzdata versions
differ, with a pointer to the user guide section.
- "Scans": the native CSV scan's fallback.
- "The codegen dispatcher" and "Guidelines": a timezone native code cannot
express goes through the dispatcher, and timezones reach native code only
through `nativeId`.
- "Testing timezone-sensitive code": `session_timezone_ids.sql`, avoiding
dates that recent tzdata releases changed, and checking that an expression ran
natively rather than in the dispatcher, since a dispatched expression takes its
whole subtree with it. It drops "put it in a `CASE`", which no longer exposes a
wrong label.
#6340, which points the PR review skills at this page, is being rebased to
match.
## How are these changes tested?
This is a docs-only change. `prettier --check` passes. A local Sphinx build
gives the same warnings with and without the change. I checked each statement
against the code on `main`: `CometTimeZone.scala`, `case_when.rs`, `utils.rs`,
`NativeBase.java`, `CometScanRule.scala` and `CometScalaUDF.scala`.
--
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]
