shoemoney opened a new pull request, #2034:
URL: https://github.com/apache/iceberg-go/pull/2034
## Problem
`partitionTypeToAvroSchema` in `schema_conversions.go` has arms for Int32,
Int64, Float32, Float64, String, Date, Time, Timestamp, TimestampTz, UUID,
Bool, Binary, Fixed, Decimal and Unknown, but none for `TimestampNsType` or
`TimestampTzNsType`. Both fall through to:
```go
return nil, fmt.Errorf("unsupported partition type: %s", f.Type.String())
```
`NewManifestWriter` calls `partitionTypeToAvroSchema` before doing any other
work, so a v3 table partitioned on a `timestamp_ns` or `timestamptz_ns` column
cannot have its manifest written at all. This is a hard failure at writer
construction, not a subtle precision issue.
## Why this looks like an oversight rather than an intended restriction
The read side already handles these types:
- `manifest.go` decodes `atype.TimestampNanos` into `TimestampNano`.
- `catalog/rest/scan_task_decoder.go` `partitionLogicalType` maps
`iceberg.TimestampNsType, iceberg.TimestampTzNsType` to `atype.TimestampNanos`.
`TimestampNsType` and `TimestampTzNsType` are first-class types in
`types.go`, `transforms.go` produces nanosecond result types, and
`table/partitioned_fanout_writer.go`, `table/arrow_utils.go` and
`table/evaluators.go` all handle nanoseconds on the write and scan paths.
Nothing in the docs, tests or comments marks nanosecond partitions as
unsupported. The library can read a partition layout it cannot write.
## Reproduction
```go
sc := NewSchema(0, NestedField{ID: 1, Name: "ts", Type: TimestampNsType{},
Required: true})
spec := NewPartitionSpecID(0,
PartitionField{FieldID: 1000, SourceIDs: []int{1}, Name: "ts_identity",
Transform: IdentityTransform{}})
_, err := NewManifestWriter(3, io.Discard, spec, sc, 1)
// err: unsupported partition type: timestamp_ns
```
## Fix
Add `TimestampNsNode` and `TimestampTzNsNode` to `internal/avro_schemas.go`,
using `atype.TimestampNanos` with `adjust-to-utc` false and true respectively,
exactly mirroring the existing `TimestampNode` / `TimestampTzNode` micros pair,
and wire both nanosecond types into the `partitionTypeToAvroSchema` switch. The
logical type matches what the manifest read path and the REST scan task decoder
already expect, so the round trip is consistent.
## Tests
Two new table-driven tests in `schema_conversions_test.go`, both covering
`timestamp_ns` and `timestamptz_ns`:
- `TestNanosecondPartitionAvroSchema` builds the Avro schema and round trips
a value with a non-zero nanosecond remainder (`...456789123`), asserting no
precision loss. A micros-based logical type would drop the trailing `123`.
- `TestNewManifestWriterV3NanosecondPartition` constructs a v3 manifest
writer over an identity partition on a nanosecond column and asserts it
succeeds.
### Before (on unmodified source)
```
--- FAIL: TestNanosecondPartitionAvroSchema (0.00s)
--- FAIL: TestNanosecondPartitionAvroSchema/timestamp_ns (0.00s)
schema_conversions_test.go:307:
Error: Received unexpected error:
unsupported partition type: timestamp_ns
--- FAIL: TestNanosecondPartitionAvroSchema/timestamptz_ns (0.00s)
schema_conversions_test.go:307:
Error: Received unexpected error:
unsupported partition type: timestamptz_ns
--- FAIL: TestNewManifestWriterV3NanosecondPartition (0.00s)
--- FAIL: TestNewManifestWriterV3NanosecondPartition/timestamp_ns (0.00s)
schema_conversions_test.go:354:
Error: Received unexpected error:
unsupported partition type: timestamp_ns
--- FAIL: TestNewManifestWriterV3NanosecondPartition/timestamptz_ns
(0.00s)
schema_conversions_test.go:354:
Error: Received unexpected error:
unsupported partition type: timestamptz_ns
FAIL github.com/apache/iceberg-go 0.015s
```
### After
```
--- PASS: TestNanosecondPartitionAvroSchema (0.00s)
--- PASS: TestNanosecondPartitionAvroSchema/timestamp_ns (0.00s)
--- PASS: TestNanosecondPartitionAvroSchema/timestamptz_ns (0.00s)
--- PASS: TestNewManifestWriterV3NanosecondPartition (0.00s)
--- PASS: TestNewManifestWriterV3NanosecondPartition/timestamp_ns (0.00s)
--- PASS: TestNewManifestWriterV3NanosecondPartition/timestamptz_ns
(0.00s)
ok github.com/apache/iceberg-go 0.017s
```
Full `go test ./...` before and after the change is identical apart from the
new tests. One pre-existing failure, `TestReaderPreservesLZ4ChecksumError` in
`puffin`, is present in both runs and is unrelated to this change.
`golangci-lint v2.12.2` reports no issues on the three touched files (its only
finding, a gofmt complaint in `table/snapshot_producers.go:1766`, also
reproduces on unmodified main).
--
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]
