vbhanuchander-lang commented on PR #17613: URL: https://github.com/apache/iceberg/pull/17613#issuecomment-5274291425
Could a committer approve the pending workflow runs on this branch? They are all at `action_required`, so CI has not run yet. Locally the full `TestRecordConverter` suite passes (62 tests). cc @bryanck and @twthorn — small addition: `RecordConverter` handles every other temporal type but falls through to the "Cannot convert" default for `TIMESTAMP_NANO`, so a table with a `timestamp_ns` column cannot be written by the sink at all. One decision worth a reviewer's eye: for a numeric input I treat the value as **milliseconds**, matching the existing `case TIMESTAMP` branch and Connect's own `Timestamp` logical type, rather than as nanoseconds. Reading it as nanos would be defensible too, but it would make the same JSON payload mean different instants depending on the Iceberg column type. `Instant`, `OffsetDateTime`, `LocalDateTime` and string inputs are unaffected and keep full nanosecond precision. `testTimestampNanoConversionTreatsNumberAsMillis` pins that choice, so it is easy to flip if you would rather have the other convention. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
