Skip to content

docs: apply the style guide's "native" rules to timezones.md - #7477

Open
Abhishek-Mohan wants to merge 4 commits into
apache:mainfrom
Abhishek-Mohan:docs/7358-timezones-native
Open

Abhishek-Mohan wants to merge 4 commits into
apache:mainfrom
Abhishek-Mohan:docs/7358-timezones-native

Conversation

@Abhishek-Mohan

@Abhishek-Mohan Abhishek-Mohan commented Nov 4, 2026 •

Copy link
Copy Markdown
Contributor

Which issue does this PR close?

Part of #7358. This PR moves contributor-guide/timezones.md over; the other pages stay open in that issue.

Rationale for this change

The style guide from #7151 rules out "runs natively", "the native path" and similar phrases (N1), and, in the contributor guide, allows "native" before a noun only when it names something under native/ or the JNI boundary (N3). timezones.md was one of the pages named in the #7151 review as still using them.

What changes are included in this PR?

One page, docs/source/contributor-guide/timezones.md, eight wording changes:

Before After Rule
feeds another native expression feeds another Rust-implemented expression N3
date_format runs natively only in UTC sessions Comet uses to_char for date_format only in UTC sessions. With spark.comet.expression.DateFormatClass.allowIncompatible=true it uses to_char in other sessions too, and the result is formatted in UTC N1, P16
has no native spelling has no spelling that Tz::from_str accepts N3
compares the native version (getTzdataVersion) compares the native library's IANA database version (getTzdataVersion) N3
Two Spark settings are not handled natively Comet's native Parquet scan does not handle two Spark settings N1, P16
the cases the native path cannot handle the cases that Comet's Rust code cannot handle, adding that make_timestamp, and to_timestamp with a format, always use the dispatcher N1, P16
a native path gated on "the session is UTC" a Rust fast path gated on "the session is UTC" N1
Check that the expression ran natively Check that the expression ran in Rust N1

The date_format row also fixes a claim: CometDateFormat.convert uses to_char outside UTC when the expression's allowIncompatible config is set. It passes no timezone to to_char, and the input is labelled "UTC", so the result is formatted in UTC.

The settings row narrows the subject from Comet to the native Parquet scan, because the Delta scan does rebase legacy-calendar values (native/core/src/parquet/datetime_rebase.rs, and the user guide's parquet.md). The bullets under it are unchanged.

The dispatcher row also fixes a claim. The old text implied every listed expression has a Rust path, but CometMakeTimestamp and CometGetTimestamp extend CometCodegenDispatch and always dispatch. Spark rewrites to_timestamp without a format to a Cast, which runs in Rust when Comet supports the cast and the session timezone.

The last row says "in Rust" rather than "in the Comet pipeline" because the sentence contrasts the Rust implementation with codegen dispatch, which also stays in the Comet pipeline.

Uses the guide allows are unchanged: "native code", "native side", "native plan", "native kernel", "native scan", "native implementation", the JVM/native boundary, and expect_native (N2, N3, N6).

How are these changes tested?

Documentation only.

  • Checked each changed claim against the code at the branch head: CometDateFormat and CometConf.getExprAllowIncompatConfigKey (spark/.../serde/datetime.scala, CometConf.scala), CometTimeZone.nativeId, NativeBase.getTzdataVersion with its Rust side in native/core/src/lib.rs, CometMakeTimestamp and CometGetTimestamp, and Spark's ParseToTimestamp replacement.
  • Checked the Delta rebase claim against delta_spark_scan.rs and datetime_rebase.rs, and the Cast routing against CometCast.getSupportLevel and QueryPlanSerde.
  • Ran the review-comet-pr and review-comet-docs skills on this PR, and review-comet-docs in audit mode on the whole page. The remaining uses of "native" are ones the guide allows, the config key and the named files and symbols exist, and both issue links resolve.
  • prettier --check passes on the changed page.
  • Built the docs with Sphinx (-n) at the base and at the head: 32 warnings on each, none new, none on this page.

Council review

Rounds: 1

Reviewer Round 1
Claude Opus 5.5 8.5/10
Gemini 4 Argon 10/10
GPT-6 Astra 8/10

Three models reviewed the PR independently against the style guide and the code. Their findings are fixed in the last commit: what the date_format opt-in does, the scope of the rebase list, and the Cast sentence in this description.

AI assistance: written with Claude Opus 5.5 through OpenCode, and reviewed by Claude Opus 5.5, Gemini 4 Argon and GPT-6 Astra in T3 Code.

Replace the phrases the style guide rules out (N1, N3) with the term
each one means: "runs natively", "not handled natively", "the native
path", "a native path" and "ran natively" become what Comet does in
Rust, and "native expression", "native spelling" and "native version"
name the Rust expression, the `Tz::from_str` parser and the native
library's IANA database version.

The `date_format` sentence also said Comet uses `to_char` only in UTC
sessions. `CometDateFormat` also uses it in other sessions when
`spark.comet.expression.DateFormatClass.allowIncompatible=true`, so the
sentence now says so.

Part of apache#7358.

Generated-by: OpenCode
@github-actions github-actions Bot added the documentation Improvements or additions to documentation label Nov 4, 2026
The codegen dispatcher paragraph said the serdes route "the cases their
Rust implementation cannot handle" through the dispatcher. `make_timestamp`
(`CometMakeTimestamp`) and `to_timestamp` with a format (`GetTimestamp`,
`CometGetTimestamp`) have no Rust implementation: both extend
`CometCodegenDispatch` and always dispatch. `to_timestamp` without a
format is rewritten by Spark to a `Cast`, which runs in Rust. Say so.

Found with the review-comet-docs skill's judgment checks.

Part of apache#7358.

Generated-by: OpenCode
The `date_format` sentence said Comet uses `to_char` outside UTC
sessions when `allowIncompatible` is set, but not what that does.
`CometDateFormat.convert` emits `to_char` without a timezone, and every
`TimestampType` input is labelled "UTC", so the result is formatted in
UTC. Say so.

The list of Spark settings Comet does not handle said "Comet" as a
whole. The Delta scan does rebase legacy-calendar dates and timestamps
(`datetime_rebase.rs`, and `parquet.md`), so scope the list to the
native Parquet scan.

Part of apache#7358.

Generated-by: OpenCode
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant