Repository navigation
Conversation
INTEGER/FLOAT now output epoch seconds instead of milliseconds, and default_timestamp_format no longer appends a timezone offset, matching ruby's timestamp_converter and DEFAULT_TIMESTAMP_FORMAT. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…mp-conversion-match-ruby Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…havior Cover the review feedback on the timestamp conversion change: - INTEGER: sub-second values are truncated (999ms -> same second) and pre-1970 values are floored (-1ms -> -1), matching ruby's Time#to_i. - FLOAT: the fraction case now asserts an exact double, plus pre-1970 cases (-500ms -> -0.5, -1ms -> -0.001 as ruby's Time#to_f rounds once). - default_timezone: timestamp -> STRING/DATE/DATETIME, string -> TIMESTAMP, and nested RECORD TIMESTAMP fields fall back to task default_timezone when the column/field has no timezone, as ruby's `timezone || default_timezone` does. The -1ms FLOAT case and the default_timezone cases fail until the converter is updated; the rest already pass and only guard the behavior. TestBigqueryRecordConverter drops its @before fixture for a createTask() helper so a test can build a task with a different default_timezone. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
column_options[].timezone and fields[].timezone defaulted to "UTC" on the option itself, so task-level default_timezone was never read and timestamp -> STRING/DATE/DATETIME (and string/nested-field parsing) ignored it. Make timezone Optional and resolve `timezone || default_timezone` at every use, as ruby's ValueConverterFactory does. FLOAT conversion added epoch seconds and a nanosecond fraction as doubles, which rounds twice and differs from ruby's Time#to_f near the epoch (-1ms gave -0.0010000000000000009). Sum via BigDecimal and round once. README gains an upgrade note: how to restore the old offset-bearing STRING default, and that INTEGER/FLOAT milliseconds can't be restored. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The base INTEGER test now uses a sub-second value so it pins truncation on its own, replacing the separate truncation test; the pre-1970 floor test moves next to it. For FLOAT, the -1ms case already covers a negative value, so it takes over the beforeEpoch name and the redundant -500ms case is dropped. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…mp-conversion-match-ruby # Conflicts: # README.md
Follow-up to the ruby-compat/08 review: - Add BigqueryTimestampFormatters, which resolves the effective timezone and timestamp format (column option, else task default) in one place instead of seven copies across the three converters, and hands out TimestampFormatter instances cached per (pattern, timezone) so the page-processing hot path no longer rebuilds one per record per column. - Tolerate a null PluginTask consistently: the DATE/DATETIME/STRING branches now fall back to the ruby defaults (UTC, "%Y-%m-%d %H:%M:%S.%6N") the same way the RECORD branch already guards against a null task. - Restore the shared @before fixture in TestBigqueryRecordConverter and build the Asia/Tokyo task only in the one test that needs it. - Cover the default_timestamp_format default, the default_timezone fallback in the string converter's DATE and DATETIME branches, the null-task fallback, and the helper's precedence and caching. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
john-abre
changed the title
2 tasks done
Base automatically changed from
ruby-compat/08-timestamp-conversion-match-ruby
to
feature/ruby-compatibility
October 8, 2026 00:32
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.
#117 のレビュー指摘に対応するフォローアップです。ただし、
default_timezoneへのフォールバックに関する README のアップグレード注記は対象外としています（この挙動は ruby 版のValueConverterFactoryと一致しているため、そのままにしています）。変更内容
BigqueryTimestampFormatters.resolveTimezone()とresolveTimestampFormat()で、BigqueryTimestampConverter、BigqueryStringConverter、BigqueryRecordConverterに 7 箇所コピーされていたcolumnOption.getTimezone().orElse(task.getDefaultTimezone())を置き換えました。BigqueryTimestampFormatters.get()がTimestampFormatterのインスタンスをConcurrentHashMapにキャッシュします。フォーマッタはパターンとゾーンにのみ依存し、これらは実行中カラムごとに固定です。またTimestampFormatterの各実装はイミュータブル（RubyDateTimeFormatter/DateTimeFormatterを保持するfinalフィールドのみ）なので、スレッド間で共有しても安全です。taskを無条件に参照しないようにしました。task がない場合は ruby 版のデフォルト（UTC、%Y-%m-%d %H:%M:%S.%6N）にフォールバックし、RECORD 分岐がすでに行っている null ガードと挙動を揃えています。TestBigqueryRecordConverterの@Beforeフィクスチャを復元し、Asia/Tokyoの task を生成するのはtestConvertTimestampField_usesDefaultTimezoneのみにしました。default_timestamp_formatのデフォルト値、string converter の DATE / DATETIME 分岐でのdefault_timezoneフォールバック（出力側でフォールバックが観測できるよう、明示的な+0000オフセット付きの入力を使用）、null task 時のフォールバック、ヘルパーの優先順位とキャッシュに対するテスト / アサーションを追加しました。検証
./gradlew spotlessCheck test: 130 テスト、失敗 0、スキップ 1（既存のもの）。🤖 Generated with Claude Code