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


##########
crates/iceberg/src/arrow/schema.rs:
##########
@@ -1178,6 +1182,7 @@ pub(crate) fn 
primitive_type_to_arrow_type_with_ree(primitive_type: &PrimitiveTy
         PrimitiveType::Uuid => make_ree(DataType::Binary),
         PrimitiveType::Fixed(_) => make_ree(DataType::Binary),
         PrimitiveType::Binary => make_ree(DataType::Binary),
+        PrimitiveType::Geometry(_) | PrimitiveType::Geography(_) => 
make_ree(DataType::LargeBinary),

Review Comment:
   LargeBinary uses 64-bit offsets for the cumulative payload of the entire 
Arrow array, so the relevant limit is the total WKB bytes in a batch, not a 
single geometry larger than 2 GiB. The read layer already maps Geo to 
LargeBinary, consistent with the existing Iceberg Binary-to-Arrow schema 
mapping. This constant-column helper must use the same storage type. Added that 
rationale in b67802d4; propagated to #3341 and #3342.



##########
crates/iceberg/src/spec/geospatial.rs:
##########
@@ -0,0 +1,227 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+//! Geospatial types defined by the Iceberg specification.
+
+use std::hash::{Hash, Hasher};
+
+use serde::{Deserialize, Serialize};
+
+use super::PrimitiveType;
+use crate::error::Result;
+
+pub(crate) const DEFAULT_GEOSPATIAL_CRS: &str = "OGC:CRS84";
+pub(super) const MAX_GEOSPATIAL_CRS_BYTES: usize = 128;
+
+#[derive(Debug, Serialize, Deserialize, Clone)]
+struct Crs(String);
+
+impl Crs {
+    fn new(crs: Option<String>) -> Result<Self> {
+        let crs = crs.unwrap_or_else(|| DEFAULT_GEOSPATIAL_CRS.to_string());
+        let crs = crs.trim().to_string();
+        if crs.len() > MAX_GEOSPATIAL_CRS_BYTES {

Review Comment:
   Removed the 128-byte CRS limit and its test in b67802d4. This was an 
arbitrary restriction without an established Iceberg contract. The syntax 
checks needed for serializing a CRS inside the type string remain.



##########
crates/iceberg/src/spec/geospatial.rs:
##########
@@ -0,0 +1,227 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+//! Geospatial types defined by the Iceberg specification.
+
+use std::hash::{Hash, Hasher};
+
+use serde::{Deserialize, Serialize};
+
+use super::PrimitiveType;
+use crate::error::Result;
+
+pub(crate) const DEFAULT_GEOSPATIAL_CRS: &str = "OGC:CRS84";
+pub(super) const MAX_GEOSPATIAL_CRS_BYTES: usize = 128;
+
+#[derive(Debug, Serialize, Deserialize, Clone)]
+struct Crs(String);
+
+impl Crs {
+    fn new(crs: Option<String>) -> Result<Self> {
+        let crs = crs.unwrap_or_else(|| DEFAULT_GEOSPATIAL_CRS.to_string());
+        let crs = crs.trim().to_string();
+        if crs.len() > MAX_GEOSPATIAL_CRS_BYTES {
+            return Err(crate::Error::new(
+                crate::ErrorKind::DataInvalid,
+                format!("Geospatial CRS must be at most 
{MAX_GEOSPATIAL_CRS_BYTES} bytes"),
+            ));
+        }
+        if crs.is_empty() || crs.contains([',', ')']) {
+            return Err(crate::Error::new(
+                crate::ErrorKind::DataInvalid,
+                "Geospatial CRS must be non-empty and must not contain ',' or 
')'",
+            ));
+        }
+
+        Ok(Self(crs))
+    }
+}
+
+impl Default for Crs {
+    fn default() -> Self {
+        Self(DEFAULT_GEOSPATIAL_CRS.to_string())
+    }
+}
+
+impl PartialEq for Crs {
+    fn eq(&self, other: &Self) -> bool {
+        self.0.eq_ignore_ascii_case(&other.0)
+    }
+}
+
+impl Eq for Crs {}
+
+impl Hash for Crs {
+    fn hash<H: Hasher>(&self, state: &mut H) {
+        for byte in self.0.bytes() {
+            state.write_u8(byte.to_ascii_lowercase());
+        }
+    }
+}
+
+/// Iceberg geometry type.
+#[derive(Debug, Serialize, Deserialize, PartialEq, Eq, Clone, Hash, Default)]
+pub struct GeometryType {
+    crs: Crs,
+}
+
+impl GeometryType {
+    /// Creates a geometry type with an optional coordinate reference system.
+    pub fn new(crs: Option<String>) -> Result<Self> {
+        Ok(Self {
+            crs: Crs::new(crs)?,
+        })
+    }
+
+    /// Returns the coordinate reference system.
+    pub fn crs(&self) -> &str {
+        &self.crs.0
+    }
+}
+
+/// Iceberg geography edge interpolation algorithm.
+#[derive(Debug, Serialize, Deserialize, PartialEq, Eq, Clone, Copy, Hash, 
Default)]
+#[serde(rename_all = "lowercase")]
+pub enum EdgeInterpolationAlgorithm {
+    /// Spherical edge interpolation.
+    #[default]
+    Spherical,
+    /// Vincenty edge interpolation.
+    Vincenty,
+    /// Thomas edge interpolation.
+    Thomas,
+    /// Andoyer edge interpolation.
+    Andoyer,
+    /// Karney edge interpolation.
+    Karney,
+}
+
+impl EdgeInterpolationAlgorithm {
+    pub(super) fn as_str(self) -> &'static str {
+        match self {
+            Self::Spherical => "spherical",
+            Self::Vincenty => "vincenty",
+            Self::Thomas => "thomas",
+            Self::Andoyer => "andoyer",
+            Self::Karney => "karney",
+        }
+    }
+
+    fn parse(value: &str) -> std::result::Result<Self, String> {
+        match value.trim().to_ascii_lowercase().as_str() {
+            "spherical" => Ok(Self::Spherical),
+            "vincenty" => Ok(Self::Vincenty),
+            "thomas" => Ok(Self::Thomas),
+            "andoyer" => Ok(Self::Andoyer),
+            "karney" => Ok(Self::Karney),
+            _ => Err(format!(
+                "Unknown geography edge interpolation algorithm: {value}"
+            )),
+        }
+    }
+}
+
+/// Iceberg geography type.
+#[derive(Debug, Serialize, Deserialize, PartialEq, Eq, Clone, Hash)]
+pub struct GeographyType {
+    crs: Crs,
+    algorithm: EdgeInterpolationAlgorithm,
+}
+
+impl Default for GeographyType {
+    fn default() -> Self {
+        Self {
+            crs: Crs::default(),
+            algorithm: EdgeInterpolationAlgorithm::Spherical,
+        }
+    }
+}
+
+impl GeographyType {
+    /// Creates a geography type with an optional coordinate reference system 
and edge interpolation algorithm.
+    pub fn new(crs: Option<String>, algorithm: EdgeInterpolationAlgorithm) -> 
Result<Self> {
+        Ok(Self {
+            crs: Crs::new(crs)?,
+            algorithm,
+        })
+    }
+
+    /// Returns the coordinate reference system.
+    pub fn crs(&self) -> &str {
+        &self.crs.0
+    }
+
+    /// Returns the edge interpolation algorithm.
+    pub fn algorithm(&self) -> EdgeInterpolationAlgorithm {
+        self.algorithm
+    }
+}
+
+fn parse_geospatial_params<'a>(
+    value: &'a str,
+    type_name: &str,
+) -> std::result::Result<Vec<&'a str>, String> {

Review Comment:
   Changed parse_geospatial_params to crate::error::Result in b67802d4, using 
DataInvalid errors rather than strings.



##########
crates/iceberg/src/spec/geospatial.rs:
##########
@@ -0,0 +1,227 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+//! Geospatial types defined by the Iceberg specification.
+
+use std::hash::{Hash, Hasher};
+
+use serde::{Deserialize, Serialize};
+
+use super::PrimitiveType;
+use crate::error::Result;
+
+pub(crate) const DEFAULT_GEOSPATIAL_CRS: &str = "OGC:CRS84";
+pub(super) const MAX_GEOSPATIAL_CRS_BYTES: usize = 128;
+
+#[derive(Debug, Serialize, Deserialize, Clone)]
+struct Crs(String);
+
+impl Crs {
+    fn new(crs: Option<String>) -> Result<Self> {
+        let crs = crs.unwrap_or_else(|| DEFAULT_GEOSPATIAL_CRS.to_string());
+        let crs = crs.trim().to_string();
+        if crs.len() > MAX_GEOSPATIAL_CRS_BYTES {
+            return Err(crate::Error::new(
+                crate::ErrorKind::DataInvalid,
+                format!("Geospatial CRS must be at most 
{MAX_GEOSPATIAL_CRS_BYTES} bytes"),
+            ));
+        }
+        if crs.is_empty() || crs.contains([',', ')']) {
+            return Err(crate::Error::new(
+                crate::ErrorKind::DataInvalid,
+                "Geospatial CRS must be non-empty and must not contain ',' or 
')'",
+            ));
+        }
+
+        Ok(Self(crs))
+    }
+}
+
+impl Default for Crs {
+    fn default() -> Self {
+        Self(DEFAULT_GEOSPATIAL_CRS.to_string())
+    }
+}
+
+impl PartialEq for Crs {
+    fn eq(&self, other: &Self) -> bool {
+        self.0.eq_ignore_ascii_case(&other.0)
+    }
+}
+
+impl Eq for Crs {}
+
+impl Hash for Crs {
+    fn hash<H: Hasher>(&self, state: &mut H) {
+        for byte in self.0.bytes() {
+            state.write_u8(byte.to_ascii_lowercase());
+        }
+    }
+}
+
+/// Iceberg geometry type.
+#[derive(Debug, Serialize, Deserialize, PartialEq, Eq, Clone, Hash, Default)]
+pub struct GeometryType {
+    crs: Crs,
+}
+
+impl GeometryType {
+    /// Creates a geometry type with an optional coordinate reference system.
+    pub fn new(crs: Option<String>) -> Result<Self> {
+        Ok(Self {
+            crs: Crs::new(crs)?,
+        })
+    }
+
+    /// Returns the coordinate reference system.
+    pub fn crs(&self) -> &str {
+        &self.crs.0
+    }
+}
+
+/// Iceberg geography edge interpolation algorithm.
+#[derive(Debug, Serialize, Deserialize, PartialEq, Eq, Clone, Copy, Hash, 
Default)]
+#[serde(rename_all = "lowercase")]
+pub enum EdgeInterpolationAlgorithm {
+    /// Spherical edge interpolation.
+    #[default]
+    Spherical,
+    /// Vincenty edge interpolation.
+    Vincenty,
+    /// Thomas edge interpolation.
+    Thomas,
+    /// Andoyer edge interpolation.
+    Andoyer,
+    /// Karney edge interpolation.
+    Karney,
+}
+
+impl EdgeInterpolationAlgorithm {
+    pub(super) fn as_str(self) -> &'static str {
+        match self {
+            Self::Spherical => "spherical",
+            Self::Vincenty => "vincenty",
+            Self::Thomas => "thomas",
+            Self::Andoyer => "andoyer",
+            Self::Karney => "karney",
+        }
+    }
+
+    fn parse(value: &str) -> std::result::Result<Self, String> {
+        match value.trim().to_ascii_lowercase().as_str() {
+            "spherical" => Ok(Self::Spherical),
+            "vincenty" => Ok(Self::Vincenty),
+            "thomas" => Ok(Self::Thomas),
+            "andoyer" => Ok(Self::Andoyer),
+            "karney" => Ok(Self::Karney),
+            _ => Err(format!(
+                "Unknown geography edge interpolation algorithm: {value}"
+            )),
+        }
+    }
+}
+
+/// Iceberg geography type.
+#[derive(Debug, Serialize, Deserialize, PartialEq, Eq, Clone, Hash)]
+pub struct GeographyType {
+    crs: Crs,
+    algorithm: EdgeInterpolationAlgorithm,
+}
+
+impl Default for GeographyType {
+    fn default() -> Self {
+        Self {
+            crs: Crs::default(),
+            algorithm: EdgeInterpolationAlgorithm::Spherical,
+        }
+    }
+}
+
+impl GeographyType {
+    /// Creates a geography type with an optional coordinate reference system 
and edge interpolation algorithm.
+    pub fn new(crs: Option<String>, algorithm: EdgeInterpolationAlgorithm) -> 
Result<Self> {
+        Ok(Self {
+            crs: Crs::new(crs)?,
+            algorithm,
+        })
+    }
+
+    /// Returns the coordinate reference system.
+    pub fn crs(&self) -> &str {
+        &self.crs.0
+    }
+
+    /// Returns the edge interpolation algorithm.
+    pub fn algorithm(&self) -> EdgeInterpolationAlgorithm {
+        self.algorithm
+    }
+}
+
+fn parse_geospatial_params<'a>(
+    value: &'a str,
+    type_name: &str,
+) -> std::result::Result<Vec<&'a str>, String> {
+    if value == type_name {
+        return Ok(vec![]);
+    }
+
+    let params = value
+        .strip_prefix(type_name)
+        .map(str::trim_start)
+        .and_then(|s| s.strip_prefix('('))
+        .and_then(|s| s.strip_suffix(')'))
+        .ok_or_else(|| format!("Invalid {type_name} type: {value}"))?;
+
+    if params.trim().is_empty() {
+        return Err(format!("{type_name} requires a non-empty CRS"));
+    }
+
+    Ok(params.split(',').map(str::trim).collect())
+}
+
+pub(super) fn parse_geometry(value: &str) -> 
std::result::Result<PrimitiveType, String> {

Review Comment:
   Changed parse_geometry to crate::error::Result in b67802d4. Constructor 
errors now propagate directly with ?.



##########
crates/iceberg/src/spec/geospatial.rs:
##########
@@ -0,0 +1,227 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+//! Geospatial types defined by the Iceberg specification.
+
+use std::hash::{Hash, Hasher};
+
+use serde::{Deserialize, Serialize};
+
+use super::PrimitiveType;
+use crate::error::Result;
+
+pub(crate) const DEFAULT_GEOSPATIAL_CRS: &str = "OGC:CRS84";
+pub(super) const MAX_GEOSPATIAL_CRS_BYTES: usize = 128;
+
+#[derive(Debug, Serialize, Deserialize, Clone)]
+struct Crs(String);
+
+impl Crs {
+    fn new(crs: Option<String>) -> Result<Self> {
+        let crs = crs.unwrap_or_else(|| DEFAULT_GEOSPATIAL_CRS.to_string());
+        let crs = crs.trim().to_string();
+        if crs.len() > MAX_GEOSPATIAL_CRS_BYTES {
+            return Err(crate::Error::new(
+                crate::ErrorKind::DataInvalid,
+                format!("Geospatial CRS must be at most 
{MAX_GEOSPATIAL_CRS_BYTES} bytes"),
+            ));
+        }
+        if crs.is_empty() || crs.contains([',', ')']) {
+            return Err(crate::Error::new(
+                crate::ErrorKind::DataInvalid,
+                "Geospatial CRS must be non-empty and must not contain ',' or 
')'",
+            ));
+        }
+
+        Ok(Self(crs))
+    }
+}
+
+impl Default for Crs {
+    fn default() -> Self {
+        Self(DEFAULT_GEOSPATIAL_CRS.to_string())
+    }
+}
+
+impl PartialEq for Crs {
+    fn eq(&self, other: &Self) -> bool {
+        self.0.eq_ignore_ascii_case(&other.0)
+    }
+}
+
+impl Eq for Crs {}
+
+impl Hash for Crs {
+    fn hash<H: Hasher>(&self, state: &mut H) {
+        for byte in self.0.bytes() {
+            state.write_u8(byte.to_ascii_lowercase());
+        }
+    }
+}
+
+/// Iceberg geometry type.
+#[derive(Debug, Serialize, Deserialize, PartialEq, Eq, Clone, Hash, Default)]
+pub struct GeometryType {
+    crs: Crs,
+}
+
+impl GeometryType {
+    /// Creates a geometry type with an optional coordinate reference system.
+    pub fn new(crs: Option<String>) -> Result<Self> {
+        Ok(Self {
+            crs: Crs::new(crs)?,
+        })
+    }
+
+    /// Returns the coordinate reference system.
+    pub fn crs(&self) -> &str {
+        &self.crs.0
+    }
+}
+
+/// Iceberg geography edge interpolation algorithm.
+#[derive(Debug, Serialize, Deserialize, PartialEq, Eq, Clone, Copy, Hash, 
Default)]
+#[serde(rename_all = "lowercase")]
+pub enum EdgeInterpolationAlgorithm {
+    /// Spherical edge interpolation.
+    #[default]
+    Spherical,
+    /// Vincenty edge interpolation.
+    Vincenty,
+    /// Thomas edge interpolation.
+    Thomas,
+    /// Andoyer edge interpolation.
+    Andoyer,
+    /// Karney edge interpolation.
+    Karney,
+}
+
+impl EdgeInterpolationAlgorithm {
+    pub(super) fn as_str(self) -> &'static str {
+        match self {
+            Self::Spherical => "spherical",
+            Self::Vincenty => "vincenty",
+            Self::Thomas => "thomas",
+            Self::Andoyer => "andoyer",
+            Self::Karney => "karney",
+        }
+    }
+
+    fn parse(value: &str) -> std::result::Result<Self, String> {
+        match value.trim().to_ascii_lowercase().as_str() {
+            "spherical" => Ok(Self::Spherical),
+            "vincenty" => Ok(Self::Vincenty),
+            "thomas" => Ok(Self::Thomas),
+            "andoyer" => Ok(Self::Andoyer),
+            "karney" => Ok(Self::Karney),
+            _ => Err(format!(
+                "Unknown geography edge interpolation algorithm: {value}"
+            )),
+        }
+    }
+}
+
+/// Iceberg geography type.
+#[derive(Debug, Serialize, Deserialize, PartialEq, Eq, Clone, Hash)]
+pub struct GeographyType {
+    crs: Crs,
+    algorithm: EdgeInterpolationAlgorithm,
+}
+
+impl Default for GeographyType {
+    fn default() -> Self {
+        Self {
+            crs: Crs::default(),
+            algorithm: EdgeInterpolationAlgorithm::Spherical,
+        }
+    }
+}
+
+impl GeographyType {
+    /// Creates a geography type with an optional coordinate reference system 
and edge interpolation algorithm.
+    pub fn new(crs: Option<String>, algorithm: EdgeInterpolationAlgorithm) -> 
Result<Self> {
+        Ok(Self {
+            crs: Crs::new(crs)?,
+            algorithm,
+        })
+    }
+
+    /// Returns the coordinate reference system.
+    pub fn crs(&self) -> &str {
+        &self.crs.0
+    }
+
+    /// Returns the edge interpolation algorithm.
+    pub fn algorithm(&self) -> EdgeInterpolationAlgorithm {
+        self.algorithm
+    }
+}
+
+fn parse_geospatial_params<'a>(
+    value: &'a str,
+    type_name: &str,
+) -> std::result::Result<Vec<&'a str>, String> {
+    if value == type_name {
+        return Ok(vec![]);
+    }
+
+    let params = value
+        .strip_prefix(type_name)
+        .map(str::trim_start)
+        .and_then(|s| s.strip_prefix('('))
+        .and_then(|s| s.strip_suffix(')'))
+        .ok_or_else(|| format!("Invalid {type_name} type: {value}"))?;
+
+    if params.trim().is_empty() {
+        return Err(format!("{type_name} requires a non-empty CRS"));
+    }
+
+    Ok(params.split(',').map(str::trim).collect())
+}
+
+pub(super) fn parse_geometry(value: &str) -> 
std::result::Result<PrimitiveType, String> {
+    let params = parse_geospatial_params(value.trim(), "geometry")?;
+    let geometry = match params.as_slice() {
+        [] => GeometryType::default(),
+        [crs] if !crs.is_empty() => {
+            GeometryType::new(Some((*crs).to_string())).map_err(|err| 
err.to_string())?
+        }
+        _ => return Err(format!("Invalid geometry type: {value}")),
+    };
+
+    Ok(PrimitiveType::Geometry(geometry))
+}
+
+pub(super) fn parse_geography(value: &str) -> 
std::result::Result<PrimitiveType, String> {

Review Comment:
   Changed parse_geography and the edge-algorithm parser to 
crate::error::Result in b67802d4. Removed the error-to-string conversions.



##########
crates/iceberg/src/spec/geospatial.rs:
##########
@@ -0,0 +1,227 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+//! Geospatial types defined by the Iceberg specification.
+
+use std::hash::{Hash, Hasher};
+
+use serde::{Deserialize, Serialize};
+
+use super::PrimitiveType;
+use crate::error::Result;
+
+pub(crate) const DEFAULT_GEOSPATIAL_CRS: &str = "OGC:CRS84";
+pub(super) const MAX_GEOSPATIAL_CRS_BYTES: usize = 128;
+
+#[derive(Debug, Serialize, Deserialize, Clone)]
+struct Crs(String);
+
+impl Crs {
+    fn new(crs: Option<String>) -> Result<Self> {
+        let crs = crs.unwrap_or_else(|| DEFAULT_GEOSPATIAL_CRS.to_string());
+        let crs = crs.trim().to_string();
+        if crs.len() > MAX_GEOSPATIAL_CRS_BYTES {
+            return Err(crate::Error::new(
+                crate::ErrorKind::DataInvalid,
+                format!("Geospatial CRS must be at most 
{MAX_GEOSPATIAL_CRS_BYTES} bytes"),
+            ));
+        }
+        if crs.is_empty() || crs.contains([',', ')']) {
+            return Err(crate::Error::new(
+                crate::ErrorKind::DataInvalid,
+                "Geospatial CRS must be non-empty and must not contain ',' or 
')'",
+            ));
+        }
+
+        Ok(Self(crs))
+    }
+}
+
+impl Default for Crs {
+    fn default() -> Self {
+        Self(DEFAULT_GEOSPATIAL_CRS.to_string())
+    }
+}
+
+impl PartialEq for Crs {
+    fn eq(&self, other: &Self) -> bool {
+        self.0.eq_ignore_ascii_case(&other.0)
+    }
+}
+
+impl Eq for Crs {}
+
+impl Hash for Crs {
+    fn hash<H: Hasher>(&self, state: &mut H) {
+        for byte in self.0.bytes() {
+            state.write_u8(byte.to_ascii_lowercase());
+        }
+    }
+}
+
+/// Iceberg geometry type.
+#[derive(Debug, Serialize, Deserialize, PartialEq, Eq, Clone, Hash, Default)]
+pub struct GeometryType {
+    crs: Crs,
+}
+
+impl GeometryType {
+    /// Creates a geometry type with an optional coordinate reference system.
+    pub fn new(crs: Option<String>) -> Result<Self> {
+        Ok(Self {
+            crs: Crs::new(crs)?,
+        })
+    }
+
+    /// Returns the coordinate reference system.
+    pub fn crs(&self) -> &str {
+        &self.crs.0
+    }
+}
+
+/// Iceberg geography edge interpolation algorithm.
+#[derive(Debug, Serialize, Deserialize, PartialEq, Eq, Clone, Copy, Hash, 
Default)]
+#[serde(rename_all = "lowercase")]
+pub enum EdgeInterpolationAlgorithm {
+    /// Spherical edge interpolation.
+    #[default]
+    Spherical,
+    /// Vincenty edge interpolation.
+    Vincenty,
+    /// Thomas edge interpolation.
+    Thomas,
+    /// Andoyer edge interpolation.
+    Andoyer,
+    /// Karney edge interpolation.
+    Karney,
+}
+
+impl EdgeInterpolationAlgorithm {
+    pub(super) fn as_str(self) -> &'static str {
+        match self {
+            Self::Spherical => "spherical",
+            Self::Vincenty => "vincenty",
+            Self::Thomas => "thomas",
+            Self::Andoyer => "andoyer",
+            Self::Karney => "karney",
+        }
+    }
+
+    fn parse(value: &str) -> std::result::Result<Self, String> {
+        match value.trim().to_ascii_lowercase().as_str() {
+            "spherical" => Ok(Self::Spherical),
+            "vincenty" => Ok(Self::Vincenty),
+            "thomas" => Ok(Self::Thomas),
+            "andoyer" => Ok(Self::Andoyer),
+            "karney" => Ok(Self::Karney),
+            _ => Err(format!(
+                "Unknown geography edge interpolation algorithm: {value}"
+            )),
+        }
+    }
+}
+
+/// Iceberg geography type.
+#[derive(Debug, Serialize, Deserialize, PartialEq, Eq, Clone, Hash)]
+pub struct GeographyType {
+    crs: Crs,
+    algorithm: EdgeInterpolationAlgorithm,
+}
+
+impl Default for GeographyType {
+    fn default() -> Self {
+        Self {
+            crs: Crs::default(),
+            algorithm: EdgeInterpolationAlgorithm::Spherical,
+        }
+    }
+}
+
+impl GeographyType {
+    /// Creates a geography type with an optional coordinate reference system 
and edge interpolation algorithm.
+    pub fn new(crs: Option<String>, algorithm: EdgeInterpolationAlgorithm) -> 
Result<Self> {
+        Ok(Self {
+            crs: Crs::new(crs)?,
+            algorithm,
+        })
+    }
+
+    /// Returns the coordinate reference system.
+    pub fn crs(&self) -> &str {
+        &self.crs.0
+    }
+
+    /// Returns the edge interpolation algorithm.
+    pub fn algorithm(&self) -> EdgeInterpolationAlgorithm {
+        self.algorithm
+    }
+}
+
+fn parse_geospatial_params<'a>(
+    value: &'a str,
+    type_name: &str,
+) -> std::result::Result<Vec<&'a str>, String> {
+    if value == type_name {

Review Comment:
   Fixed both the PrimitiveType dispatch and parameter parser to recognize 
geometry/geography case-insensitively in b67802d4. CRS spelling is preserved, 
and serialization uses the canonical lowercase type and algorithm names. 
Updated the existing serialization test with mixed-case inputs.



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