Skip to content

feat(cubesql): Support parsing IANA timezones#11140

Merged
MazterQyou merged 1 commit into
masterfrom
cubesql/iana-timezones
Jun 22, 2026
Merged

feat(cubesql): Support parsing IANA timezones#11140
MazterQyou merged 1 commit into
masterfrom
cubesql/iana-timezones

Conversation

@MazterQyou

@MazterQyou MazterQyou commented Jun 22, 2026

Copy link
Copy Markdown
Member

Check List

  • Tests have been run in packages where changes have been made if available
  • Linter has been run for changed code
  • Tests for the changes have been added if not covered yet
  • Docs have been added / updated if required

Description of Changes Made

This PR bumps cube-js/arrow-datafusion@1a849d7, adding support for parsing IANA timezones. Related test is included.

@MazterQyou MazterQyou requested review from a team as code owners June 22, 2026 18:09
@github-actions github-actions Bot added the rust Pull requests that update Rust code label Jun 22, 2026
@claude

claude Bot commented Jun 22, 2026

Copy link
Copy Markdown
Contributor

Claude finished @MazterQyou's task in 31s —— View job


Code Review

Verdict: LGTM — small, focused dependency bump with good test coverage. 0 high, 0 medium, 1 low.

Review details
  • Read PR diff
  • Inspect test added in rust/cubesql/cubesql/src/compile/mod.rs
  • Verify Cargo.toml/Cargo.lock changes
  • Check for existing review threads (none)

Summary

This PR bumps cube-js/arrow-datafusion from 1a75802e5859141d to pick up IANA timezone parsing (e.g., America/Los_Angeles) when casting string literals to TIMESTAMP. A new test test_where_filter_timezone in rust/cubesql/cubesql/src/compile/mod.rs:3073 validates that the timezone offset is correctly applied: a literal 2025-01-01 00:00:00 America/Los_Angeles is converted to 2025-01-01T08:00:00.000Z UTC before being sent to Cube. Lockfiles (packages/cubejs-backend-native/Cargo.lock, rust/cubesql/Cargo.lock) updated consistently.

Findings

Code Quality / Testing (low)rust/cubesql/cubesql/src/compile/mod.rs:3073

The test covers the happy path (a January timestamp → UTC-8). Consider also adding coverage for:

  • A DST-affected date (e.g., July, which would be UTC-7 for America/Los_Angeles) to lock in that DST is handled correctly, not just a fixed offset.
  • A non-North-American IANA zone (e.g., Europe/Berlin or Asia/Tokyo) to guard against region-specific regressions in the underlying datafusion parser.

These aren't blockers — the single test demonstrates the parsing works — but DST handling is a common source of subtle timezone bugs and worth pinning down.

Security / Performance

No concerns. Pure dependency bump + test addition; no new attack surface, no hot-path changes.

Docs

No user-facing surface changed (the SQL syntax was already accepted; only the value semantics for IANA names are now correct), so no docs update is required.

