fix(clickhouse): keep IAM-role S3 test TTL within toDateTime range#4434
Merged
Conversation
TestIAMRoleCanIssueSelectFromS3 set PEERDB_CLICKHOUSE_RAW_TABLE_TTL_DAYS to 36500 (100y) so the 2025-dated fixture rows wouldn't be evicted. After #4425 wrapped the raw-table TTL expression in toDateTime() (a 32-bit type capped at 2106-02-07), a 2025 base + 100y overflows that ceiling and wraps to a past date, so ClickHouse evicted the rows immediately and COUNT(*) returned 0 instead of 3. Lower the test override to 25000 days (~68y, ~2093) so it stays within the toDateTime range while still keeping the fixture rows. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Code reviewNo issues found. Checked for bugs and CLAUDE.md compliance. |
pfcoperez
approved these changes
Jun 16, 2026
pfcoperez
left a comment
pfcoperez
left a comment
Member
There was a problem hiding this comment.
LGTM, nit comment on the need of going that far in the future. 10/20 years are well over 90s TTL while still within the product expected lifetime.
| // Fixture rows have _peerdb_timestamp values from 2025; push the raw-table TTL far enough out that it doesn't | ||
| // evict them. The TTL expression wraps the timestamp in toDateTime() (a 32-bit type capped at 2106-02-07), so the | ||
| // 2025 base + INTERVAL must stay under that ceiling; 25000 days (~68y) lands around 2093, comfortably within range. | ||
| t.Setenv("PEERDB_CLICKHOUSE_RAW_TABLE_TTL_DAYS", "25000") |
Member
There was a problem hiding this comment.
Why don't we just add 10 years?
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
2 participants
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.
Problem
connectors/clickhouse/TestIAMRoleCanIssueSelectFromS3is failing onmain(e.g. this run). It loads a 3-row Avro fixture into a raw table and assertsCOUNT(*) == 3, but gets0— the rows are silently evicted by the table TTL.Root cause
#4425 changed the raw-table TTL expression to wrap the timestamp in
toDateTime():TTL toDateTime(fromUnixTimestamp64Nano(_peerdb_timestamp)) + INTERVAL N DAYtoDateTimeis a 32-bit type capped at 2106-02-07. The test deliberately setsPEERDB_CLICKHOUSE_RAW_TABLE_TTL_DAYS=36500(100 years) because the fixture rows carry 2025 timestamps and the default 90-day TTL would otherwise evict them. With the oldDateTime64expression2025 + 100y ≈ 2125was fine; withtoDateTimeit overflows the 2106 ceiling and wraps to a past date, so the TTL evaluates as already-expired and ClickHouse drops the rows →COUNT(*) = 0.The production default (90 days) never overflows, so this only affects the test's large override. The backward-compat
toDateTimefix from #4425 is left intact.Fix
Lower the test's TTL override from 36500 to 25000 days (~68y, lands around 2093), keeping it within the
toDateTimerange while still preventing eviction of the 2025 fixture rows. Added a comment explaining the 2106 ceiling.🤖 Generated with Claude Code