Skip to content

Retention SET POLICY AT TIME ZONE (Spark 3.5 SQL surface) - #735

Open
mkuchenbecker wants to merge 7 commits into
mkuchenbecker/retention-timezone-supportfrom
mkuchenbecker/retention-timezone-grammar
Open

mkuchenbecker wants to merge 7 commits into
mkuchenbecker/retention-timezone-supportfrom
mkuchenbecker/retention-timezone-grammar

Conversation

@mkuchenbecker

@mkuchenbecker mkuchenbecker commented Sep 17, 2026

Copy link
Copy Markdown
Collaborator

Stack

Part of a stacked change (listed base first):

  1. Add optional time zone to retention policy retention job #732: retention time zone, backend core
  2. Retention SET POLICY AT TIME ZONE (Spark 3.5 SQL surface) #735: retention time zone, Spark 3.5 SQL configuration surface (this PR)

Based on #732. (#733 was merged into #732.)


Summary

Adds the SQL syntax for setting a retention time zone, so a table owner can configure it in place instead of through the REST policy object:

ALTER TABLE db.t SET POLICY (RETENTION=30d WITH TIMEZONE 'America/Los_Angeles');

The zone is an IANA id such as America/Los_Angeles or a fixed offset such as +05:30, and it works with or without a column pattern. The backend that measures retention in the zone is #732.

Changes

  • Client-facing API Changes
  • Internal API Changes
  • Bug Fixes
  • New Features
  • Performance Improvements
  • Code Style
  • Refactoring
  • Documentation
  • Tests

Spark 3.5 extension only. The grammar gains an optional WITH TIMEZONE '<zone>' clause on the retention policy; WITH and TIMEZONE are non-reserved, so they stay usable as identifiers. The parser carries the zone through the SetRetentionPolicy plan, and execution rejects a zone that Java's ZoneId cannot resolve, then writes the policy (including the zone) as JSON with a JSON library. A statement with no clause writes the same policy as before.

Testing Done

  • Added new tests for the changes made.

Java 17 integration test. SetRetentionTimeZoneStatementTest runs SET POLICY ... WITH TIMEZONE through the OpenHouse catalog against the embedded tables service, reads the stored policy back, and parses it into the typed policy model to check the zone round-trips. It covers an IANA zone, a fixed offset, the no-zone case, and rejection of an unresolvable zone.

Additional Information

  • Breaking Changes
  • Deprecations
  • Large PR broken into smaller PRs, and PR plan linked in the description.

Based on #732 (backend core); #733 (zoned integration tests) was merged into #732. Scoped to the Spark 3.5 extension.

🤖 Generated with GitHub Copilot CLI

@mkuchenbecker
mkuchenbecker force-pushed the mkuchenbecker/retention-timezone-dml branch from 1116db2 to 30a3116 Compare September 17, 2026 16:57
@mkuchenbecker
mkuchenbecker force-pushed the mkuchenbecker/retention-timezone-grammar branch from 1cabb2f to a45ec05 Compare September 17, 2026 17:00
@mkuchenbecker
mkuchenbecker force-pushed the mkuchenbecker/retention-timezone-dml branch from 30a3116 to d79adb8 Compare September 17, 2026 21:25
@mkuchenbecker
mkuchenbecker force-pushed the mkuchenbecker/retention-timezone-grammar branch from 64c083b to b1e31eb Compare September 17, 2026 21:25
mkuchenbecker added a commit that referenced this pull request Sep 17, 2026
…th a library

- Rename the SQL clause from AT TIME ZONE to WITH TIMEZONE.
- Reject an unresolvable time zone at the SET POLICY boundary with a clear error
  instead of silently persisting a policy the retention job cannot evaluate.
- Build the retention policy JSON with a Jackson object writer, which escapes
  owner-supplied values, and drop the hand-written escaper.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@mkuchenbecker
mkuchenbecker force-pushed the mkuchenbecker/retention-timezone-grammar branch from b1e31eb to 11b2de5 Compare September 17, 2026 23:44
@mkuchenbecker
mkuchenbecker changed the base branch from mkuchenbecker/retention-timezone-dml to mkuchenbecker/retention-timezone-support September 17, 2026 23:44
val mapper = new ObjectMapper()
val retention = mapper.createObjectNode()
retention.put("count", count)
retention.put("granularity", granularity)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

