Skip to content

fix(core): parse fixed-offset timezones in timestamp logical types - #8950

Open
jackylee-ch wants to merge 1 commit into
lance-format:mainfrom
jackylee-ch:fix/timestamp-fixed-offset-timezone
Open

fix(core): parse fixed-offset timezones in timestamp logical types#8950
jackylee-ch wants to merge 1 commit into
lance-format:mainfrom
jackylee-ch:fix/timestamp-fixed-offset-timezone

Conversation

@jackylee-ch

Copy link
Copy Markdown
Contributor

A timestamp column whose timezone is a fixed offset cannot be read back. The logical type writes the timezone verbatim, so Timestamp(Microsecond, "+08:00") is encoded as timestamp:us:+08:00, but the parser split that string on : and required exactly three segments. The offset contributes a fourth, so the parse failed and Field::data_type, which unwraps it, panicked instead of returning the error.

pyarrow produces this shape routinely: pa.timestamp("us", tz="+08:00"), and any pandas column localized to a fixed datetime.timezone carries +08:00 too. Named zones such as UTC and offsets written without a colon (+0800) were unaffected.

The fix rejoins the trailing segments, matching what the neighbouring fixed_size_list arm already does for the same reason. Only the read side changes; the encoding is untouched. The tests cover each time unit against every timezone spelling, both directions, and the field and schema round trips a write performs.

@github-actions github-actions Bot added the bug Something isn't working label Sep 2, 2026
@jackylee-ch
jackylee-ch force-pushed the fix/timestamp-fixed-offset-timezone branch from bce54f6 to c7b20f6 Compare September 2, 2026 14:02
lance-gatekeeper[bot]

This comment was marked as outdated.

@lance-gatekeeper lance-gatekeeper Bot added the K-approved Latest Gatekeeper recommendation permits acceptance. label Sep 2, 2026
@jackylee-ch
jackylee-ch force-pushed the fix/timestamp-fixed-offset-timezone branch from c7b20f6 to 6e948b8 Compare September 2, 2026 14:37
@lance-gatekeeper lance-gatekeeper Bot removed the K-approved Latest Gatekeeper recommendation permits acceptance. label Sep 2, 2026
lance-gatekeeper[bot]

This comment was marked as outdated.

@lance-gatekeeper lance-gatekeeper Bot added the K-approved Latest Gatekeeper recommendation permits acceptance. label Sep 2, 2026
The logical type writes a timestamp's timezone verbatim, so
Timestamp(Microsecond, "+08:00") is encoded as "timestamp:us:+08:00".
The parser split that string on ':' and required exactly three segments,
but the offset contributes a fourth, so the parse failed and
Field::data_type, which unwraps it, panicked instead of erroring.

Rejoin the trailing segments, matching what the neighbouring
fixed_size_list arm already does for the same reason. Only the read side
changes; the encoding is untouched.
@jackylee-ch
jackylee-ch force-pushed the fix/timestamp-fixed-offset-timezone branch from 6e948b8 to 919d153 Compare September 3, 2026 02:49
@lance-gatekeeper lance-gatekeeper Bot removed the K-approved Latest Gatekeeper recommendation permits acceptance. label Sep 3, 2026

@lance-gatekeeper lance-gatekeeper Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Gate recommendation: approve.

The source tree is unchanged from the previously reviewed revision, so the fixed-offset read-after-write compatibility result and its focused regression evidence remain valid.

@lance-gatekeeper lance-gatekeeper Bot added the K-approved Latest Gatekeeper recommendation permits acceptance. label Sep 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working K-approved Latest Gatekeeper recommendation permits acceptance.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant