Repository navigation
docs: apply the style guide's "native" rules to timezones.md - #7477
Open
Abhishek-Mohan wants to merge 4 commits into
Open
Abhishek-Mohan wants to merge 4 commits into
Abhishek-Mohan wants to merge 4 commits into
Conversation
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
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
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Which issue does this PR close?
Part of #7358. This PR moves
contributor-guide/timezones.mdover; 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 undernative/or the JNI boundary (N3).timezones.mdwas 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:N3date_formatruns natively only in UTC sessionsto_charfordate_formatonly in UTC sessions. Withspark.comet.expression.DateFormatClass.allowIncompatible=trueit usesto_charin other sessions too, and the result is formatted in UTCN1,P16Tz::from_stracceptsN3getTzdataVersion)getTzdataVersion)N3N1,P16make_timestamp, andto_timestampwith a format, always use the dispatcherN1,P16N1N1The
date_formatrow also fixes a claim:CometDateFormat.convertusesto_charoutside UTC when the expression'sallowIncompatibleconfig is set. It passes no timezone toto_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'sparquet.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
CometMakeTimestampandCometGetTimestampextendCometCodegenDispatchand always dispatch. Spark rewritesto_timestampwithout a format to aCast, 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.
CometDateFormatandCometConf.getExprAllowIncompatConfigKey(spark/.../serde/datetime.scala,CometConf.scala),CometTimeZone.nativeId,NativeBase.getTzdataVersionwith its Rust side innative/core/src/lib.rs,CometMakeTimestampandCometGetTimestamp, and Spark'sParseToTimestampreplacement.delta_spark_scan.rsanddatetime_rebase.rs, and theCastrouting againstCometCast.getSupportLevelandQueryPlanSerde.review-comet-prandreview-comet-docsskills on this PR, andreview-comet-docsin 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 --checkpasses on the changed page.-n) at the base and at the head: 32 warnings on each, none new, none on this page.Council review
Rounds: 1
Three models reviewed the PR independently against the style guide and the code. Their findings are fixed in the last commit: what the
date_formatopt-in does, the scope of the rebase list, and theCastsentence 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.