• Branch: [`cubesql/iana-timezones`](https://github.com/cube-js/cube/tree/cubesql/iana-timezones)

@codecov

codecov Bot commented Jun 22, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 79.12%. Comparing base (bd8b6e6) to head (4943021).
⚠️ Report is 3 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master   #11140      +/-   ##
==========================================
- Coverage   83.63%   79.12%   -4.51%     
==========================================
  Files         256      473     +217     
  Lines       79019    96538   +17519     
  Branches        0     3558    +3558     
==========================================
+ Hits        66085    76387   +10302     
- Misses      12934    19633    +6699     
- Partials        0      518     +518     
Flag Coverage Δ
cube-backend 58.74% <ø> (?)
cubesql 83.63% <100.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@MazterQyou MazterQyou force-pushed the cubesql/iana-timezones branch from 2d517d8 to 74feab7 Compare June 22, 2026 19:16
@claude

claude Bot commented Jun 22, 2026

Copy link
Copy Markdown
Contributor

Claude finished @MazterQyou's task in 39s —— View job


Code Review

Verdict: LGTM — dependency bump + targeted test. 0 high, 0 medium, 1 low.

Review details

Summary

This PR bumps cube-js/arrow-datafusion to 4da39d62f1bcd1ef33a0af337330e7536476fe8b (note: the PR description says 5859141 — the actual rev in rust/cubesql/cubesql/Cargo.toml:13 differs, likely a follow-up bump on the dep branch). The change picks up IANA timezone parsing so casts like CAST('2025-01-01 00:00:00 America/Los_Angeles' AS TIMESTAMP) resolve to the correct UTC instant.

A new test test_where_filter_timezone at rust/cubesql/cubesql/src/compile/mod.rs:3073 asserts that the literal is converted to 2025-01-01T08:00:00.000Z (UTC-8 for America/Los_Angeles in January) before reaching the Cube request. Lockfiles in both packages/cubejs-backend-native/Cargo.lock and rust/cubesql/Cargo.lock are updated consistently.

Findings

Testing (low)rust/cubesql/cubesql/src/compile/mod.rs:3073

The single test covers a fixed-offset January date. Worth considering (not blockers):

  • A DST-affected date (e.g., July → UTC-7 for America/Los_Angeles) to lock in DST handling rather than a static offset.
  • A non-North-American zone (e.g., Europe/Berlin, Asia/Tokyo) to guard against region-specific regressions in the underlying parser.

Security / Performance

No concerns — dependency bump + test only; no hot-path or attack-surface changes.

Docs

The accepted SQL syntax is unchanged (only the value semantics for IANA names are now correct), so no docs update needed.

Nit

PR description mentions rev 5859141 but the committed rev is 4da39d62…. Worth updating the description for traceability.

• Branch: [`cubesql/iana-timezones`](https://github.com/cube-js/cube/tree/cubesql/iana-timezones)

Signed-off-by: Alex Qyoun-ae <4062971+MazterQyou@users.noreply.github.com>
@MazterQyou MazterQyou force-pushed the cubesql/iana-timezones branch from 74feab7 to 4943021 Compare June 22, 2026 20:31
@claude

claude Bot commented Jun 22, 2026

Copy link
Copy Markdown
Contributor

Claude finished @MazterQyou's task in 33s —— View job


Code Review

Verdict: LGTM — dependency bump + targeted test. 0 high, 0 medium, 1 low.

Review details
  • Read PR diff (git diff origin/master...HEAD)
  • Inspect rust/cubesql/cubesql/Cargo.toml rev change
  • Inspect new test in rust/cubesql/cubesql/src/compile/mod.rs
  • Verify lockfile changes are consistent

Summary

This sync updates cube-js/arrow-datafusion rev to 1a849d723be7a13dfe4cbb83a574d5da735e885f (note: differs from the rev mentioned in the PR description and prior reviews — author appears to have rebased the dep branch). The downstream effect is unchanged: IANA timezone names (e.g. America/Los_Angeles) are now parsed correctly when casting string literals to TIMESTAMP.

The added test test_where_filter_timezone at rust/cubesql/cubesql/src/compile/mod.rs:3073 asserts that CAST('2025-01-01 00:00:00 America/Los_Angeles' AS TIMESTAMP) is normalized to 2025-01-01T08:00:00.000Z (UTC-8 in January) before being sent to Cube as a beforeOrOnDate filter. Lockfiles in packages/cubejs-backend-native/Cargo.lock and rust/cubesql/Cargo.lock are updated consistently.

Findings

Testing (low)rust/cubesql/cubesql/src/compile/mod.rs:3073

The single test covers a fixed-offset January date. Worth considering (not blockers):

  • A DST-affected date (e.g. July → UTC-7 for America/Los_Angeles) to lock in DST handling rather than a static offset.
  • A non-North-American zone (e.g. Europe/Berlin, Asia/Tokyo) to guard against region-specific regressions in the underlying parser.

Security / Performance

No concerns — dependency bump + test only; no hot-path or attack-surface changes.

Docs

The accepted SQL syntax is unchanged (only the value semantics for IANA names are now correct), so no docs update needed.

Nit

PR description says rev 5859141; committed rev is 1a849d72…. Worth syncing the description for traceability.

• Branch: cubesql/iana-timezones

@MazterQyou MazterQyou merged commit c79eb98 into master Jun 22, 2026
168 of 173 checks passed
@MazterQyou MazterQyou deleted the cubesql/iana-timezones branch June 22, 2026 21:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

rust Pull requests that update Rust code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant