You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "abir-eon (via GitHub)" <gi...@apache.org> on 2026/07/13 20:21:24 UTC
[PR] fix(avro): decode nullable logical timestamps [arrow-go]
abir-eon opened a new pull request, #935:
URL: https://github.com/apache/arrow-go/pull/935
### Rationale for this change
This is a follow-up to https://github.com/apache/arrow-go/pull/832, which added support for Avro local timestamp logical types.
For nullable timestamp fields, Hamba decodes a selected logical union branch as a one-entry map such as `"long.local-timestamp-micros": time.Time`. `appendTimestampData` only inspected the plain `"long"` branch, so it treated valid non-null logical timestamp values as null.
### What changes are included in this PR?
- Recognize Hamba's exact union branch names for `timestamp-millis`, `timestamp-micros`, `local-timestamp-millis`, and `local-timestamp-micros`, while preserving plain `long` unions.
- Route decoded `time.Time` union values through the existing timestamp conversion, including UTC wall-clock reinterpretation for local timestamps.
- Add an OCF regression test with one non-null and one null nullable local timestamp, plus focused coverage for all four logical timestamp branches.
### Are these changes tested?
Yes. The regression failed before the fix because both records were appended as null. The following now pass:
- `go test ./arrow/avro/... -count=1`
- `golangci-lint v2.11.3 run --timeout=5m ./arrow/avro/...`
### Are there any user-facing changes?
Yes. Non-null values in nullable Avro timestamp logical fields are now decoded as Arrow timestamps instead of null.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(avro): decode nullable logical timestamps [arrow-go]
Posted by "zeroshade (via GitHub)" <gi...@apache.org>.
zeroshade commented on code in PR #935:
URL: https://github.com/apache/arrow-go/pull/935#discussion_r3580664006
##########
arrow/avro/reader_test.go:
##########
@@ -276,6 +277,92 @@ func TestOCFReaderBytesValues(t *testing.T) {
assert.True(t, nullable.IsNull(1))
}
+func TestOCFReaderNullableLocalTimestampMicros(t *testing.T) {
Review Comment:
Optional: this end-to-end OCF test only exercises `local-timestamp-micros`. `TestAppendTimestampDataUnionBranches` covers all four branch names at the appender level, but OCF-level coverage for nullable global `timestamp-millis`/`timestamp-micros` and `local-timestamp-millis` would also catch any integration mismatch in Hamba's branch naming/decoding for those flavors.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(avro): decode nullable logical timestamps [arrow-go]
Posted by "zeroshade (via GitHub)" <gi...@apache.org>.
zeroshade merged PR #935:
URL: https://github.com/apache/arrow-go/pull/935
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(avro): decode nullable logical timestamps [arrow-go]
Posted by "abir-eon (via GitHub)" <gi...@apache.org>.
abir-eon commented on code in PR #935:
URL: https://github.com/apache/arrow-go/pull/935#discussion_r3584387812
##########
arrow/avro/reader_test.go:
##########
@@ -276,6 +277,92 @@ func TestOCFReaderBytesValues(t *testing.T) {
assert.True(t, nullable.IsNull(1))
}
+func TestOCFReaderNullableLocalTimestampMicros(t *testing.T) {
Review Comment:
Addressed in c70bea62. TestOCFReaderNullableTimestamps now exercises timestamp-millis, timestamp-micros, local-timestamp-millis, and local-timestamp-micros end to end through OCF, with one non-null and one null record per type. Verified under TZ=UTC and TZ=Asia/Jerusalem. Thanks!
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org