wirybeaver commented on code in PR #2653:
URL: https://github.com/apache/iceberg-rust/pull/2653#discussion_r3449493959


##########
crates/iceberg/src/spec/datatypes.rs:
##########
@@ -204,6 +207,157 @@ impl From<MapType> for Type {
     }
 }
 
+/// Iceberg geometry type.
+#[derive(Debug, Serialize, Deserialize, PartialEq, Eq, Clone, Hash, Default)]
+pub struct GeometryType {
+    crs: Option<String>,
+}
+
+impl GeometryType {
+    /// Creates a geometry type with an optional coordinate reference system.
+    pub fn new(crs: Option<String>) -> Result<Self> {
+        Ok(Self {
+            crs: normalize_crs(crs)?,
+        })
+    }
+
+    /// Returns the coordinate reference system, or `None` for the Iceberg 
default CRS.
+    pub fn crs(&self) -> Option<&str> {
+        self.crs.as_deref()
+    }
+}
+
+/// Iceberg geography type.
+#[derive(Debug, Serialize, Deserialize, PartialEq, Eq, Clone)]
+pub struct GeographyType {
+    crs: Option<String>,
+    algorithm: WkbEdges,
+}
+
+impl Default for GeographyType {
+    fn default() -> Self {
+        Self {
+            crs: None,
+            algorithm: WkbEdges::Spherical,
+        }
+    }
+}
+
+impl Hash for GeographyType {
+    fn hash<H: Hasher>(&self, state: &mut H) {
+        self.crs.hash(state);
+        wkb_edges_as_str(self.algorithm).hash(state);
+    }
+}
+
+impl GeographyType {
+    /// Creates a geography type with an optional coordinate reference system 
and edge interpolation algorithm.
+    pub fn new(crs: Option<String>, algorithm: WkbEdges) -> Result<Self> {

Review Comment:
   Done in 3c8a92e. `GeographyType` now uses an Iceberg-owned 
`EdgeInterpolationAlgorithm`, and Arrow schema conversion maps to/from 
`parquet_geospatial::WkbEdges` only at the WKB extension metadata boundary.



##########
crates/iceberg/src/spec/values/datum.rs:
##########
@@ -417,8 +417,10 @@ impl Datum {
             PrimitiveType::Uuid => {
                 
PrimitiveLiteral::UInt128(u128::from_be_bytes(bytes.try_into()?))
             }
-            PrimitiveType::Fixed(_) => 
PrimitiveLiteral::Binary(Vec::from(bytes)),
-            PrimitiveType::Binary => 
PrimitiveLiteral::Binary(Vec::from(bytes)),
+            PrimitiveType::Fixed(_)
+            | PrimitiveType::Binary
+            | PrimitiveType::Geometry(_)
+            | PrimitiveType::Geography(_) => 
PrimitiveLiteral::Binary(Vec::from(bytes)),

Review Comment:
   Agreed. This PR still omits spatial lower/upper bounds from the Parquet 
writer, so it does not claim WKB-based bound support. If we add spatial bounds 
later, the bound codec should use the spec point encoding (`x:y[:z][:m]` as 
little-endian f64 values), not WKB.



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