Repository navigation
Conversation
| // Timestamp#toString uses the ISO 'T' separator, which the default | ||
| // partition time formatter cannot parse. Render it as the SQL string. | ||
| values[index] = | ||
| DateTimeUtils.formatTimestamp( |
There was a problem hiding this comment.
This handles TIMESTAMP_LTZ as if it were a timezone-free TIMESTAMP. The standard TimestampToStringCastRule formats LTZ values with TimeZone.getDefault(), so in a non-UTC timezone this can shift the extracted partition time-and even its date-relative to the logical/stored current-format partition value. Could we use the timezone-aware overload for TIMESTAMP_WITH_LOCAL_TIME_ZONE and add a regression test with a non-UTC default timezone?
JingsongLi
left a comment
There was a problem hiding this comment.
Requirement fit: SUPPORTED. Implementation: FINDINGS.
I independently confirmed the already-reported LTZ timezone problem in a real table: with Asia/Shanghai, current partition names, values-time expiration and one-day retention, an instant stored as local partition 2023-01-02 04:00:00 is deleted at local 2023-01-03 02:00:00, when it is only 22 hours old. Both that partition and a legitimately older control partition disappear. The otherwise identical NTZ/Shanghai and LTZ/UTC controls pass. This is a data-retention correctness blocker; please keep the LTZ conversion consistent with the existing timezone-aware cast. See the existing thread: #10851 (comment).
There is also one independent, baseline-verified formatter regression below. Normal JDK 8 core verify passes the 30 existing expiration/extractor cases. The extra head tests have the three failures described here; with both production files restored to merge-base eb4958c, the exact two custom-formatter cases pass. The worktree has been restored to the reviewed head.
| // Timestamp#toString uses the ISO 'T' separator, which the default | ||
| // partition time formatter cannot parse. Render it as the SQL string. | ||
| values[index] = | ||
| DateTimeUtils.formatTimestamp( |
There was a problem hiding this comment.
[P2] Preserve existing explicit timestamp formatter inputs
The unconditional SQL rendering changes the input of an existing configured partition.timestamp-formatter. A TIMESTAMP(0) partition with partition.timestamp-pattern=$f0 and formatter yyyy-MM-dd'T'HH:mm previously receives Timestamp.toString() values such as 2023-01-01T00:00, so expiration works. It now receives 2023-01-01 00:00:00; the configured extractor cannot parse it, and the expired partition is silently retained.
I reproduced actual writes, expiration and readback for January 1/January 5 partitions at a January 6 check with two-day retention. Both legacy and current partition-name modes retain the old partition on this head; the identical cases expire it successfully with the two production files restored to the PR merge-base. Please keep explicit formatter inputs compatible, or adapt timestamp extraction to the configured formatter, while retaining the stored names for deletion.
Partition expiration with the values-time strategy never expired a partition keyed by a TIMESTAMP or TIMESTAMP_LTZ column: the value was rendered with Timestamp#toString, whose ISO form (2023-01-01T00:00) the default partition time formatter cannot parse. Render such values as the SQL timestamp string for time extraction, and use the stored partition name when dropping them, as already done for DATE keys.
47635b1 to
f6a0c24
Compare
Purpose
Partition expiration with the values-time strategy never expired a partition keyed by a TIMESTAMP or TIMESTAMP_LTZ column: the value was rendered with Timestamp#toString, whose ISO form (2023-01-01T00:00) the default partition time formatter cannot parse. Render such values as the SQL timestamp string for time extraction, and use the stored partition name when dropping them, as already done for DATE keys.
Tests
Add UT