Skip to content

Support reading Avro timestamp-nanos logical type - #4136

Open
ump45nose wants to merge 1 commit into
apache:mainfrom
ump45nose:fix/avro-timestamp-nanos
Open

ump45nose wants to merge 1 commit into
apache:mainfrom
ump45nose:fix/avro-timestamp-nanos

Conversation

@ump45nose

Copy link
Copy Markdown

Part of #4078 (the "Avro timestamp-nanos raises Unknown logical type" item).

Rationale for this change

AvroSchemaConversion.iceberg_to_avro already writes timestamp_ns / timestamptz_ns as {"type": "long", "logicalType": "timestamp-nanos", "adjust-to-utc": ...}, but avro_to_iceberg has no branch for timestamp-nanos. So an Avro file PyIceberg writes with a nanosecond timestamp field can't be opened by AvroFile, because it converts the schema from the file header first:

ValueError: Unknown logical/physical type combination: {'type': 'long', 'logicalType': 'timestamp-nanos', 'adjust-to-utc': False}

This change maps timestamp-nanos to TimestampNanoType, or to TimestamptzNanoType when adjust-to-utc is true, the same way timestamp-micros is handled. The readers and writers for these types already exist in pyiceberg/avro, so only the schema conversion was missing. I also fixed the parameter type hints on the two visit_timestamp*_ns methods, which said TimestampType / TimestamptzType.

Are these changes tested?

Yes:

  • tests/utils/test_schema_conversion.py: timestamp-nanos with adjust-to-utc missing, false, and true, plus an Iceberg -> Avro -> Iceberg schema round trip.
  • tests/avro/test_file.py: writes a file with timestamp_ns and optional timestamptz_ns fields using AvroOutputFile and reads it back with AvroFile.

All 5 new tests fail on main and pass with this change. pytest tests/avro tests/utils/test_schema_conversion.py passes (193 tests), and ruff check/format are clean.

Are there any user-facing changes?

Avro files with timestamp-nanos fields can now be read. No API changes.


AI disclosure: I used an AI coding assistant to help write this change and its tests. I reviewed the code and ran the tests myself.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant