[PR] fix(glue): give timestamptz and unknown columns a Glue type instead of failing [iceberg-rust]

["ump45nose (via GitHub)" <gi...@apache.org>]

ump45nose opened a new pull request, #3488:
URL: https://github.com/apache/iceberg-rust/pull/3488

   ## Which issue does this PR close?
   
   - Closes #3375.
   
   ## What changes are included in this PR?
   
   `GlueSchemaBuilder` returned `FeatureUnsupported` for `timestamptz`, `timestamptz_ns` and `unknown`, so creating, registering or committing to a Glue table with one of those columns failed, even though Glue column types are display-only for Iceberg tables. As suggested in https://github.com/apache/iceberg-rust/pull/2933#discussion_r4201735423, the conversion no longer fails for any type:
   
   - `timestamptz` -> `timestamp`, as iceberg-java's `IcebergToGlueConverter` does (its `TIMESTAMP` case covers both).
   - `timestamptz_ns` -> `timestamp_ns`, the spelling this crate already uses for `timestamp_ns`.
   - `unknown` -> `unknown`, the type's display string, which is also what Java's default case produces.
   
   The existing spellings for every other type are unchanged. The timestamptz mapping is the same one proposed in #2956, which was closed as stale.
   
   ## Are these changes tested?
   
   Yes. `test_schema_with_simple_fields` now includes a `timestamptz` column, and `test_schema_with_v3_primitive_types` covers `timestamptz_ns` and `unknown` in a v3 table. Both fail on `main` (`1be091a`) with `FeatureUnsupported`. `cargo test -p iceberg-catalog-glue --lib` and `cargo clippy -p iceberg-catalog-glue --all-targets -- -D warnings` pass.
   
   ## AI Disclosure
   
   I used an AI assistant to find the failing arms, compare them with iceberg-java, and draft the tests; I reviewed the change and ran the tests myself. Worth a reviewer's look: the nanosecond spelling, since Java writes `timestamp_nano` for both nanosecond types while this crate already wrote `timestamp_ns`, and I kept the crate's spelling rather than change existing tables' Glue columns. Mapping `timestamptz` to `timestamp` drops the time zone from the display type, matching Java. I did not run this against a real Glue catalog.
   


-- 
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: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org

Re: [PR] fix(glue): give timestamptz and unknown columns a Glue type instead of failing [iceberg-rust]

["AIM-Software-Tools (via GitHub) via issues" <is...@iceberg.apache.org>]

AIM-Software-Tools commented on PR #3488:
URL: https://github.com/apache/iceberg-rust/pull/3488#issuecomment-6578715157

   We hit #3375 in production: appending to a Glue-catalogued table with a `timestamptz` column fails in both `create_table` and `update_table`, because each one rebuilds the Glue columns. We've been running the same mapping as this PR (`timestamptz` → Glue `timestamp`, matching the Java `GlueCatalog`) from a vendored 0.10.1 since the end of October. Creates, appends, and Athena reads on those tables all work. We'd like to drop the vendored copy once this is released, so a review would be much appreciated.


-- 
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: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org