wirybeaver commented on code in PR #2933:
URL: https://github.com/apache/iceberg-rust/pull/2933#discussion_r4225768742
##########
crates/catalog/glue/src/schema.rs:
##########
@@ -173,6 +173,12 @@ impl SchemaVisitor for GlueSchemaBuilder {
"string".to_string()
}
PrimitiveType::Binary | PrimitiveType::Fixed(_) =>
"binary".to_string(),
+ PrimitiveType::Geometry(_) | PrimitiveType::Geography(_) => {
+ return Err(Error::new(
+ ErrorKind::FeatureUnsupported,
+ format!("Conversion from {p:?} is not supported"),
+ ));
+ }
Review Comment:
Agreed. Updated in 50bebdca: Geometry/Geography now use `p.to_string()` for
Glue display metadata instead of returning `FeatureUnsupported`. I kept the
existing Glue-compatible spellings for known types, added the display-only
rationale in code, and opened #3375 for the broader existing non-failing
fallback cleanup. The change is propagated through #3341 and #3342.
##########
crates/catalog/glue/src/schema.rs:
##########
@@ -560,4 +568,23 @@ mod tests {
assert_eq!(result, expected);
Ok(())
}
+
+ #[test]
+ fn test_schema_with_geospatial_type_is_unsupported() {
Review Comment:
Updated in 50bebdca: removed the unsupported-error expectation and replaced
it with a positive assertion that Glue receives `geometry(OGC:CRS84)`.
##########
crates/catalog/hms/src/schema.rs:
##########
@@ -117,7 +117,10 @@ impl SchemaVisitor for HiveSchemaBuilder {
PrimitiveType::Time | PrimitiveType::String | PrimitiveType::Uuid
=> {
"string".to_string()
}
- PrimitiveType::Binary | PrimitiveType::Fixed(_) =>
"binary".to_string(),
+ PrimitiveType::Binary
+ | PrimitiveType::Fixed(_)
+ | PrimitiveType::Geometry(_)
+ | PrimitiveType::Geography(_) => "binary".to_string(),
Review Comment:
Follow-up after CTTY’s Glue clarification: HMS still rejects Geo because
Hive parses these type strings, matching Java HMS behavior. Glue is
display-only and now emits `p.to_string()` instead of failing (50bebdca); the
broader Glue fallback cleanup is tracked in #3375.
--
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]