make this function pure. No local member access.


/**
* Reject an invalid time zone at this boundary rather than persisting a policy the retention job
* cannot resolve. A table owner supplies the zone as free text in the SQL statement.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

document the unchecked exception in the signature. Explain why its unchecked ( run override doesn't have checked exceptions)

@@ -0,0 +1,119 @@
package com.linkedin.openhouse.spark.statementtest;

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you need a proper integration test setting and validating this through the api. Use actual Json parsing vs "contains" in this class.

mkuchenbecker added a commit that referenced this pull request Sep 18, 2026
…th a library

- Rename the SQL clause from AT TIME ZONE to WITH TIMEZONE.
- Reject an unresolvable time zone at the SET POLICY boundary with a clear error
  instead of silently persisting a policy the retention job cannot evaluate.
- Build the retention policy JSON with a Jackson object writer, which escapes
  owner-supplied values, and drop the hand-written escaper.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@mkuchenbecker
mkuchenbecker force-pushed the mkuchenbecker/retention-timezone-grammar branch from 11b2de5 to 984f01f Compare September 18, 2026 18:30
@mkuchenbecker mkuchenbecker changed the title Retention SET POLICY AT TIME ZONE (Spark 3.5 SQL surface, stacked on #733) Retention SET POLICY AT TIME ZONE (Spark 3.5 SQL surface) Sep 18, 2026
mkuchenbecker added a commit that referenced this pull request Sep 18, 2026
- Move retentionPolicyJson and validateTimeZone to the companion object as pure
  functions that read only their arguments, no instance-member access.
- Document that validateTimeZone throws an unchecked IllegalArgumentException,
  and why it is unchecked: the inherited run override declares no checked
  exception, matching the unchecked rejection the sibling policy execs use.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
mkuchenbecker added a commit that referenced this pull request Sep 18, 2026
…yped JSON

Rewrite the retention time-zone test as an OpenHouseSparkITest that runs SET
POLICY through the OpenHouse catalog against the embedded tables service, then
reads the stored policy back and parses it into the typed Policies model to
assert the zone round-trips, instead of asserting on a substring of the raw
property. Covers an IANA zone, a fixed offset, the no-zone case, and rejection
of an unresolvable zone.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
mkuchenbecker added a commit that referenced this pull request Sep 18, 2026
…th a library

- Rename the SQL clause from AT TIME ZONE to WITH TIMEZONE.
- Reject an unresolvable time zone at the SET POLICY boundary with a clear error
  instead of silently persisting a policy the retention job cannot evaluate.
- Build the retention policy JSON with a Jackson object writer, which escapes
  owner-supplied values, and drop the hand-written escaper.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
mkuchenbecker added a commit that referenced this pull request Sep 18, 2026
- Move retentionPolicyJson and validateTimeZone to the companion object as pure
  functions that read only their arguments, no instance-member access.
- Document that validateTimeZone throws an unchecked IllegalArgumentException,
  and why it is unchecked: the inherited run override declares no checked
  exception, matching the unchecked rejection the sibling policy execs use.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
mkuchenbecker added a commit that referenced this pull request Sep 18, 2026
…yped JSON

Rewrite the retention time-zone test as an OpenHouseSparkITest that runs SET
POLICY through the OpenHouse catalog against the embedded tables service, then
reads the stored policy back and parses it into the typed Policies model to
assert the zone round-trips, instead of asserting on a substring of the raw
property. Covers an IANA zone, a fixed offset, the no-zone case, and rejection
of an unresolvable zone.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@mkuchenbecker
mkuchenbecker force-pushed the mkuchenbecker/retention-timezone-grammar branch from 2d12ff9 to 7c5686b Compare September 18, 2026 20:24
mkuchenbecker added a commit that referenced this pull request Sep 18, 2026
…th a library

- Rename the SQL clause from AT TIME ZONE to WITH TIMEZONE.
- Reject an unresolvable time zone at the SET POLICY boundary with a clear error
  instead of silently persisting a policy the retention job cannot evaluate.
- Build the retention policy JSON with a Jackson object writer, which escapes
  owner-supplied values, and drop the hand-written escaper.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
mkuchenbecker added a commit that referenced this pull request Sep 18, 2026
- Move retentionPolicyJson and validateTimeZone to the companion object as pure
  functions that read only their arguments, no instance-member access.
- Document that validateTimeZone throws an unchecked IllegalArgumentException,
  and why it is unchecked: the inherited run override declares no checked
  exception, matching the unchecked rejection the sibling policy execs use.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
mkuchenbecker added a commit that referenced this pull request Sep 18, 2026
…yped JSON

Rewrite the retention time-zone test as an OpenHouseSparkITest that runs SET
POLICY through the OpenHouse catalog against the embedded tables service, then
reads the stored policy back and parses it into the typed Policies model to
assert the zone round-trips, instead of asserting on a substring of the raw
property. Covers an IANA zone, a fixed offset, the no-zone case, and rejection
of an unresolvable zone.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@mkuchenbecker
mkuchenbecker force-pushed the mkuchenbecker/retention-timezone-grammar branch from 7c5686b to a0e31bc Compare September 18, 2026 21:16
mkuchenbecker and others added 3 commits September 18, 2026 14:41
Extend the OpenHouse Spark 3.5 SQL extension so a table owner can configure the
retention time zone from SQL:

  ALTER TABLE db.t SET POLICY (RETENTION=30D AT TIME ZONE 'America/Los_Angeles')

The grammar gains an optional AT TIME ZONE STRING on the retention clause (AT,
TIME, ZONE are non-reserved so they remain valid identifiers). The AST builder
threads the zone through the SetRetentionPolicy plan and the exec serializes it
into the retention policy JSON as "timeZone":"...". An absent clause omits the
field, so existing statements are unchanged.

Tests: SetRetentionTimeZoneStatementTest (spark-3.5 itest) asserts the stored
policy JSON for an IANA zone, a fixed offset, a zone with a column pattern, and
the no-zone case.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The Spark 3.5 SET POLICY execution built the retention policy JSON by
interpolating the time zone, column name, and column pattern directly into
the string. A value containing a double quote or backslash could break out of
the JSON string and inject additional policy fields.

Escape those free-text values per the JSON string grammar before embedding
them, so a crafted value is stored as an inert literal that the server-side
validators then accept or reject on its own merits.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Address pedantic review nits in the new statement test: replace the manual
StringBuilder loop in the property accessor with a stream pipeline, declare the
checked setup failure with throws Exception instead of hiding it behind
@SneakyThrows, and drop the redundant null initializer on the session field.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
mkuchenbecker and others added 4 commits September 18, 2026 14:41
…th a library

- Rename the SQL clause from AT TIME ZONE to WITH TIMEZONE.
- Reject an unresolvable time zone at the SET POLICY boundary with a clear error
  instead of silently persisting a policy the retention job cannot evaluate.
- Build the retention policy JSON with a Jackson object writer, which escapes
  owner-supplied values, and drop the hand-written escaper.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Move retentionPolicyJson and validateTimeZone to the companion object as pure
  functions that read only their arguments, no instance-member access.
- Document that validateTimeZone throws an unchecked IllegalArgumentException,
  and why it is unchecked: the inherited run override declares no checked
  exception, matching the unchecked rejection the sibling policy execs use.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…yped JSON

Rewrite the retention time-zone test as an OpenHouseSparkITest that runs SET
POLICY through the OpenHouse catalog against the embedded tables service, then
reads the stored policy back and parses it into the typed Policies model to
assert the zone round-trips, instead of asserting on a substring of the raw
property. Covers an IANA zone, a fixed offset, the no-zone case, and rejection
of an unresolvable zone.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@mkuchenbecker
mkuchenbecker force-pushed the mkuchenbecker/retention-timezone-grammar branch from a0e31bc to 5a89a4a Compare September 18, 2026 21:41
@mkuchenbecker
mkuchenbecker marked this pull request as ready for review September 21, 2026 20:15

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant