manuzhang opened a new pull request, #954:
URL: https://github.com/apache/iceberg-cpp/pull/954

   ## What
   
   Moves the avro-cpp pin in `cmake_modules/IcebergThirdpartyToolchain.cmake` 
from `997d50d` to `209a373` (current `apache/avro` main, 74 commits ahead). The 
C++ changes in the range:
   
   - AVRO-4351 (`209a373`): avro-cpp now keeps custom attributes on primitive 
schema nodes when parsing and printing schema JSON.
   - `6444e8d`: `std::formatter` for `avro::Name`/`avro::Type` templated on the 
format context, fixing C++23 builds.
   - `cc134b3`: JSON decoder rejects the lone low surrogate U+DFFF.
   
   Adds tests that pin AVRO-4351:
   
   - `AvroSchemaProjectionTest.ProjectTimestampTzFromParsedAdjustToUtc` / 
`RejectTimestampFromParsedAdjustToUtc` — parse `adjust-to-utc` from schema JSON 
and project `timestamptz`, `timestamptz_ns` and `timestamp` against it.
   - `AvroReaderParameterizedTest.TimestampTzTypes` — write `timestamptz` and 
`timestamptz_ns` columns to a file and read them back, in both decoder modes.
   - `AvroWriterTest.WriteTimestampTzTypes` — write all four timestamp types, 
assert `adjust-to-utc` is present on each primitive node in the physical file 
schema, and read the data back, in both encoder modes.
   
   Adds a comment on `GetAdjustToUtc()` recording why the pin matters.
   
   ## Why
   
   `GetAdjustToUtc()` reads `adjust-to-utc` off the primitive Avro node, and 
`ValidateAvroSchemaEvolution` gates `timestamp` vs `timestamptz` on it. The 
reader takes its file schema from the Avro file header, which avro-cpp parses 
from JSON. Before AVRO-4351, `Compiler.cc` only collected custom attributes for 
records, arrays and maps, and `NodePrimitive::printJson` never emitted them. So 
the attribute was lost twice: the writer dropped it from the file header, and 
the reader dropped it when parsing. Every timestamp read from a file then 
looked like it had no timezone, and projecting a `timestamptz` or 
`timestamptz_ns` column out of an Avro file failed with `Cannot read Iceberg 
type: timestamptz from Avro type: ...`.
   
   No test wrote and re-read a `timestamptz` column through a real file — the 
existing round trips use `TimestampType` only, and the `timestamptz` cases in 
`avro_data_test.cc` build the node in memory with `ToAvroNodeVisitor` — which 
is why this went unnoticed.
   
   ## Behavior change
   
   - `timestamptz` and `timestamptz_ns` columns can now be read from Avro files.
   - Avro files written by iceberg-cpp now carry `adjust-to-utc` on timestamp 
primitives in the file header schema, as the Iceberg spec requires.
   
   ## Testing
   
   Built locally with gcc 15 at C++23 against the new pin and ran `avro_test`: 
244/244 pass, including the 6 new tests (parameterized variants included). 
Rebuilt against the old pin `997d50d`: the same 6 tests fail — 
reader/projection tests with `Cannot read Iceberg type: timestamptz ...`, 
writer test with `node->customAttributes()` being `0` — confirming they pin the 
fix. `clang-format --dry-run -Werror` and `git diff --check` are clean on the 
changed files.
   
   🤖 Generated with [Claude Code](https://claude.com/claude-code)
   


-- 
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]

Reply via email to