Add optional time zone to retention policy (backend core) - #731
Closed
mkuchenbecker wants to merge 2 commits into
Closed
mkuchenbecker wants to merge 2 commits into
mkuchenbecker wants to merge 2 commits into
Conversation
Retention policy gains an optional timeZone (IANA id or fixed offset). When set, the retention boundary is evaluated in that zone instead of UTC. The zone is applied to the boundary value, not the column, so the delete stays a metadata-only partition drop; native timestamp boundaries are snapped down to the UTC partition edge, and string-pattern boundaries are anchored to the zone. Absent zone reproduces today's UTC behavior exactly. - Retention API model: optional timeZone field. - RetentionPolicySpecValidator: reject a zone ZoneId cannot resolve. - SparkJobUtil.createDeleteStatement/createDeleteFilter: zone-aware boundary with inclusive-kept / exclusive-deleted semantics and partition-edge snapping. - Thread timeZone through Operations.runRetention, RetentionSparkApp (--timeZone), RetentionConfig, TablesClient, TableRetentionTask. - Unit tests for validator and delete-boundary (native snap, string anchor, filter micros, fractional-hour zone); existing tests keep UTC behavior. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
mkuchenbecker
commented
Sep 15, 2026
| return false; | ||
| } | ||
| if (!validateTimeZoneIfPresent(retention)) { | ||
| failureMessage = |
Collaborator
Author
There was a problem hiding this comment.
Time zone should require a SQL conf be set to enable for replicaiotn time zone support. This avoids an invalid time from impacting an existing table.
Review blocker: createDeleteStatement (SQL wall-clock INTERVAL) and createDeleteFilter (instant-based ZonedDateTime.minus for HOUR) computed different zoned string-partition boundaries across DST, so with backup enabled the manifests could certify a narrower range than the executed delete and delete a partition outside the backed-up range. Both paths now derive the boundary from one wall-clock helper (zonedStringBoundary), so they cannot diverge. Cleanups from the review: fail explicitly for unsupported granularity units instead of silently truncating to days; name the microsecond conversion; rename the local zoned to hasTimeZoneOverride; make the CLI and schema descriptions complete sentences. Tests: add a DST statement/filter consistency case and zoned MONTH and YEAR boundary cases; update the zoned string-pattern expectation to the shared literal. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Collaborator
Author
|
Superseded by #732. Recreated with the head branch on linkedin/openhouse instead of the personal fork. |
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.
Summary
Adds an optional
timeZoneto the retention policy. When set, the retention age boundary is evaluated in that zone instead of UTC, applied to the boundary value rather than the column so the delete stays a metadata-only partition drop; a policy with no zone is unchanged. This PR is the OpenHouse backend core, opened as a draft; the SQLAT TIME ZONEgrammar and the li-openhouse orchestration are follow-ups. The full design is below.Design
Overview
Today retention evaluates its age boundary only in UTC. This change adds an optional time zone: when set, retention evaluates the boundary in that zone, and a policy with no zone is unchanged.
Requirements
The time zone is a requirement input; its business rationale is out of scope.
America/Los_Angeles, or a fixed offset such as+05:30.Behavior
Set a zone with an
AT TIME ZONEclause, with or without a column pattern:The boundary is the start of the current period in the zone, moved back by the count: truncate the current time to the retention granularity, then subtract count periods. Retention keeps rows at or after the boundary and deletes rows before it, so the boundary is inclusive on the kept side and exclusive on the deleted side.
The kept window is the count most recent complete periods plus the current in-progress period, so it spans between count and count+1 periods. With 2-day retention one minute after local midnight, the current day is nearly empty, so retention keeps close to two full days; twelve hours later it keeps two days plus the half day so far. With hourly retention, each run that crosses a local hour advances the boundary by one hour and deletes the oldest kept hour.
Accuracy at the boundary depends on the column:
The boundary uses the zone's offset in effect on the boundary date, so an IANA zone tracks daylight saving and a fixed offset does not.
To adopt, add the clause to an existing policy. Nothing else changes.
Decisions that affect behavior
The zone applies to the boundary value, not the column; the native-timestamp boundary snaps to a partition edge. Both keep retention metadata-only, which the rejected alternatives do not.
Worked example
A daily table keeps 30 days in
America/Los_Angeles, run at2024-02-01T02:00Z, which is still January 31 in the zone. The boundary is local January 1 midnight,2024-01-01T08:00Z, snapped to2024-01-01T00:00Z, so the2024-01-01partition is kept. In UTC the run reads as February 1 and would drop it.Changes
New Features: an optional
timeZoneon the retention policy; the retention job evaluates its age boundary in that zone. Internal API Changes:SparkJobUtil.createDeleteStatementandcreateDeleteFilter,Operations.runRetention,RetentionSparkApp(--timeZone),RetentionConfig,TablesClient, andTableRetentionTaskgain atimeZoneparameter or field. Tests: added zone-aware unit tests; existing UTC tests are unchanged.Testing Done
Java 17 unit tests pass:
SparkJobUtilTest(9, including the native snap, the string anchor, the Iceberg filter micros, and a fractional-hour zone),RetentionPolicySpecValidatorTest(new time-zone validation plus existing),AppsTest, andTableRetentionTaskTest. A blank zone reproduces today's SQL and Iceberg expression byte for byte, so existing behavior is unchanged.Additional Information
This feature is delivered in layers: this PR is the OpenHouse backend core, followed by the SQL
AT TIME ZONEgrammar in the Spark extensions, then the li-openhouse orchestration (the LinkedIn retention app, Central Policy Store mapping, and emitted stats).🤖 Generated with GitHub Copilot CLI