You are viewing a plain text version of this content. The canonical link for it is here.

Posted to github@arrow.apache.org by "mbrobbel (via GitHub)" <gi...@apache.org> on 2024/05/31 13:09:40 UTC

[PR] Add `ExtensionType` for `uuid` and map to parquet logical type [arrow-rs]

mbrobbel opened a new pull request, #5822:
URL: https://github.com/apache/arrow-rs/pull/5822

   # Rationale for this change
    
   It would be nice to better support reading and writing the canonical `uuid` extension type with the arrow and parquet crate i.e. mapping between the arrow extension type and the parquet logical `uuid` type.
   
   # What changes are included in this PR?
   
   This adds an `ExtensionType` enum for canonical extension types.
   
   # Are there any user-facing changes?
   
   Users can now annotate their logical types with extension types, and for `uuid` they are propagated via the arrow writer to map to the parquet `uuid` logical type.
   
   This needs tests and better docs, but I'd like to get some feedback on the approach first, because there are many different ways to implement this.
   
   I quickly tested this change with [narrow](https://github.com/mbrobbel/narrow/pull/183/files#diff-8868964a71a65db2aaef344fc98c97484058a8a9d46dc45d91a9d47313ffe100R21-R26) and those `uuid` fields (in the parquet file) are now picked up as `uuid` instead of `blob` by DuckDB.


-- 
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: github-unsubscribe@arrow.apache.org

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

Re: [PR] Add `ExtensionType` trait and `CanonicalExtensionType` enum [arrow-rs]

Posted by "alamb (via GitHub)" <gi...@apache.org>.

alamb commented on code in PR #5822:
URL: https://github.com/apache/arrow-rs/pull/5822#discussion_r2083123001


##########
arrow-schema/src/field.rs:
##########
@@ -350,6 +355,167 @@ impl Field {
         self
     }
 
+    /// Returns the extension type name of this [`Field`], if set.
+    ///
+    /// This returns the value of [`EXTENSION_TYPE_NAME_KEY`], if set in
+    /// [`Field::metadata`]. If the key is missing, there is no extension type
+    /// name and this returns `None`.
+    ///
+    /// # Example
+    ///
+    /// ```
+    /// # use arrow_schema::{DataType, extension::EXTENSION_TYPE_NAME_KEY, Field};
+    ///
+    /// let field = Field::new("", DataType::Null, false);
+    /// assert_eq!(field.extension_type_name(), None);
+    ///
+    /// let field = Field::new("", DataType::Null, false).with_metadata(
+    ///    [(EXTENSION_TYPE_NAME_KEY.to_owned(), "example".to_owned())]
+    ///        .into_iter()
+    ///        .collect(),
+    /// );
+    /// assert_eq!(field.extension_type_name(), Some("example"));
+    /// ```
+    pub fn extension_type_name(&self) -> Option<&str> {
+        self.metadata()
+            .get(EXTENSION_TYPE_NAME_KEY)
+            .map(String::as_ref)
+    }
+
+    /// Returns the extension type metadata of this [`Field`], if set.
+    ///
+    /// This returns the value of [`EXTENSION_TYPE_METADATA_KEY`], if set in
+    /// [`Field::metadata`]. If the key is missing, there is no extension type
+    /// metadata and this returns `None`.
+    ///
+    /// # Example
+    ///
+    /// ```
+    /// # use arrow_schema::{DataType, extension::EXTENSION_TYPE_METADATA_KEY, Field};
+    ///
+    /// let field = Field::new("", DataType::Null, false);
+    /// assert_eq!(field.extension_type_metadata(), None);
+    ///
+    /// let field = Field::new("", DataType::Null, false).with_metadata(
+    ///    [(EXTENSION_TYPE_METADATA_KEY.to_owned(), "example".to_owned())]
+    ///        .into_iter()
+    ///        .collect(),
+    /// );
+    /// assert_eq!(field.extension_type_metadata(), Some("example"));
+    /// ```
+    pub fn extension_type_metadata(&self) -> Option<&str> {
+        self.metadata()
+            .get(EXTENSION_TYPE_METADATA_KEY)
+            .map(String::as_ref)
+    }
+
+    /// Returns an instance of the given [`ExtensionType`] of this [`Field`],
+    /// if set in the [`Field::metadata`].
+    ///
+    /// # Error
+    ///
+    /// Returns an error if
+    /// - this field does not have the name of this extension type
+    ///   ([`ExtensionType::NAME`]) in the [`Field::metadata`] (mismatch or
+    ///   missing)
+    /// - the deserialization of the metadata
+    ///   ([`ExtensionType::deserialize_metadata`]) fails
+    /// - the construction of the extension type ([`ExtensionType::try_new`])
+    ///   fail (for example when the [`Field::data_type`] is not supported by
+    ///   the extension type ([`ExtensionType::supports_data_type`]))
+    pub fn try_extension_type<E: ExtensionType>(&self) -> Result<E, ArrowError> {
+        // Check the extension name in the metadata
+        match self.extension_type_name() {
+            // It should match the name of the given extension type
+            Some(name) if name == E::NAME => {
+                // Deserialize the metadata and try to construct the extension
+                // type
+                E::deserialize_metadata(self.extension_type_metadata())
+                    .and_then(|metadata| E::try_new(self.data_type(), metadata))
+            }
+            // Name mismatch
+            Some(name) => Err(ArrowError::InvalidArgumentError(format!(
+                "Field extension type name mismatch, expected {}, found {name}",
+                E::NAME
+            ))),
+            // Name missing
+            None => Err(ArrowError::InvalidArgumentError(
+                "Field extension type name missing".to_owned(),
+            )),
+        }
+    }
+
+    /// Returns an instance of the given [`ExtensionType`] of this [`Field`],
+    /// panics if this [`Field`] does not have this extension type.
+    ///
+    /// # Panic
+    ///
+    /// This calls [`Field::try_extension_type`] and panics when it returns an
+    /// error.
+    pub fn extension_type<E: ExtensionType>(&self) -> E {
+        self.try_extension_type::<E>()
+            .unwrap_or_else(|e| panic!("{e}"))
+    }
+
+    /// Updates the metadata of this [`Field`] with the [`ExtensionType::NAME`]
+    /// and [`ExtensionType::metadata`] of the given [`ExtensionType`], if the
+    /// given extension type supports the [`Field::data_type`] of this field
+    /// ([`ExtensionType::supports_data_type`]).
+    ///
+    /// If the given extension type defines no metadata, a previously set
+    /// value of [`EXTENSION_TYPE_METADATA_KEY`] is cleared.
+    ///
+    /// # Error
+    ///
+    /// This functions returns an error if the data type of this field does not
+    /// match any of the supported storage types of the given extension type.
+    pub fn try_with_extension_type<E: ExtensionType>(
+        &mut self,
+        extension_type: E,
+    ) -> Result<(), ArrowError> {
+        // Make sure the data type of this field is supported
+        extension_type.supports_data_type(&self.data_type)?;
+
+        self.metadata
+            .insert(EXTENSION_TYPE_NAME_KEY.to_owned(), E::NAME.to_owned());
+        match extension_type.serialize_metadata() {
+            Some(metadata) => self
+                .metadata
+                .insert(EXTENSION_TYPE_METADATA_KEY.to_owned(), metadata),
+            // If this extension type has no metadata, we make sure to
+            // clear previously set metadata.
+            None => self.metadata.remove(EXTENSION_TYPE_METADATA_KEY),
+        };
+
+        Ok(())
+    }
+
+    /// Updates the metadata of this [`Field`] with the [`ExtensionType::NAME`]
+    /// and [`ExtensionType::metadata`] of the given [`ExtensionType`].
+    ///
+    /// # Panics
+    ///
+    /// This calls [`Field::try_with_extension_type`] and panics when it
+    /// returns an error.
+    pub fn with_extension_type<E: ExtensionType>(mut self, extension_type: E) -> Self {

Review Comment:
   I was thinking about how to update the documentation to capture this conversation but I can't really come up with an specific changes to recommend



-- 
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: github-unsubscribe@arrow.apache.org

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

Re: [PR] Add `ExtensionType` trait and `CanonicalExtensionType` enum [arrow-rs]

Posted by "mbrobbel (via GitHub)" <gi...@apache.org>.

mbrobbel commented on code in PR #5822:
URL: https://github.com/apache/arrow-rs/pull/5822#discussion_r1926026340


##########
arrow-schema/src/extension/mod.rs:
##########
@@ -0,0 +1,252 @@
+// 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.
+
+//! Extension types.
+
+#[cfg(feature = "canonical-extension-types")]
+mod canonical;
+#[cfg(feature = "canonical-extension-types")]
+pub use canonical::*;
+
+use crate::{ArrowError, DataType};
+
+/// The metadata key for the string name identifying an [`ExtensionType`].
+pub const EXTENSION_TYPE_NAME_KEY: &str = "ARROW:extension:name";
+
+/// The metadata key for a serialized representation of the [`ExtensionType`]
+/// necessary to reconstruct the custom type.
+pub const EXTENSION_TYPE_METADATA_KEY: &str = "ARROW:extension:metadata";
+
+/// Extension types.
+///
+/// User-defined “extension” types can be defined setting certain key value
+/// pairs in the [`Field`] metadata structure. These extension keys are:
+/// - [`EXTENSION_TYPE_NAME_KEY`]
+/// - [`EXTENSION_TYPE_METADATA_KEY`]
+///
+/// Canonical extension types support in this crate requires the
+/// `canonical-extension-types` feature.
+///
+/// Extension types may or may not use the [`EXTENSION_TYPE_METADATA_KEY`]
+/// field.
+///
+/// # Example
+///
+/// The example below demonstrates how to implement this trait for a `Uuid`
+/// type. Note this is not the canonical extension type for `Uuid`, which does
+/// not include information about the `Uuid` version.
+///
+/// ```
+/// # use arrow_schema::ArrowError;
+/// # fn main() -> Result<(), ArrowError> {
+/// use arrow_schema::{DataType, extension::ExtensionType, Field};
+/// use std::{fmt, str::FromStr};
+///
+/// /// The different Uuid versions.
+/// #[derive(Clone, Copy, Debug, PartialEq)]
+/// enum UuidVersion {
+///     V1,
+///     V2,
+///     V3,
+///     V4,
+///     V5,
+///     V6,
+///     V7,
+///     V8,
+/// }
+///
+/// // We'll use `Display` to serialize.
+/// impl fmt::Display for UuidVersion {
+///     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+///         write!(
+///             f,
+///             "{}",
+///             match self {
+///                 Self::V1 => "V1",
+///                 Self::V2 => "V2",
+///                 Self::V3 => "V3",
+///                 Self::V4 => "V4",
+///                 Self::V5 => "V5",
+///                 Self::V6 => "V6",
+///                 Self::V7 => "V7",
+///                 Self::V8 => "V8",
+///             }
+///         )
+///     }
+/// }
+///
+/// // And `FromStr` to deserialize.
+/// impl FromStr for UuidVersion {
+///     type Err = ArrowError;
+///
+///     fn from_str(s: &str) -> Result<Self, Self::Err> {
+///         match s {
+///             "V1" => Ok(Self::V1),
+///             "V2" => Ok(Self::V2),
+///             "V3" => Ok(Self::V3),
+///             "V4" => Ok(Self::V4),
+///             "V5" => Ok(Self::V5),
+///             "V6" => Ok(Self::V6),
+///             "V7" => Ok(Self::V7),
+///             "V8" => Ok(Self::V8),
+///             _ => Err(ArrowError::ParseError("Invalid UuidVersion".to_owned())),
+///         }
+///     }
+/// }
+///
+/// /// This is the extension type, not the container for Uuid values. It
+/// /// stores the Uuid version (this is the metadata of this extension type).
+/// #[derive(Clone, Copy, Debug, PartialEq)]
+/// struct Uuid(UuidVersion);
+///
+/// impl ExtensionType for Uuid {
+///     // We use a namespace as suggested by the specification.
+///     const NAME: &'static str = "myorg.example.uuid";
+///
+///     // The metadata type is the Uuid version.
+///     type Metadata = UuidVersion;
+///
+///     // We just return a reference to the Uuid version.
+///     fn metadata(&self) -> &Self::Metadata {
+///         &self.0
+///     }
+///
+///     // We use the `Display` implementation to serialize the Uuid
+///     // version.
+///     fn serialize_metadata(&self) -> Option<String> {
+///         Some(self.0.to_string())
+///     }
+///
+///     // We use the `FromStr` implementation to deserialize the Uuid
+///     // version.
+///     fn deserialize_metadata(metadata: Option<&str>) -> Result<Self::Metadata, ArrowError> {
+///         metadata.map_or_else(
+///             || {
+///                 Err(ArrowError::InvalidArgumentError(
+///                     "Uuid extension type metadata missing".to_owned(),
+///                 ))
+///             },
+///             str::parse,
+///         )
+///     }
+///
+///     // The only supported data type is `FixedSizeBinary(16)`.
+///     fn supports_data_type(&self, data_type: &DataType) -> Result<(), ArrowError> {
+///         match data_type {
+///             DataType::FixedSizeBinary(16) => Ok(()),
+///             data_type => Err(ArrowError::InvalidArgumentError(format!(
+///                 "Uuid data type mismatch, expected FixedSizeBinary(16), found {data_type}"
+///             ))),
+///         }
+///     }
+///
+///     // We should always check if the data type is supported before
+///     // constructing the extension type.
+///     fn try_new(data_type: &DataType, metadata: Self::Metadata) -> Result<Self, ArrowError> {
+///         let uuid = Self(metadata);
+///         uuid.supports_data_type(data_type)?;
+///         Ok(uuid)
+///     }
+/// }
+///
+/// // We can now construct the extension type.
+/// let uuid_v1 = Uuid(UuidVersion::V1);
+///
+/// // And add it to a field.
+/// let mut field =
+///     Field::new("", DataType::FixedSizeBinary(16), false).with_extension_type(uuid_v1);
+///
+/// // And extract it from this field.
+/// assert_eq!(field.try_extension_type::<Uuid>()?, uuid_v1);
+///
+/// // When we try to add this to a field with an unsupported data type we
+/// // get an error.
+/// let result = Field::new("", DataType::Null, false).try_with_extension_type(uuid_v1);
+/// assert!(result.is_err());
+/// # Ok(()) }
+/// ```
+///
+/// <https://arrow.apache.org/docs/format/Columnar.html#extension-types>
+///
+/// [`Field`]: crate::Field
+pub trait ExtensionType: Sized {
+    /// The name identifying this extension type.
+    ///
+    /// This is the string value that is used for the
+    /// [`EXTENSION_TYPE_NAME_KEY`] in the [`Field::metadata`] of a [`Field`]
+    /// to identify this extension type.
+    ///
+    /// We recommend that you use a “namespace”-style prefix for extension
+    /// type names to minimize the possibility of conflicts with multiple Arrow
+    /// readers and writers in the same application. For example, use
+    /// `myorg.name_of_type` instead of simply `name_of_type`.
+    ///
+    /// Extension names beginning with `arrow.` are reserved for canonical
+    /// extension types, they should not be used for third-party extension
+    /// types.
+    ///
+    /// Extension names are case-sensitive.
+    ///
+    /// [`Field`]: crate::Field
+    /// [`Field::metadata`]: crate::Field::metadata
+    const NAME: &'static str;
+
+    /// The metadata type of this extension type.
+    ///
+    /// If an extension type defines no metadata it should use `()` to indicate
+    /// this.
+    type Metadata;
+
+    /// Returns a reference to the metadata of this extension type, or `&()` if
+    /// if this extension type defines no metadata (`Self::Metadata=()`).
+    fn metadata(&self) -> &Self::Metadata;
+
+    /// Returns the serialized representation of the metadata of this extension
+    /// type, or `None` if this extension type defines no metadata
+    /// (`Self::Metadata=()`).
+    ///
+    /// This is string value that is used for the
+    /// [`EXTENSION_TYPE_METADATA_KEY`] in the [`Field::metadata`] of a
+    /// [`Field`].
+    ///
+    /// [`Field`]: crate::Field
+    /// [`Field::metadata`]: crate::Field::metadata
+    fn serialize_metadata(&self) -> Option<String>;

Review Comment:
   This would be in conflict with the docs for `deserialize_metadata`: 
   
   > An extension type that defines no metadata should expect `None` for the serialized metadata and return `Ok(())`.
   
   > This function should return an error when **unexpected metadata is set (for extension types without metadata)**
   
   We could add a note as you suggested for `serialize_metadata` and then modify the comments in `deserialize_metadata` to be suggestions only?
   
   The `Uuid` canonical extension type requires no metadata, so if we change the recommendation here we should also change that implementation maybe?



-- 
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: github-unsubscribe@arrow.apache.org

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

Re: [PR] Add `ExtensionType` trait and `CanonicalExtensionType` enum [arrow-rs]

Posted by "mbrobbel (via GitHub)" <gi...@apache.org>.

mbrobbel commented on code in PR #5822:
URL: https://github.com/apache/arrow-rs/pull/5822#discussion_r1928880550


##########
arrow-schema/src/extension/mod.rs:
##########
@@ -0,0 +1,252 @@
+// 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.
+
+//! Extension types.
+
+#[cfg(feature = "canonical_extension_types")]
+mod canonical;
+#[cfg(feature = "canonical_extension_types")]
+pub use canonical::*;
+
+use crate::{ArrowError, DataType};
+
+/// The metadata key for the string name identifying an [`ExtensionType`].
+pub const EXTENSION_TYPE_NAME_KEY: &str = "ARROW:extension:name";
+
+/// The metadata key for a serialized representation of the [`ExtensionType`]
+/// necessary to reconstruct the custom type.
+pub const EXTENSION_TYPE_METADATA_KEY: &str = "ARROW:extension:metadata";
+
+/// Extension types.
+///
+/// User-defined “extension” types can be defined setting certain key value
+/// pairs in the [`Field`] metadata structure. These extension keys are:
+/// - [`EXTENSION_TYPE_NAME_KEY`]
+/// - [`EXTENSION_TYPE_METADATA_KEY`]
+///
+/// Canonical extension types support in this crate requires the
+/// `canonical_extension_types` feature.
+///
+/// Extension types may or may not use the [`EXTENSION_TYPE_METADATA_KEY`]
+/// field.
+///
+/// # Example
+///
+/// The example below demonstrates how to implement this trait for a `Uuid`
+/// type. Note this is not the canonical extension type for `Uuid`, which does
+/// not include information about the `Uuid` version.
+///
+/// ```
+/// # use arrow_schema::ArrowError;
+/// # fn main() -> Result<(), ArrowError> {
+/// use arrow_schema::{DataType, extension::ExtensionType, Field};
+/// use std::{fmt, str::FromStr};
+///
+/// /// The different Uuid versions.
+/// #[derive(Clone, Copy, Debug, PartialEq)]
+/// enum UuidVersion {
+///     V1,
+///     V2,
+///     V3,
+///     V4,
+///     V5,
+///     V6,
+///     V7,
+///     V8,
+/// }
+///
+/// // We'll use `Display` to serialize.
+/// impl fmt::Display for UuidVersion {
+///     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+///         write!(
+///             f,
+///             "{}",
+///             match self {
+///                 Self::V1 => "V1",
+///                 Self::V2 => "V2",
+///                 Self::V3 => "V3",
+///                 Self::V4 => "V4",
+///                 Self::V5 => "V5",
+///                 Self::V6 => "V6",
+///                 Self::V7 => "V7",
+///                 Self::V8 => "V8",
+///             }
+///         )
+///     }
+/// }
+///
+/// // And `FromStr` to deserialize.
+/// impl FromStr for UuidVersion {
+///     type Err = ArrowError;
+///
+///     fn from_str(s: &str) -> Result<Self, Self::Err> {
+///         match s {
+///             "V1" => Ok(Self::V1),
+///             "V2" => Ok(Self::V2),
+///             "V3" => Ok(Self::V3),
+///             "V4" => Ok(Self::V4),
+///             "V5" => Ok(Self::V5),
+///             "V6" => Ok(Self::V6),
+///             "V7" => Ok(Self::V7),
+///             "V8" => Ok(Self::V8),
+///             _ => Err(ArrowError::ParseError("Invalid UuidVersion".to_owned())),
+///         }
+///     }
+/// }
+///
+/// /// This is the extension type, not the container for Uuid values. It
+/// /// stores the Uuid version (this is the metadata of this extension type).
+/// #[derive(Clone, Copy, Debug, PartialEq)]
+/// struct Uuid(UuidVersion);
+///
+/// impl ExtensionType for Uuid {
+///     // We use a namespace as suggested by the specification.
+///     const NAME: &'static str = "myorg.example.uuid";
+///
+///     // The metadata type is the Uuid version.
+///     type Metadata = UuidVersion;
+///
+///     // We just return a reference to the Uuid version.
+///     fn metadata(&self) -> &Self::Metadata {
+///         &self.0
+///     }
+///
+///     // We use the `Display` implementation to serialize the Uuid
+///     // version.
+///     fn serialize_metadata(&self) -> Option<String> {
+///         Some(self.0.to_string())
+///     }
+///
+///     // We use the `FromStr` implementation to deserialize the Uuid
+///     // version.
+///     fn deserialize_metadata(metadata: Option<&str>) -> Result<Self::Metadata, ArrowError> {
+///         metadata.map_or_else(
+///             || {
+///                 Err(ArrowError::InvalidArgumentError(
+///                     "Uuid extension type metadata missing".to_owned(),
+///                 ))
+///             },
+///             str::parse,
+///         )
+///     }
+///
+///     // The only supported data type is `FixedSizeBinary(16)`.
+///     fn supports_data_type(&self, data_type: &DataType) -> Result<(), ArrowError> {
+///         match data_type {
+///             DataType::FixedSizeBinary(16) => Ok(()),
+///             data_type => Err(ArrowError::InvalidArgumentError(format!(
+///                 "Uuid data type mismatch, expected FixedSizeBinary(16), found {data_type}"
+///             ))),
+///         }
+///     }
+///
+///     // We should always check if the data type is supported before
+///     // constructing the extension type.
+///     fn try_new(data_type: &DataType, metadata: Self::Metadata) -> Result<Self, ArrowError> {
+///         let uuid = Self(metadata);
+///         uuid.supports_data_type(data_type)?;
+///         Ok(uuid)
+///     }
+/// }
+///
+/// // We can now construct the extension type.
+/// let uuid_v1 = Uuid(UuidVersion::V1);
+///
+/// // And add it to a field.
+/// let mut field =
+///     Field::new("", DataType::FixedSizeBinary(16), false).with_extension_type(uuid_v1);
+///
+/// // And extract it from this field.
+/// assert_eq!(field.try_extension_type::<Uuid>()?, uuid_v1);
+///
+/// // When we try to add this to a field with an unsupported data type we
+/// // get an error.
+/// let result = Field::new("", DataType::Null, false).try_with_extension_type(uuid_v1);
+/// assert!(result.is_err());
+/// # Ok(()) }
+/// ```
+///
+/// <https://arrow.apache.org/docs/format/Columnar.html#extension-types>
+///
+/// [`Field`]: crate::Field
+pub trait ExtensionType: Sized {
+    /// The name identifying this extension type.
+    ///
+    /// This is the string value that is used for the
+    /// [`EXTENSION_TYPE_NAME_KEY`] in the [`Field::metadata`] of a [`Field`]
+    /// to identify this extension type.
+    ///
+    /// We recommend that you use a “namespace”-style prefix for extension
+    /// type names to minimize the possibility of conflicts with multiple Arrow
+    /// readers and writers in the same application. For example, use
+    /// `myorg.name_of_type` instead of simply `name_of_type`.
+    ///
+    /// Extension names beginning with `arrow.` are reserved for canonical
+    /// extension types, they should not be used for third-party extension
+    /// types.
+    ///
+    /// Extension names are case-sensitive.
+    ///
+    /// [`Field`]: crate::Field
+    /// [`Field::metadata`]: crate::Field::metadata
+    const NAME: &'static str;
+
+    /// The metadata type of this extension type.
+    ///
+    /// If an extension type defines no metadata it should use `()` to indicate
+    /// this.
+    type Metadata;
+
+    /// Returns a reference to the metadata of this extension type, or `&()` if
+    /// if this extension type defines no metadata (`Self::Metadata=()`).
+    fn metadata(&self) -> &Self::Metadata;
+
+    /// Returns the serialized representation of the metadata of this extension
+    /// type, or `None` if this extension type defines no metadata
+    /// (`Self::Metadata=()`).
+    ///
+    /// This is string value that is used for the
+    /// [`EXTENSION_TYPE_METADATA_KEY`] in the [`Field::metadata`] of a
+    /// [`Field`].
+    ///
+    /// [`Field`]: crate::Field
+    /// [`Field::metadata`]: crate::Field::metadata
+    fn serialize_metadata(&self) -> Option<String>;

Review Comment:
   It looks like all the pieces are there (in `geoarrow-rs`), so I guess it will be straightforward to add `ExtensionType` implementations that can be used with the added extension type related methods of `Field`.



-- 
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: github-unsubscribe@arrow.apache.org

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

Re: [PR] Add `ExtensionType` trait and `CanonicalExtensionType` enum [arrow-rs]

Posted by "alamb (via GitHub)" <gi...@apache.org>.

alamb commented on PR #5822:
URL: https://github.com/apache/arrow-rs/pull/5822#issuecomment-2613426521

   I think we should leave this PR open at least through the weekend to gather additional feedback. I think given the size/scope of it perhaps we can plan to merge it after I cut the release candidate for the next arrow-rs release (mid-late next week) if there are no other concerns / comments.
   
   THank you again @mbrobbel  for pushing this along


-- 
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: github-unsubscribe@arrow.apache.org

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

Re: [PR] Add `ExtensionType` trait and `CanonicalExtensionType` enum [arrow-rs]

Posted by "mbrobbel (via GitHub)" <gi...@apache.org>.

mbrobbel commented on code in PR #5822:
URL: https://github.com/apache/arrow-rs/pull/5822#discussion_r1926010554


##########
arrow-schema/src/extension/mod.rs:
##########
@@ -0,0 +1,252 @@
+// 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.
+
+//! Extension types.
+
+#[cfg(feature = "canonical_extension_types")]
+mod canonical;
+#[cfg(feature = "canonical_extension_types")]
+pub use canonical::*;
+
+use crate::{ArrowError, DataType};
+
+/// The metadata key for the string name identifying an [`ExtensionType`].
+pub const EXTENSION_TYPE_NAME_KEY: &str = "ARROW:extension:name";
+
+/// The metadata key for a serialized representation of the [`ExtensionType`]
+/// necessary to reconstruct the custom type.
+pub const EXTENSION_TYPE_METADATA_KEY: &str = "ARROW:extension:metadata";
+
+/// Extension types.
+///
+/// User-defined “extension” types can be defined setting certain key value
+/// pairs in the [`Field`] metadata structure. These extension keys are:
+/// - [`EXTENSION_TYPE_NAME_KEY`]
+/// - [`EXTENSION_TYPE_METADATA_KEY`]
+///
+/// Canonical extension types support in this crate requires the
+/// `canonical_extension_types` feature.
+///
+/// Extension types may or may not use the [`EXTENSION_TYPE_METADATA_KEY`]
+/// field.
+///
+/// # Example
+///
+/// The example below demonstrates how to implement this trait for a `Uuid`
+/// type. Note this is not the canonical extension type for `Uuid`, which does
+/// not include information about the `Uuid` version.
+///
+/// ```
+/// # use arrow_schema::ArrowError;
+/// # fn main() -> Result<(), ArrowError> {
+/// use arrow_schema::{DataType, extension::ExtensionType, Field};
+/// use std::{fmt, str::FromStr};
+///
+/// /// The different Uuid versions.
+/// #[derive(Clone, Copy, Debug, PartialEq)]
+/// enum UuidVersion {
+///     V1,
+///     V2,
+///     V3,
+///     V4,
+///     V5,
+///     V6,
+///     V7,
+///     V8,
+/// }
+///
+/// // We'll use `Display` to serialize.
+/// impl fmt::Display for UuidVersion {
+///     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+///         write!(
+///             f,
+///             "{}",
+///             match self {
+///                 Self::V1 => "V1",
+///                 Self::V2 => "V2",
+///                 Self::V3 => "V3",
+///                 Self::V4 => "V4",
+///                 Self::V5 => "V5",
+///                 Self::V6 => "V6",
+///                 Self::V7 => "V7",
+///                 Self::V8 => "V8",
+///             }
+///         )
+///     }
+/// }
+///
+/// // And `FromStr` to deserialize.
+/// impl FromStr for UuidVersion {
+///     type Err = ArrowError;
+///
+///     fn from_str(s: &str) -> Result<Self, Self::Err> {
+///         match s {
+///             "V1" => Ok(Self::V1),
+///             "V2" => Ok(Self::V2),
+///             "V3" => Ok(Self::V3),
+///             "V4" => Ok(Self::V4),
+///             "V5" => Ok(Self::V5),
+///             "V6" => Ok(Self::V6),
+///             "V7" => Ok(Self::V7),
+///             "V8" => Ok(Self::V8),
+///             _ => Err(ArrowError::ParseError("Invalid UuidVersion".to_owned())),
+///         }
+///     }
+/// }
+///
+/// /// This is the extension type, not the container for Uuid values. It
+/// /// stores the Uuid version (this is the metadata of this extension type).
+/// #[derive(Clone, Copy, Debug, PartialEq)]
+/// struct Uuid(UuidVersion);
+///
+/// impl ExtensionType for Uuid {
+///     // We use a namespace as suggested by the specification.
+///     const NAME: &'static str = "myorg.example.uuid";
+///
+///     // The metadata type is the Uuid version.
+///     type Metadata = UuidVersion;
+///
+///     // We just return a reference to the Uuid version.
+///     fn metadata(&self) -> &Self::Metadata {
+///         &self.0
+///     }
+///
+///     // We use the `Display` implementation to serialize the Uuid
+///     // version.
+///     fn serialize_metadata(&self) -> Option<String> {
+///         Some(self.0.to_string())
+///     }
+///
+///     // We use the `FromStr` implementation to deserialize the Uuid
+///     // version.
+///     fn deserialize_metadata(metadata: Option<&str>) -> Result<Self::Metadata, ArrowError> {
+///         metadata.map_or_else(
+///             || {
+///                 Err(ArrowError::InvalidArgumentError(
+///                     "Uuid extension type metadata missing".to_owned(),
+///                 ))
+///             },
+///             str::parse,
+///         )
+///     }
+///
+///     // The only supported data type is `FixedSizeBinary(16)`.
+///     fn supports_data_type(&self, data_type: &DataType) -> Result<(), ArrowError> {
+///         match data_type {
+///             DataType::FixedSizeBinary(16) => Ok(()),
+///             data_type => Err(ArrowError::InvalidArgumentError(format!(
+///                 "Uuid data type mismatch, expected FixedSizeBinary(16), found {data_type}"
+///             ))),
+///         }
+///     }
+///
+///     // We should always check if the data type is supported before
+///     // constructing the extension type.
+///     fn try_new(data_type: &DataType, metadata: Self::Metadata) -> Result<Self, ArrowError> {
+///         let uuid = Self(metadata);
+///         uuid.supports_data_type(data_type)?;
+///         Ok(uuid)
+///     }
+/// }
+///
+/// // We can now construct the extension type.
+/// let uuid_v1 = Uuid(UuidVersion::V1);
+///
+/// // And add it to a field.
+/// let mut field =
+///     Field::new("", DataType::FixedSizeBinary(16), false).with_extension_type(uuid_v1);
+///
+/// // And extract it from this field.
+/// assert_eq!(field.try_extension_type::<Uuid>()?, uuid_v1);
+///
+/// // When we try to add this to a field with an unsupported data type we
+/// // get an error.
+/// let result = Field::new("", DataType::Null, false).try_with_extension_type(uuid_v1);
+/// assert!(result.is_err());
+/// # Ok(()) }
+/// ```
+///
+/// <https://arrow.apache.org/docs/format/Columnar.html#extension-types>
+///
+/// [`Field`]: crate::Field
+pub trait ExtensionType: Sized {
+    /// The name identifying this extension type.
+    ///
+    /// This is the string value that is used for the
+    /// [`EXTENSION_TYPE_NAME_KEY`] in the [`Field::metadata`] of a [`Field`]
+    /// to identify this extension type.
+    ///
+    /// We recommend that you use a “namespace”-style prefix for extension
+    /// type names to minimize the possibility of conflicts with multiple Arrow
+    /// readers and writers in the same application. For example, use
+    /// `myorg.name_of_type` instead of simply `name_of_type`.
+    ///
+    /// Extension names beginning with `arrow.` are reserved for canonical
+    /// extension types, they should not be used for third-party extension
+    /// types.
+    ///
+    /// Extension names are case-sensitive.
+    ///
+    /// [`Field`]: crate::Field
+    /// [`Field::metadata`]: crate::Field::metadata
+    const NAME: &'static str;
+
+    /// The metadata type of this extension type.
+    ///
+    /// If an extension type defines no metadata it should use `()` to indicate
+    /// this.
+    type Metadata;
+
+    /// Returns a reference to the metadata of this extension type, or `&()` if
+    /// if this extension type defines no metadata (`Self::Metadata=()`).
+    fn metadata(&self) -> &Self::Metadata;
+
+    /// Returns the serialized representation of the metadata of this extension
+    /// type, or `None` if this extension type defines no metadata
+    /// (`Self::Metadata=()`).
+    ///
+    /// This is string value that is used for the
+    /// [`EXTENSION_TYPE_METADATA_KEY`] in the [`Field::metadata`] of a
+    /// [`Field`].
+    ///
+    /// [`Field`]: crate::Field
+    /// [`Field::metadata`]: crate::Field::metadata
+    fn serialize_metadata(&self) -> Option<String>;

Review Comment:
   Ah this is great, I'll take a better look at this tomorrow and figure out if the current definition works with GeoArrow. 
   Note that in the spec extension types are defined for Field, not for Array.



-- 
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: github-unsubscribe@arrow.apache.org

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

Re: [PR] Add `ExtensionType` trait and `CanonicalExtensionType` enum [arrow-rs]

Posted by "paleolimbot (via GitHub)" <gi...@apache.org>.

paleolimbot commented on code in PR #5822:
URL: https://github.com/apache/arrow-rs/pull/5822#discussion_r1926065958


##########
arrow-schema/src/extension/mod.rs:
##########
@@ -0,0 +1,252 @@
+// 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.
+
+//! Extension types.
+
+#[cfg(feature = "canonical-extension-types")]
+mod canonical;
+#[cfg(feature = "canonical-extension-types")]
+pub use canonical::*;
+
+use crate::{ArrowError, DataType};
+
+/// The metadata key for the string name identifying an [`ExtensionType`].
+pub const EXTENSION_TYPE_NAME_KEY: &str = "ARROW:extension:name";
+
+/// The metadata key for a serialized representation of the [`ExtensionType`]
+/// necessary to reconstruct the custom type.
+pub const EXTENSION_TYPE_METADATA_KEY: &str = "ARROW:extension:metadata";
+
+/// Extension types.
+///
+/// User-defined “extension” types can be defined setting certain key value
+/// pairs in the [`Field`] metadata structure. These extension keys are:
+/// - [`EXTENSION_TYPE_NAME_KEY`]
+/// - [`EXTENSION_TYPE_METADATA_KEY`]
+///
+/// Canonical extension types support in this crate requires the
+/// `canonical-extension-types` feature.
+///
+/// Extension types may or may not use the [`EXTENSION_TYPE_METADATA_KEY`]
+/// field.
+///
+/// # Example
+///
+/// The example below demonstrates how to implement this trait for a `Uuid`
+/// type. Note this is not the canonical extension type for `Uuid`, which does
+/// not include information about the `Uuid` version.
+///
+/// ```
+/// # use arrow_schema::ArrowError;
+/// # fn main() -> Result<(), ArrowError> {
+/// use arrow_schema::{DataType, extension::ExtensionType, Field};
+/// use std::{fmt, str::FromStr};
+///
+/// /// The different Uuid versions.
+/// #[derive(Clone, Copy, Debug, PartialEq)]
+/// enum UuidVersion {
+///     V1,
+///     V2,
+///     V3,
+///     V4,
+///     V5,
+///     V6,
+///     V7,
+///     V8,
+/// }
+///
+/// // We'll use `Display` to serialize.
+/// impl fmt::Display for UuidVersion {
+///     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+///         write!(
+///             f,
+///             "{}",
+///             match self {
+///                 Self::V1 => "V1",
+///                 Self::V2 => "V2",
+///                 Self::V3 => "V3",
+///                 Self::V4 => "V4",
+///                 Self::V5 => "V5",
+///                 Self::V6 => "V6",
+///                 Self::V7 => "V7",
+///                 Self::V8 => "V8",
+///             }
+///         )
+///     }
+/// }
+///
+/// // And `FromStr` to deserialize.
+/// impl FromStr for UuidVersion {
+///     type Err = ArrowError;
+///
+///     fn from_str(s: &str) -> Result<Self, Self::Err> {
+///         match s {
+///             "V1" => Ok(Self::V1),
+///             "V2" => Ok(Self::V2),
+///             "V3" => Ok(Self::V3),
+///             "V4" => Ok(Self::V4),
+///             "V5" => Ok(Self::V5),
+///             "V6" => Ok(Self::V6),
+///             "V7" => Ok(Self::V7),
+///             "V8" => Ok(Self::V8),
+///             _ => Err(ArrowError::ParseError("Invalid UuidVersion".to_owned())),
+///         }
+///     }
+/// }
+///
+/// /// This is the extension type, not the container for Uuid values. It
+/// /// stores the Uuid version (this is the metadata of this extension type).
+/// #[derive(Clone, Copy, Debug, PartialEq)]
+/// struct Uuid(UuidVersion);
+///
+/// impl ExtensionType for Uuid {
+///     // We use a namespace as suggested by the specification.
+///     const NAME: &'static str = "myorg.example.uuid";
+///
+///     // The metadata type is the Uuid version.
+///     type Metadata = UuidVersion;
+///
+///     // We just return a reference to the Uuid version.
+///     fn metadata(&self) -> &Self::Metadata {
+///         &self.0
+///     }
+///
+///     // We use the `Display` implementation to serialize the Uuid
+///     // version.
+///     fn serialize_metadata(&self) -> Option<String> {
+///         Some(self.0.to_string())
+///     }
+///
+///     // We use the `FromStr` implementation to deserialize the Uuid
+///     // version.
+///     fn deserialize_metadata(metadata: Option<&str>) -> Result<Self::Metadata, ArrowError> {
+///         metadata.map_or_else(
+///             || {
+///                 Err(ArrowError::InvalidArgumentError(
+///                     "Uuid extension type metadata missing".to_owned(),
+///                 ))
+///             },
+///             str::parse,
+///         )
+///     }
+///
+///     // The only supported data type is `FixedSizeBinary(16)`.
+///     fn supports_data_type(&self, data_type: &DataType) -> Result<(), ArrowError> {
+///         match data_type {
+///             DataType::FixedSizeBinary(16) => Ok(()),
+///             data_type => Err(ArrowError::InvalidArgumentError(format!(
+///                 "Uuid data type mismatch, expected FixedSizeBinary(16), found {data_type}"
+///             ))),
+///         }
+///     }
+///
+///     // We should always check if the data type is supported before
+///     // constructing the extension type.
+///     fn try_new(data_type: &DataType, metadata: Self::Metadata) -> Result<Self, ArrowError> {
+///         let uuid = Self(metadata);
+///         uuid.supports_data_type(data_type)?;
+///         Ok(uuid)
+///     }
+/// }
+///
+/// // We can now construct the extension type.
+/// let uuid_v1 = Uuid(UuidVersion::V1);
+///
+/// // And add it to a field.
+/// let mut field =
+///     Field::new("", DataType::FixedSizeBinary(16), false).with_extension_type(uuid_v1);
+///
+/// // And extract it from this field.
+/// assert_eq!(field.try_extension_type::<Uuid>()?, uuid_v1);
+///
+/// // When we try to add this to a field with an unsupported data type we
+/// // get an error.
+/// let result = Field::new("", DataType::Null, false).try_with_extension_type(uuid_v1);
+/// assert!(result.is_err());
+/// # Ok(()) }
+/// ```
+///
+/// <https://arrow.apache.org/docs/format/Columnar.html#extension-types>
+///
+/// [`Field`]: crate::Field
+pub trait ExtensionType: Sized {
+    /// The name identifying this extension type.
+    ///
+    /// This is the string value that is used for the
+    /// [`EXTENSION_TYPE_NAME_KEY`] in the [`Field::metadata`] of a [`Field`]
+    /// to identify this extension type.
+    ///
+    /// We recommend that you use a “namespace”-style prefix for extension
+    /// type names to minimize the possibility of conflicts with multiple Arrow
+    /// readers and writers in the same application. For example, use
+    /// `myorg.name_of_type` instead of simply `name_of_type`.
+    ///
+    /// Extension names beginning with `arrow.` are reserved for canonical
+    /// extension types, they should not be used for third-party extension
+    /// types.
+    ///
+    /// Extension names are case-sensitive.
+    ///
+    /// [`Field`]: crate::Field
+    /// [`Field::metadata`]: crate::Field::metadata
+    const NAME: &'static str;
+
+    /// The metadata type of this extension type.
+    ///
+    /// If an extension type defines no metadata it should use `()` to indicate
+    /// this.
+    type Metadata;
+
+    /// Returns a reference to the metadata of this extension type, or `&()` if
+    /// if this extension type defines no metadata (`Self::Metadata=()`).
+    fn metadata(&self) -> &Self::Metadata;
+
+    /// Returns the serialized representation of the metadata of this extension
+    /// type, or `None` if this extension type defines no metadata
+    /// (`Self::Metadata=()`).
+    ///
+    /// This is string value that is used for the
+    /// [`EXTENSION_TYPE_METADATA_KEY`] in the [`Field::metadata`] of a
+    /// [`Field`].
+    ///
+    /// [`Field`]: crate::Field
+    /// [`Field::metadata`]: crate::Field::metadata
+    fn serialize_metadata(&self) -> Option<String>;

Review Comment:
   At least Arrow C++ and DuckDB will export `ARROW:extension:metadata` as an empty string if there are no parameters, so something will have to handle that (if they are interested in interacting with other Arrow implementations). In GeoArrow I personally accept `"{}"`, `""` or missing metadata.



-- 
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: github-unsubscribe@arrow.apache.org

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

Re: [PR] Add `ExtensionType` for `uuid` and map to parquet logical type [arrow-rs]

Posted by "mbrobbel (via GitHub)" <gi...@apache.org>.

mbrobbel commented on PR #5822:
URL: https://github.com/apache/arrow-rs/pull/5822#issuecomment-2377970403

   I updated the PR to define a trait for extension types instead. Ready for another round of feedback.


-- 
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: github-unsubscribe@arrow.apache.org

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

Re: [PR] Add `ExtensionType` trait and `CanonicalExtensionType` enum [arrow-rs]

Posted by "mbrobbel (via GitHub)" <gi...@apache.org>.

mbrobbel commented on code in PR #5822:
URL: https://github.com/apache/arrow-rs/pull/5822#discussion_r1928524109


##########
arrow-schema/src/extension/canonical/bool8.rs:
##########
@@ -0,0 +1,143 @@
+// 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.
+
+//! 8-bit Boolean
+//!
+//! <https://arrow.apache.org/docs/format/CanonicalExtensions.html#bit-boolean>
+
+use crate::{extension::ExtensionType, ArrowError, DataType};
+
+/// The extension type for `8-bit Boolean`.
+///
+/// Extension name: `arrow.bool8`.
+///
+/// The storage type of the extension is `Int8` where:
+/// - false is denoted by the value 0.
+/// - true can be specified using any non-zero value. Preferably 1.
+///
+/// <https://arrow.apache.org/docs/format/CanonicalExtensions.html#bit-boolean>
+#[derive(Debug, Default, Clone, Copy, PartialEq)]
+pub struct Bool8;
+
+impl ExtensionType for Bool8 {
+    const NAME: &'static str = "arrow.bool8";
+
+    type Metadata = &'static str;
+
+    fn metadata(&self) -> &Self::Metadata {
+        &""
+    }
+
+    fn serialize_metadata(&self) -> Option<String> {
+        Some(String::default())
+    }
+
+    fn deserialize_metadata(metadata: Option<&str>) -> Result<Self::Metadata, ArrowError> {
+        const ERR: &str = "Bool8 extension type expects an empty string as metadata";
+        metadata.map_or_else(
+            || Err(ArrowError::InvalidArgumentError(ERR.to_owned())),
+            |value| match value {
+                "" => Ok(""),
+                _ => Err(ArrowError::InvalidArgumentError(ERR.to_owned())),
+            },
+        )

Review Comment:
   [757b041](https://github.com/apache/arrow-rs/pull/5822/commits/757b041ee1a80735d1314dc905e299b6034535da)



-- 
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: github-unsubscribe@arrow.apache.org

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

Re: [PR] Add `ExtensionType` trait and `CanonicalExtensionType` enum [arrow-rs]

Posted by "mbrobbel (via GitHub)" <gi...@apache.org>.

mbrobbel commented on code in PR #5822:
URL: https://github.com/apache/arrow-rs/pull/5822#discussion_r1928558746


##########
arrow-schema/src/extension/canonical/json.rs:
##########
@@ -0,0 +1,189 @@
+// 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.
+
+//! JSON
+//!
+//! <https://arrow.apache.org/docs/format/CanonicalExtensions.html#json>
+
+use serde_json::Value;
+
+use crate::{extension::ExtensionType, ArrowError, DataType};
+
+/// The extension type for `JSON`.
+///
+/// Extension name: `arrow.json`.
+///
+/// The storage type of this extension is `String` or `LargeString` or
+/// `StringView`. Only UTF-8 encoded JSON as specified in [rfc8259](https://datatracker.ietf.org/doc/html/rfc8259)
+/// is supported.
+///
+/// This type does not have any parameters.
+///
+/// Metadata is either an empty string or a JSON string with an empty

Review Comment:
   I did consider doing this. The spec states (https://arrow.apache.org/docs/format/CanonicalExtensions.html#json):
   
   > Description of the serialization:
   > Metadata is either an empty string or a JSON string with an empty object. **In the future, additional fields may be added**, but they are not required to interpret the array.
   
   So I figured, I'll support the future use-case, however, if/when in the future fields are added here it would actually be better to use a strongly typed definition. I like your suggestion of using `Empty` here and since the metadata content currently is not `pub`, if fields are added in the future, we can just rename and modify the definition of the inner metadata type without introducing a breaking change.
   
   [c6f0443](https://github.com/apache/arrow-rs/pull/5822/commits/c6f0443c7cc6eff4d0765366166545d34ef91d69)
   
   



-- 
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: github-unsubscribe@arrow.apache.org

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

Re: [PR] Add `ExtensionType` trait and `CanonicalExtensionType` enum [arrow-rs]

Posted by "alamb (via GitHub)" <gi...@apache.org>.

alamb commented on PR #5822:
URL: https://github.com/apache/arrow-rs/pull/5822#issuecomment-2629356886

   I merged up from main to run the CI once more before merging this


-- 
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: github-unsubscribe@arrow.apache.org

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

Re: [PR] Add `ExtensionType` trait and `CanonicalExtensionType` enum [arrow-rs]

Posted by "paleolimbot (via GitHub)" <gi...@apache.org>.

paleolimbot commented on code in PR #5822:
URL: https://github.com/apache/arrow-rs/pull/5822#discussion_r1926078044


##########
arrow-schema/src/extension/mod.rs:
##########
@@ -0,0 +1,252 @@
+// 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.
+
+//! Extension types.
+
+#[cfg(feature = "canonical_extension_types")]
+mod canonical;
+#[cfg(feature = "canonical_extension_types")]
+pub use canonical::*;
+
+use crate::{ArrowError, DataType};
+
+/// The metadata key for the string name identifying an [`ExtensionType`].
+pub const EXTENSION_TYPE_NAME_KEY: &str = "ARROW:extension:name";
+
+/// The metadata key for a serialized representation of the [`ExtensionType`]
+/// necessary to reconstruct the custom type.
+pub const EXTENSION_TYPE_METADATA_KEY: &str = "ARROW:extension:metadata";
+
+/// Extension types.
+///
+/// User-defined “extension” types can be defined setting certain key value
+/// pairs in the [`Field`] metadata structure. These extension keys are:
+/// - [`EXTENSION_TYPE_NAME_KEY`]
+/// - [`EXTENSION_TYPE_METADATA_KEY`]
+///
+/// Canonical extension types support in this crate requires the
+/// `canonical_extension_types` feature.
+///
+/// Extension types may or may not use the [`EXTENSION_TYPE_METADATA_KEY`]
+/// field.
+///
+/// # Example
+///
+/// The example below demonstrates how to implement this trait for a `Uuid`
+/// type. Note this is not the canonical extension type for `Uuid`, which does
+/// not include information about the `Uuid` version.
+///
+/// ```
+/// # use arrow_schema::ArrowError;
+/// # fn main() -> Result<(), ArrowError> {
+/// use arrow_schema::{DataType, extension::ExtensionType, Field};
+/// use std::{fmt, str::FromStr};
+///
+/// /// The different Uuid versions.
+/// #[derive(Clone, Copy, Debug, PartialEq)]
+/// enum UuidVersion {
+///     V1,
+///     V2,
+///     V3,
+///     V4,
+///     V5,
+///     V6,
+///     V7,
+///     V8,
+/// }
+///
+/// // We'll use `Display` to serialize.
+/// impl fmt::Display for UuidVersion {
+///     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+///         write!(
+///             f,
+///             "{}",
+///             match self {
+///                 Self::V1 => "V1",
+///                 Self::V2 => "V2",
+///                 Self::V3 => "V3",
+///                 Self::V4 => "V4",
+///                 Self::V5 => "V5",
+///                 Self::V6 => "V6",
+///                 Self::V7 => "V7",
+///                 Self::V8 => "V8",
+///             }
+///         )
+///     }
+/// }
+///
+/// // And `FromStr` to deserialize.
+/// impl FromStr for UuidVersion {
+///     type Err = ArrowError;
+///
+///     fn from_str(s: &str) -> Result<Self, Self::Err> {
+///         match s {
+///             "V1" => Ok(Self::V1),
+///             "V2" => Ok(Self::V2),
+///             "V3" => Ok(Self::V3),
+///             "V4" => Ok(Self::V4),
+///             "V5" => Ok(Self::V5),
+///             "V6" => Ok(Self::V6),
+///             "V7" => Ok(Self::V7),
+///             "V8" => Ok(Self::V8),
+///             _ => Err(ArrowError::ParseError("Invalid UuidVersion".to_owned())),
+///         }
+///     }
+/// }
+///
+/// /// This is the extension type, not the container for Uuid values. It
+/// /// stores the Uuid version (this is the metadata of this extension type).
+/// #[derive(Clone, Copy, Debug, PartialEq)]
+/// struct Uuid(UuidVersion);
+///
+/// impl ExtensionType for Uuid {
+///     // We use a namespace as suggested by the specification.
+///     const NAME: &'static str = "myorg.example.uuid";
+///
+///     // The metadata type is the Uuid version.
+///     type Metadata = UuidVersion;
+///
+///     // We just return a reference to the Uuid version.
+///     fn metadata(&self) -> &Self::Metadata {
+///         &self.0
+///     }
+///
+///     // We use the `Display` implementation to serialize the Uuid
+///     // version.
+///     fn serialize_metadata(&self) -> Option<String> {
+///         Some(self.0.to_string())
+///     }
+///
+///     // We use the `FromStr` implementation to deserialize the Uuid
+///     // version.
+///     fn deserialize_metadata(metadata: Option<&str>) -> Result<Self::Metadata, ArrowError> {
+///         metadata.map_or_else(
+///             || {
+///                 Err(ArrowError::InvalidArgumentError(
+///                     "Uuid extension type metadata missing".to_owned(),
+///                 ))
+///             },
+///             str::parse,
+///         )
+///     }
+///
+///     // The only supported data type is `FixedSizeBinary(16)`.
+///     fn supports_data_type(&self, data_type: &DataType) -> Result<(), ArrowError> {
+///         match data_type {
+///             DataType::FixedSizeBinary(16) => Ok(()),
+///             data_type => Err(ArrowError::InvalidArgumentError(format!(
+///                 "Uuid data type mismatch, expected FixedSizeBinary(16), found {data_type}"
+///             ))),
+///         }
+///     }
+///
+///     // We should always check if the data type is supported before
+///     // constructing the extension type.
+///     fn try_new(data_type: &DataType, metadata: Self::Metadata) -> Result<Self, ArrowError> {
+///         let uuid = Self(metadata);
+///         uuid.supports_data_type(data_type)?;
+///         Ok(uuid)
+///     }
+/// }
+///
+/// // We can now construct the extension type.
+/// let uuid_v1 = Uuid(UuidVersion::V1);
+///
+/// // And add it to a field.
+/// let mut field =
+///     Field::new("", DataType::FixedSizeBinary(16), false).with_extension_type(uuid_v1);
+///
+/// // And extract it from this field.
+/// assert_eq!(field.try_extension_type::<Uuid>()?, uuid_v1);
+///
+/// // When we try to add this to a field with an unsupported data type we
+/// // get an error.
+/// let result = Field::new("", DataType::Null, false).try_with_extension_type(uuid_v1);
+/// assert!(result.is_err());
+/// # Ok(()) }
+/// ```
+///
+/// <https://arrow.apache.org/docs/format/Columnar.html#extension-types>
+///
+/// [`Field`]: crate::Field
+pub trait ExtensionType: Sized {
+    /// The name identifying this extension type.
+    ///
+    /// This is the string value that is used for the
+    /// [`EXTENSION_TYPE_NAME_KEY`] in the [`Field::metadata`] of a [`Field`]
+    /// to identify this extension type.
+    ///
+    /// We recommend that you use a “namespace”-style prefix for extension
+    /// type names to minimize the possibility of conflicts with multiple Arrow
+    /// readers and writers in the same application. For example, use
+    /// `myorg.name_of_type` instead of simply `name_of_type`.
+    ///
+    /// Extension names beginning with `arrow.` are reserved for canonical
+    /// extension types, they should not be used for third-party extension
+    /// types.
+    ///
+    /// Extension names are case-sensitive.
+    ///
+    /// [`Field`]: crate::Field
+    /// [`Field::metadata`]: crate::Field::metadata
+    const NAME: &'static str;
+
+    /// The metadata type of this extension type.
+    ///
+    /// If an extension type defines no metadata it should use `()` to indicate
+    /// this.
+    type Metadata;
+
+    /// Returns a reference to the metadata of this extension type, or `&()` if
+    /// if this extension type defines no metadata (`Self::Metadata=()`).
+    fn metadata(&self) -> &Self::Metadata;
+
+    /// Returns the serialized representation of the metadata of this extension
+    /// type, or `None` if this extension type defines no metadata
+    /// (`Self::Metadata=()`).
+    ///
+    /// This is string value that is used for the
+    /// [`EXTENSION_TYPE_METADATA_KEY`] in the [`Field::metadata`] of a
+    /// [`Field`].
+    ///
+    /// [`Field`]: crate::Field
+    /// [`Field::metadata`]: crate::Field::metadata
+    fn serialize_metadata(&self) -> Option<String>;

Review Comment:
   I consider the CRS a property of the type in geoarrow-c and geoarrow-python (which is also how it is implemented in the draft Parquet LogicalType, GeoParquet, and the draft Iceberg spec). My reading of this is that it's compatible with GeoArrow (and will be great!). (A Python implementation is here: https://github.com/geoarrow/geoarrow-python/blob/main/geoarrow-types/src/geoarrow/types/type_pyarrow.py ).
   
   It's true that CRS equality is thorny (generally it's expressed as a probability)...I implement this as part of the type and implement `==` on the type.
   



-- 
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: github-unsubscribe@arrow.apache.org

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

Re: [PR] Add `ExtensionType` trait and `CanonicalExtensionType` enum [arrow-rs]

Posted by "alamb (via GitHub)" <gi...@apache.org>.

alamb commented on code in PR #5822:
URL: https://github.com/apache/arrow-rs/pull/5822#discussion_r1925926421


##########
arrow-schema/src/extension/mod.rs:
##########
@@ -0,0 +1,252 @@
+// 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.
+
+//! Extension types.
+
+#[cfg(feature = "canonical-extension-types")]

Review Comment:
   I think adding a note in the module docs is good enough



-- 
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: github-unsubscribe@arrow.apache.org

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

Re: [PR] Add `ExtensionType` trait and `CanonicalExtensionType` enum [arrow-rs]

Posted by "alamb (via GitHub)" <gi...@apache.org>.

alamb commented on PR #5822:
URL: https://github.com/apache/arrow-rs/pull/5822#issuecomment-2605936622

   Thank you @mbrobbel  -- I plan to review this carefully tomorrow


-- 
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: github-unsubscribe@arrow.apache.org

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

Re: [PR] Add `ExtensionType` trait and `CanonicalExtensionType` enum [arrow-rs]

Posted by "kylebarron (via GitHub)" <gi...@apache.org>.

kylebarron commented on code in PR #5822:
URL: https://github.com/apache/arrow-rs/pull/5822#discussion_r2080287456


##########
arrow-schema/src/field.rs:
##########
@@ -350,6 +355,167 @@ impl Field {
         self
     }
 
+    /// Returns the extension type name of this [`Field`], if set.
+    ///
+    /// This returns the value of [`EXTENSION_TYPE_NAME_KEY`], if set in
+    /// [`Field::metadata`]. If the key is missing, there is no extension type
+    /// name and this returns `None`.
+    ///
+    /// # Example
+    ///
+    /// ```
+    /// # use arrow_schema::{DataType, extension::EXTENSION_TYPE_NAME_KEY, Field};
+    ///
+    /// let field = Field::new("", DataType::Null, false);
+    /// assert_eq!(field.extension_type_name(), None);
+    ///
+    /// let field = Field::new("", DataType::Null, false).with_metadata(
+    ///    [(EXTENSION_TYPE_NAME_KEY.to_owned(), "example".to_owned())]
+    ///        .into_iter()
+    ///        .collect(),
+    /// );
+    /// assert_eq!(field.extension_type_name(), Some("example"));
+    /// ```
+    pub fn extension_type_name(&self) -> Option<&str> {
+        self.metadata()
+            .get(EXTENSION_TYPE_NAME_KEY)
+            .map(String::as_ref)
+    }
+
+    /// Returns the extension type metadata of this [`Field`], if set.
+    ///
+    /// This returns the value of [`EXTENSION_TYPE_METADATA_KEY`], if set in
+    /// [`Field::metadata`]. If the key is missing, there is no extension type
+    /// metadata and this returns `None`.
+    ///
+    /// # Example
+    ///
+    /// ```
+    /// # use arrow_schema::{DataType, extension::EXTENSION_TYPE_METADATA_KEY, Field};
+    ///
+    /// let field = Field::new("", DataType::Null, false);
+    /// assert_eq!(field.extension_type_metadata(), None);
+    ///
+    /// let field = Field::new("", DataType::Null, false).with_metadata(
+    ///    [(EXTENSION_TYPE_METADATA_KEY.to_owned(), "example".to_owned())]
+    ///        .into_iter()
+    ///        .collect(),
+    /// );
+    /// assert_eq!(field.extension_type_metadata(), Some("example"));
+    /// ```
+    pub fn extension_type_metadata(&self) -> Option<&str> {
+        self.metadata()
+            .get(EXTENSION_TYPE_METADATA_KEY)
+            .map(String::as_ref)
+    }
+
+    /// Returns an instance of the given [`ExtensionType`] of this [`Field`],
+    /// if set in the [`Field::metadata`].
+    ///
+    /// # Error
+    ///
+    /// Returns an error if
+    /// - this field does not have the name of this extension type
+    ///   ([`ExtensionType::NAME`]) in the [`Field::metadata`] (mismatch or
+    ///   missing)
+    /// - the deserialization of the metadata
+    ///   ([`ExtensionType::deserialize_metadata`]) fails
+    /// - the construction of the extension type ([`ExtensionType::try_new`])
+    ///   fail (for example when the [`Field::data_type`] is not supported by
+    ///   the extension type ([`ExtensionType::supports_data_type`]))
+    pub fn try_extension_type<E: ExtensionType>(&self) -> Result<E, ArrowError> {
+        // Check the extension name in the metadata
+        match self.extension_type_name() {
+            // It should match the name of the given extension type
+            Some(name) if name == E::NAME => {
+                // Deserialize the metadata and try to construct the extension
+                // type
+                E::deserialize_metadata(self.extension_type_metadata())
+                    .and_then(|metadata| E::try_new(self.data_type(), metadata))
+            }
+            // Name mismatch
+            Some(name) => Err(ArrowError::InvalidArgumentError(format!(
+                "Field extension type name mismatch, expected {}, found {name}",
+                E::NAME
+            ))),
+            // Name missing
+            None => Err(ArrowError::InvalidArgumentError(
+                "Field extension type name missing".to_owned(),
+            )),
+        }
+    }
+
+    /// Returns an instance of the given [`ExtensionType`] of this [`Field`],
+    /// panics if this [`Field`] does not have this extension type.
+    ///
+    /// # Panic
+    ///
+    /// This calls [`Field::try_extension_type`] and panics when it returns an
+    /// error.
+    pub fn extension_type<E: ExtensionType>(&self) -> E {
+        self.try_extension_type::<E>()
+            .unwrap_or_else(|e| panic!("{e}"))
+    }
+
+    /// Updates the metadata of this [`Field`] with the [`ExtensionType::NAME`]
+    /// and [`ExtensionType::metadata`] of the given [`ExtensionType`], if the
+    /// given extension type supports the [`Field::data_type`] of this field
+    /// ([`ExtensionType::supports_data_type`]).
+    ///
+    /// If the given extension type defines no metadata, a previously set
+    /// value of [`EXTENSION_TYPE_METADATA_KEY`] is cleared.
+    ///
+    /// # Error
+    ///
+    /// This functions returns an error if the data type of this field does not
+    /// match any of the supported storage types of the given extension type.
+    pub fn try_with_extension_type<E: ExtensionType>(
+        &mut self,
+        extension_type: E,
+    ) -> Result<(), ArrowError> {
+        // Make sure the data type of this field is supported
+        extension_type.supports_data_type(&self.data_type)?;
+
+        self.metadata
+            .insert(EXTENSION_TYPE_NAME_KEY.to_owned(), E::NAME.to_owned());
+        match extension_type.serialize_metadata() {
+            Some(metadata) => self
+                .metadata
+                .insert(EXTENSION_TYPE_METADATA_KEY.to_owned(), metadata),
+            // If this extension type has no metadata, we make sure to
+            // clear previously set metadata.
+            None => self.metadata.remove(EXTENSION_TYPE_METADATA_KEY),
+        };
+
+        Ok(())
+    }
+
+    /// Updates the metadata of this [`Field`] with the [`ExtensionType::NAME`]
+    /// and [`ExtensionType::metadata`] of the given [`ExtensionType`].
+    ///
+    /// # Panics
+    ///
+    /// This calls [`Field::try_with_extension_type`] and panics when it
+    /// returns an error.
+    pub fn with_extension_type<E: ExtensionType>(mut self, extension_type: E) -> Self {

Review Comment:
   I'll just note it's a little weird to me that `with_extension_type` returns `Self` whereas `try_with_extension_type` returns `Result<()>`. I would've expected `try_with_extension_type` to return `Result<Self>`?



-- 
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: github-unsubscribe@arrow.apache.org

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

Re: [PR] Add `ExtensionType` trait and `CanonicalExtensionType` enum [arrow-rs]

Posted by "Tpt (via GitHub)" <gi...@apache.org>.

Tpt commented on code in PR #5822:
URL: https://github.com/apache/arrow-rs/pull/5822#discussion_r1927507731


##########
arrow-schema/src/extension/canonical/json.rs:
##########
@@ -0,0 +1,189 @@
+// 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.
+
+//! JSON
+//!
+//! <https://arrow.apache.org/docs/format/CanonicalExtensions.html#json>
+
+use serde_json::Value;
+
+use crate::{extension::ExtensionType, ArrowError, DataType};
+
+/// The extension type for `JSON`.
+///
+/// Extension name: `arrow.json`.
+///
+/// The storage type of this extension is `String` or `LargeString` or
+/// `StringView`. Only UTF-8 encoded JSON as specified in [rfc8259](https://datatracker.ietf.org/doc/html/rfc8259)
+/// is supported.
+///
+/// This type does not have any parameters.
+///
+/// Metadata is either an empty string or a JSON string with an empty

Review Comment:
   Based on implementation I guess it's not "an empty string" but "a JSON string with an empty string" (ie `"\"\""` and not `""`)



##########
arrow-schema/src/extension/canonical/bool8.rs:
##########
@@ -0,0 +1,143 @@
+// 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.
+
+//! 8-bit Boolean
+//!
+//! <https://arrow.apache.org/docs/format/CanonicalExtensions.html#bit-boolean>
+
+use crate::{extension::ExtensionType, ArrowError, DataType};
+
+/// The extension type for `8-bit Boolean`.
+///
+/// Extension name: `arrow.bool8`.
+///
+/// The storage type of the extension is `Int8` where:
+/// - false is denoted by the value 0.
+/// - true can be specified using any non-zero value. Preferably 1.
+///
+/// <https://arrow.apache.org/docs/format/CanonicalExtensions.html#bit-boolean>
+#[derive(Debug, Default, Clone, Copy, PartialEq)]
+pub struct Bool8;
+
+impl ExtensionType for Bool8 {
+    const NAME: &'static str = "arrow.bool8";
+
+    type Metadata = &'static str;
+
+    fn metadata(&self) -> &Self::Metadata {
+        &""
+    }
+
+    fn serialize_metadata(&self) -> Option<String> {
+        Some(String::default())
+    }
+
+    fn deserialize_metadata(metadata: Option<&str>) -> Result<Self::Metadata, ArrowError> {
+        const ERR: &str = "Bool8 extension type expects an empty string as metadata";
+        metadata.map_or_else(
+            || Err(ArrowError::InvalidArgumentError(ERR.to_owned())),
+            |value| match value {
+                "" => Ok(""),
+                _ => Err(ArrowError::InvalidArgumentError(ERR.to_owned())),
+            },
+        )

Review Comment:
   slightly shorter:
   ```suggestion
           if metadata.map_or(false, str::is_empty) {
              Ok("")
           } else {
              Err(ArrowError::InvalidArgumentError("Bool8 extension type expects an empty string as metadata".into()))
           }
   ```



##########
arrow-schema/src/extension/canonical/bool8.rs:
##########
@@ -0,0 +1,143 @@
+// 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.
+
+//! 8-bit Boolean
+//!
+//! <https://arrow.apache.org/docs/format/CanonicalExtensions.html#bit-boolean>
+
+use crate::{extension::ExtensionType, ArrowError, DataType};
+
+/// The extension type for `8-bit Boolean`.
+///
+/// Extension name: `arrow.bool8`.
+///
+/// The storage type of the extension is `Int8` where:
+/// - false is denoted by the value 0.
+/// - true can be specified using any non-zero value. Preferably 1.
+///
+/// <https://arrow.apache.org/docs/format/CanonicalExtensions.html#bit-boolean>
+#[derive(Debug, Default, Clone, Copy, PartialEq)]
+pub struct Bool8;
+
+impl ExtensionType for Bool8 {
+    const NAME: &'static str = "arrow.bool8";
+
+    type Metadata = &'static str;

Review Comment:
   Maybe bad idea: use `()` here. This way it's not possible to encode bad metadata value (any string that is not empty)



-- 
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: github-unsubscribe@arrow.apache.org

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

Re: [PR] Add `ExtensionType` trait and `CanonicalExtensionType` enum [arrow-rs]

Posted by "mbrobbel (via GitHub)" <gi...@apache.org>.

mbrobbel commented on code in PR #5822:
URL: https://github.com/apache/arrow-rs/pull/5822#discussion_r1926003195


##########
arrow-schema/src/extension/mod.rs:
##########
@@ -0,0 +1,252 @@
+// 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.
+
+//! Extension types.
+
+#[cfg(feature = "canonical-extension-types")]
+mod canonical;
+#[cfg(feature = "canonical-extension-types")]
+pub use canonical::*;
+
+use crate::{ArrowError, DataType};
+
+/// The metadata key for the string name identifying an [`ExtensionType`].
+pub const EXTENSION_TYPE_NAME_KEY: &str = "ARROW:extension:name";
+
+/// The metadata key for a serialized representation of the [`ExtensionType`]
+/// necessary to reconstruct the custom type.
+pub const EXTENSION_TYPE_METADATA_KEY: &str = "ARROW:extension:metadata";
+
+/// Extension types.
+///
+/// User-defined “extension” types can be defined setting certain key value
+/// pairs in the [`Field`] metadata structure. These extension keys are:
+/// - [`EXTENSION_TYPE_NAME_KEY`]
+/// - [`EXTENSION_TYPE_METADATA_KEY`]
+///
+/// Canonical extension types support in this crate requires the
+/// `canonical-extension-types` feature.
+///
+/// Extension types may or may not use the [`EXTENSION_TYPE_METADATA_KEY`]
+/// field.
+///
+/// # Example
+///
+/// The example below demonstrates how to implement this trait for a `Uuid`
+/// type. Note this is not the canonical extension type for `Uuid`, which does
+/// not include information about the `Uuid` version.
+///
+/// ```
+/// # use arrow_schema::ArrowError;
+/// # fn main() -> Result<(), ArrowError> {
+/// use arrow_schema::{DataType, extension::ExtensionType, Field};
+/// use std::{fmt, str::FromStr};
+///
+/// /// The different Uuid versions.
+/// #[derive(Clone, Copy, Debug, PartialEq)]
+/// enum UuidVersion {
+///     V1,
+///     V2,
+///     V3,
+///     V4,
+///     V5,
+///     V6,
+///     V7,
+///     V8,
+/// }
+///
+/// // We'll use `Display` to serialize.
+/// impl fmt::Display for UuidVersion {
+///     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+///         write!(
+///             f,
+///             "{}",
+///             match self {
+///                 Self::V1 => "V1",
+///                 Self::V2 => "V2",
+///                 Self::V3 => "V3",
+///                 Self::V4 => "V4",
+///                 Self::V5 => "V5",
+///                 Self::V6 => "V6",
+///                 Self::V7 => "V7",
+///                 Self::V8 => "V8",
+///             }
+///         )
+///     }
+/// }
+///
+/// // And `FromStr` to deserialize.
+/// impl FromStr for UuidVersion {
+///     type Err = ArrowError;
+///
+///     fn from_str(s: &str) -> Result<Self, Self::Err> {
+///         match s {
+///             "V1" => Ok(Self::V1),
+///             "V2" => Ok(Self::V2),
+///             "V3" => Ok(Self::V3),
+///             "V4" => Ok(Self::V4),
+///             "V5" => Ok(Self::V5),
+///             "V6" => Ok(Self::V6),
+///             "V7" => Ok(Self::V7),
+///             "V8" => Ok(Self::V8),
+///             _ => Err(ArrowError::ParseError("Invalid UuidVersion".to_owned())),
+///         }
+///     }
+/// }
+///
+/// /// This is the extension type, not the container for Uuid values. It
+/// /// stores the Uuid version (this is the metadata of this extension type).
+/// #[derive(Clone, Copy, Debug, PartialEq)]
+/// struct Uuid(UuidVersion);
+///
+/// impl ExtensionType for Uuid {
+///     // We use a namespace as suggested by the specification.
+///     const NAME: &'static str = "myorg.example.uuid";
+///
+///     // The metadata type is the Uuid version.
+///     type Metadata = UuidVersion;
+///
+///     // We just return a reference to the Uuid version.
+///     fn metadata(&self) -> &Self::Metadata {
+///         &self.0
+///     }
+///
+///     // We use the `Display` implementation to serialize the Uuid
+///     // version.
+///     fn serialize_metadata(&self) -> Option<String> {
+///         Some(self.0.to_string())
+///     }
+///
+///     // We use the `FromStr` implementation to deserialize the Uuid
+///     // version.
+///     fn deserialize_metadata(metadata: Option<&str>) -> Result<Self::Metadata, ArrowError> {
+///         metadata.map_or_else(
+///             || {
+///                 Err(ArrowError::InvalidArgumentError(
+///                     "Uuid extension type metadata missing".to_owned(),
+///                 ))
+///             },
+///             str::parse,
+///         )
+///     }
+///
+///     // The only supported data type is `FixedSizeBinary(16)`.
+///     fn supports_data_type(&self, data_type: &DataType) -> Result<(), ArrowError> {
+///         match data_type {
+///             DataType::FixedSizeBinary(16) => Ok(()),
+///             data_type => Err(ArrowError::InvalidArgumentError(format!(
+///                 "Uuid data type mismatch, expected FixedSizeBinary(16), found {data_type}"
+///             ))),
+///         }
+///     }
+///
+///     // We should always check if the data type is supported before
+///     // constructing the extension type.
+///     fn try_new(data_type: &DataType, metadata: Self::Metadata) -> Result<Self, ArrowError> {
+///         let uuid = Self(metadata);
+///         uuid.supports_data_type(data_type)?;
+///         Ok(uuid)
+///     }
+/// }
+///
+/// // We can now construct the extension type.
+/// let uuid_v1 = Uuid(UuidVersion::V1);
+///
+/// // And add it to a field.
+/// let mut field =
+///     Field::new("", DataType::FixedSizeBinary(16), false).with_extension_type(uuid_v1);
+///
+/// // And extract it from this field.
+/// assert_eq!(field.try_extension_type::<Uuid>()?, uuid_v1);
+///
+/// // When we try to add this to a field with an unsupported data type we
+/// // get an error.
+/// let result = Field::new("", DataType::Null, false).try_with_extension_type(uuid_v1);
+/// assert!(result.is_err());
+/// # Ok(()) }
+/// ```
+///
+/// <https://arrow.apache.org/docs/format/Columnar.html#extension-types>
+///
+/// [`Field`]: crate::Field
+pub trait ExtensionType: Sized {
+    /// The name identifying this extension type.
+    ///
+    /// This is the string value that is used for the
+    /// [`EXTENSION_TYPE_NAME_KEY`] in the [`Field::metadata`] of a [`Field`]
+    /// to identify this extension type.
+    ///
+    /// We recommend that you use a “namespace”-style prefix for extension
+    /// type names to minimize the possibility of conflicts with multiple Arrow
+    /// readers and writers in the same application. For example, use
+    /// `myorg.name_of_type` instead of simply `name_of_type`.
+    ///
+    /// Extension names beginning with `arrow.` are reserved for canonical
+    /// extension types, they should not be used for third-party extension
+    /// types.
+    ///
+    /// Extension names are case-sensitive.
+    ///
+    /// [`Field`]: crate::Field
+    /// [`Field::metadata`]: crate::Field::metadata
+    const NAME: &'static str;
+
+    /// The metadata type of this extension type.
+    ///
+    /// If an extension type defines no metadata it should use `()` to indicate
+    /// this.
+    type Metadata;

Review Comment:
   Unfortunately associated type defaults are not stabilized yet. I added a note in [b78e692](https://github.com/apache/arrow-rs/pull/5822/commits/b78e6925d76b35a739341f9a86cde91e58074ecc).



-- 
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: github-unsubscribe@arrow.apache.org

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

Re: [PR] Add `ExtensionType` trait and `CanonicalExtensionType` enum [arrow-rs]

Posted by "Tpt (via GitHub)" <gi...@apache.org>.

Tpt commented on code in PR #5822:
URL: https://github.com/apache/arrow-rs/pull/5822#discussion_r1928563550


##########
arrow-schema/src/extension/canonical/json.rs:
##########
@@ -0,0 +1,189 @@
+// 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.
+
+//! JSON
+//!
+//! <https://arrow.apache.org/docs/format/CanonicalExtensions.html#json>
+
+use serde_json::Value;
+
+use crate::{extension::ExtensionType, ArrowError, DataType};
+
+/// The extension type for `JSON`.
+///
+/// Extension name: `arrow.json`.
+///
+/// The storage type of this extension is `String` or `LargeString` or
+/// `StringView`. Only UTF-8 encoded JSON as specified in [rfc8259](https://datatracker.ietf.org/doc/html/rfc8259)
+/// is supported.
+///
+/// This type does not have any parameters.
+///
+/// Metadata is either an empty string or a JSON string with an empty

Review Comment:
   Great! It might be possible to also use a similar approach for the `Bool8` metadata.



-- 
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: github-unsubscribe@arrow.apache.org

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

Re: [PR] Add `ExtensionType` trait and `CanonicalExtensionType` enum [arrow-rs]

Posted by "alamb (via GitHub)" <gi...@apache.org>.

alamb commented on code in PR #5822:
URL: https://github.com/apache/arrow-rs/pull/5822#discussion_r1925339826


##########
arrow-schema/src/extension/mod.rs:
##########
@@ -0,0 +1,252 @@
+// 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.
+
+//! Extension types.
+
+#[cfg(feature = "canonical-extension-types")]
+mod canonical;
+#[cfg(feature = "canonical-extension-types")]
+pub use canonical::*;
+
+use crate::{ArrowError, DataType};
+
+/// The metadata key for the string name identifying an [`ExtensionType`].
+pub const EXTENSION_TYPE_NAME_KEY: &str = "ARROW:extension:name";
+
+/// The metadata key for a serialized representation of the [`ExtensionType`]
+/// necessary to reconstruct the custom type.
+pub const EXTENSION_TYPE_METADATA_KEY: &str = "ARROW:extension:metadata";
+
+/// Extension types.
+///
+/// User-defined “extension” types can be defined setting certain key value
+/// pairs in the [`Field`] metadata structure. These extension keys are:
+/// - [`EXTENSION_TYPE_NAME_KEY`]
+/// - [`EXTENSION_TYPE_METADATA_KEY`]
+///
+/// Canonical extension types support in this crate requires the
+/// `canonical-extension-types` feature.
+///
+/// Extension types may or may not use the [`EXTENSION_TYPE_METADATA_KEY`]
+/// field.
+///
+/// # Example
+///
+/// The example below demonstrates how to implement this trait for a `Uuid`
+/// type. Note this is not the canonical extension type for `Uuid`, which does
+/// not include information about the `Uuid` version.
+///
+/// ```
+/// # use arrow_schema::ArrowError;
+/// # fn main() -> Result<(), ArrowError> {
+/// use arrow_schema::{DataType, extension::ExtensionType, Field};
+/// use std::{fmt, str::FromStr};
+///
+/// /// The different Uuid versions.
+/// #[derive(Clone, Copy, Debug, PartialEq)]
+/// enum UuidVersion {
+///     V1,
+///     V2,
+///     V3,
+///     V4,
+///     V5,
+///     V6,
+///     V7,
+///     V8,
+/// }
+///
+/// // We'll use `Display` to serialize.
+/// impl fmt::Display for UuidVersion {
+///     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+///         write!(
+///             f,
+///             "{}",
+///             match self {
+///                 Self::V1 => "V1",
+///                 Self::V2 => "V2",
+///                 Self::V3 => "V3",
+///                 Self::V4 => "V4",
+///                 Self::V5 => "V5",
+///                 Self::V6 => "V6",
+///                 Self::V7 => "V7",
+///                 Self::V8 => "V8",
+///             }
+///         )
+///     }
+/// }
+///
+/// // And `FromStr` to deserialize.
+/// impl FromStr for UuidVersion {
+///     type Err = ArrowError;
+///
+///     fn from_str(s: &str) -> Result<Self, Self::Err> {
+///         match s {
+///             "V1" => Ok(Self::V1),
+///             "V2" => Ok(Self::V2),
+///             "V3" => Ok(Self::V3),
+///             "V4" => Ok(Self::V4),
+///             "V5" => Ok(Self::V5),
+///             "V6" => Ok(Self::V6),
+///             "V7" => Ok(Self::V7),
+///             "V8" => Ok(Self::V8),
+///             _ => Err(ArrowError::ParseError("Invalid UuidVersion".to_owned())),
+///         }
+///     }
+/// }
+///
+/// /// This is the extension type, not the container for Uuid values. It
+/// /// stores the Uuid version (this is the metadata of this extension type).
+/// #[derive(Clone, Copy, Debug, PartialEq)]
+/// struct Uuid(UuidVersion);
+///
+/// impl ExtensionType for Uuid {
+///     // We use a namespace as suggested by the specification.
+///     const NAME: &'static str = "myorg.example.uuid";
+///
+///     // The metadata type is the Uuid version.
+///     type Metadata = UuidVersion;
+///
+///     // We just return a reference to the Uuid version.
+///     fn metadata(&self) -> &Self::Metadata {
+///         &self.0
+///     }
+///
+///     // We use the `Display` implementation to serialize the Uuid
+///     // version.
+///     fn serialize_metadata(&self) -> Option<String> {
+///         Some(self.0.to_string())
+///     }
+///
+///     // We use the `FromStr` implementation to deserialize the Uuid
+///     // version.
+///     fn deserialize_metadata(metadata: Option<&str>) -> Result<Self::Metadata, ArrowError> {
+///         metadata.map_or_else(
+///             || {
+///                 Err(ArrowError::InvalidArgumentError(
+///                     "Uuid extension type metadata missing".to_owned(),
+///                 ))
+///             },
+///             str::parse,
+///         )
+///     }
+///
+///     // The only supported data type is `FixedSizeBinary(16)`.
+///     fn supports_data_type(&self, data_type: &DataType) -> Result<(), ArrowError> {
+///         match data_type {
+///             DataType::FixedSizeBinary(16) => Ok(()),
+///             data_type => Err(ArrowError::InvalidArgumentError(format!(
+///                 "Uuid data type mismatch, expected FixedSizeBinary(16), found {data_type}"
+///             ))),
+///         }
+///     }
+///
+///     // We should always check if the data type is supported before
+///     // constructing the extension type.
+///     fn try_new(data_type: &DataType, metadata: Self::Metadata) -> Result<Self, ArrowError> {
+///         let uuid = Self(metadata);
+///         uuid.supports_data_type(data_type)?;
+///         Ok(uuid)
+///     }
+/// }
+///
+/// // We can now construct the extension type.
+/// let uuid_v1 = Uuid(UuidVersion::V1);
+///
+/// // And add it to a field.
+/// let mut field =
+///     Field::new("", DataType::FixedSizeBinary(16), false).with_extension_type(uuid_v1);
+///
+/// // And extract it from this field.
+/// assert_eq!(field.try_extension_type::<Uuid>()?, uuid_v1);
+///
+/// // When we try to add this to a field with an unsupported data type we
+/// // get an error.
+/// let result = Field::new("", DataType::Null, false).try_with_extension_type(uuid_v1);
+/// assert!(result.is_err());
+/// # Ok(()) }
+/// ```
+///
+/// <https://arrow.apache.org/docs/format/Columnar.html#extension-types>
+///
+/// [`Field`]: crate::Field
+pub trait ExtensionType: Sized {
+    /// The name identifying this extension type.
+    ///
+    /// This is the string value that is used for the
+    /// [`EXTENSION_TYPE_NAME_KEY`] in the [`Field::metadata`] of a [`Field`]
+    /// to identify this extension type.
+    ///
+    /// We recommend that you use a “namespace”-style prefix for extension
+    /// type names to minimize the possibility of conflicts with multiple Arrow
+    /// readers and writers in the same application. For example, use
+    /// `myorg.name_of_type` instead of simply `name_of_type`.
+    ///
+    /// Extension names beginning with `arrow.` are reserved for canonical
+    /// extension types, they should not be used for third-party extension
+    /// types.
+    ///
+    /// Extension names are case-sensitive.
+    ///
+    /// [`Field`]: crate::Field
+    /// [`Field::metadata`]: crate::Field::metadata
+    const NAME: &'static str;
+
+    /// The metadata type of this extension type.
+    ///
+    /// If an extension type defines no metadata it should use `()` to indicate
+    /// this.
+    type Metadata;

Review Comment:
   I do think any metadata needs to be *serialized* to a string (which is the same as what the PR does)
   
   ```rust
       fn serialize_metadata(&self) -> Option<String>;
   ```
   
   I am just wondering why have a separate Rust type for Metadata
   
   For eample, if I were doing this my first inclination would be to do
   
   ```rust
   impl ExtensionType for MyAwesomeType {
       ...
       type Metadata = Self;
       ....
   }
   
   So then I could just serialize/deserialize instances of my awesome type



-- 
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: github-unsubscribe@arrow.apache.org

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

Re: [PR] Add `ExtensionType` trait and `CanonicalExtensionType` enum [arrow-rs]

Posted by "kylebarron (via GitHub)" <gi...@apache.org>.

kylebarron commented on code in PR #5822:
URL: https://github.com/apache/arrow-rs/pull/5822#discussion_r1925627198


##########
parquet/src/arrow/schema/mod.rs:
##########
@@ -2163,4 +2211,52 @@ mod tests {
     fn test_get_arrow_schema_from_metadata() {
         assert!(get_arrow_schema_from_metadata("").is_err());
     }
+
+    #[test]
+    #[cfg(feature = "arrow_canonical_extension_types")]
+    fn arrow_uuid_to_parquet_uuid() -> Result<()> {
+        let arrow_schema = Schema::new(vec![Field::new(
+            "uuid",
+            DataType::FixedSizeBinary(16),
+            false,
+        )
+        .with_extension_type(Uuid)]);
+
+        let parquet_schema = ArrowSchemaConverter::new().convert(&arrow_schema)?;
+
+        assert_eq!(
+            parquet_schema.column(0).logical_type(),
+            Some(LogicalType::Uuid)
+        );
+
+        // TODO: roundtrip
+        // let arrow_schema = parquet_to_arrow_schema(&parquet_schema, None)?;
+        // assert_eq!(arrow_schema.field(0).try_extension_type::<Uuid>()?, Uuid);
+
+        Ok(())
+    }
+
+    #[test]
+    #[cfg(feature = "arrow_canonical_extension_types")]
+    fn arrow_json_to_parquet_json() -> Result<()> {
+        let arrow_schema = Schema::new(vec![
+            Field::new("json", DataType::Utf8, false).with_extension_type(Json::default())
+        ]);
+
+        let parquet_schema = ArrowSchemaConverter::new().convert(&arrow_schema)?;
+
+        assert_eq!(
+            parquet_schema.column(0).logical_type(),
+            Some(LogicalType::Json)
+        );
+
+        // TODO: roundtrip
+        // let arrow_schema = parquet_to_arrow_schema(&parquet_schema, None)?;
+        // assert_eq!(
+        //     arrow_schema.field(0).try_extension_type::<Json>()?,
+        //     Json::default()
+        // );

Review Comment:
   Is this left for a future PR?



##########
arrow-schema/src/extension/mod.rs:
##########
@@ -0,0 +1,252 @@
+// 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.
+
+//! Extension types.
+
+#[cfg(feature = "canonical_extension_types")]
+mod canonical;
+#[cfg(feature = "canonical_extension_types")]
+pub use canonical::*;
+
+use crate::{ArrowError, DataType};
+
+/// The metadata key for the string name identifying an [`ExtensionType`].
+pub const EXTENSION_TYPE_NAME_KEY: &str = "ARROW:extension:name";
+
+/// The metadata key for a serialized representation of the [`ExtensionType`]
+/// necessary to reconstruct the custom type.
+pub const EXTENSION_TYPE_METADATA_KEY: &str = "ARROW:extension:metadata";
+
+/// Extension types.
+///
+/// User-defined “extension” types can be defined setting certain key value
+/// pairs in the [`Field`] metadata structure. These extension keys are:
+/// - [`EXTENSION_TYPE_NAME_KEY`]
+/// - [`EXTENSION_TYPE_METADATA_KEY`]
+///
+/// Canonical extension types support in this crate requires the
+/// `canonical_extension_types` feature.
+///
+/// Extension types may or may not use the [`EXTENSION_TYPE_METADATA_KEY`]
+/// field.
+///
+/// # Example
+///
+/// The example below demonstrates how to implement this trait for a `Uuid`
+/// type. Note this is not the canonical extension type for `Uuid`, which does
+/// not include information about the `Uuid` version.
+///
+/// ```
+/// # use arrow_schema::ArrowError;
+/// # fn main() -> Result<(), ArrowError> {
+/// use arrow_schema::{DataType, extension::ExtensionType, Field};
+/// use std::{fmt, str::FromStr};
+///
+/// /// The different Uuid versions.
+/// #[derive(Clone, Copy, Debug, PartialEq)]
+/// enum UuidVersion {
+///     V1,
+///     V2,
+///     V3,
+///     V4,
+///     V5,
+///     V6,
+///     V7,
+///     V8,
+/// }
+///
+/// // We'll use `Display` to serialize.
+/// impl fmt::Display for UuidVersion {
+///     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+///         write!(
+///             f,
+///             "{}",
+///             match self {
+///                 Self::V1 => "V1",
+///                 Self::V2 => "V2",
+///                 Self::V3 => "V3",
+///                 Self::V4 => "V4",
+///                 Self::V5 => "V5",
+///                 Self::V6 => "V6",
+///                 Self::V7 => "V7",
+///                 Self::V8 => "V8",
+///             }
+///         )
+///     }
+/// }
+///
+/// // And `FromStr` to deserialize.
+/// impl FromStr for UuidVersion {
+///     type Err = ArrowError;
+///
+///     fn from_str(s: &str) -> Result<Self, Self::Err> {
+///         match s {
+///             "V1" => Ok(Self::V1),
+///             "V2" => Ok(Self::V2),
+///             "V3" => Ok(Self::V3),
+///             "V4" => Ok(Self::V4),
+///             "V5" => Ok(Self::V5),
+///             "V6" => Ok(Self::V6),
+///             "V7" => Ok(Self::V7),
+///             "V8" => Ok(Self::V8),
+///             _ => Err(ArrowError::ParseError("Invalid UuidVersion".to_owned())),
+///         }
+///     }
+/// }
+///
+/// /// This is the extension type, not the container for Uuid values. It
+/// /// stores the Uuid version (this is the metadata of this extension type).
+/// #[derive(Clone, Copy, Debug, PartialEq)]
+/// struct Uuid(UuidVersion);
+///
+/// impl ExtensionType for Uuid {
+///     // We use a namespace as suggested by the specification.
+///     const NAME: &'static str = "myorg.example.uuid";
+///
+///     // The metadata type is the Uuid version.
+///     type Metadata = UuidVersion;
+///
+///     // We just return a reference to the Uuid version.
+///     fn metadata(&self) -> &Self::Metadata {
+///         &self.0
+///     }
+///
+///     // We use the `Display` implementation to serialize the Uuid
+///     // version.
+///     fn serialize_metadata(&self) -> Option<String> {
+///         Some(self.0.to_string())
+///     }
+///
+///     // We use the `FromStr` implementation to deserialize the Uuid
+///     // version.
+///     fn deserialize_metadata(metadata: Option<&str>) -> Result<Self::Metadata, ArrowError> {
+///         metadata.map_or_else(
+///             || {
+///                 Err(ArrowError::InvalidArgumentError(
+///                     "Uuid extension type metadata missing".to_owned(),
+///                 ))
+///             },
+///             str::parse,
+///         )
+///     }
+///
+///     // The only supported data type is `FixedSizeBinary(16)`.
+///     fn supports_data_type(&self, data_type: &DataType) -> Result<(), ArrowError> {
+///         match data_type {
+///             DataType::FixedSizeBinary(16) => Ok(()),
+///             data_type => Err(ArrowError::InvalidArgumentError(format!(
+///                 "Uuid data type mismatch, expected FixedSizeBinary(16), found {data_type}"
+///             ))),
+///         }
+///     }
+///
+///     // We should always check if the data type is supported before
+///     // constructing the extension type.
+///     fn try_new(data_type: &DataType, metadata: Self::Metadata) -> Result<Self, ArrowError> {
+///         let uuid = Self(metadata);
+///         uuid.supports_data_type(data_type)?;
+///         Ok(uuid)
+///     }
+/// }
+///
+/// // We can now construct the extension type.
+/// let uuid_v1 = Uuid(UuidVersion::V1);
+///
+/// // And add it to a field.
+/// let mut field =
+///     Field::new("", DataType::FixedSizeBinary(16), false).with_extension_type(uuid_v1);
+///
+/// // And extract it from this field.
+/// assert_eq!(field.try_extension_type::<Uuid>()?, uuid_v1);
+///
+/// // When we try to add this to a field with an unsupported data type we
+/// // get an error.
+/// let result = Field::new("", DataType::Null, false).try_with_extension_type(uuid_v1);
+/// assert!(result.is_err());
+/// # Ok(()) }
+/// ```
+///
+/// <https://arrow.apache.org/docs/format/Columnar.html#extension-types>
+///
+/// [`Field`]: crate::Field
+pub trait ExtensionType: Sized {
+    /// The name identifying this extension type.
+    ///
+    /// This is the string value that is used for the
+    /// [`EXTENSION_TYPE_NAME_KEY`] in the [`Field::metadata`] of a [`Field`]
+    /// to identify this extension type.
+    ///
+    /// We recommend that you use a “namespace”-style prefix for extension
+    /// type names to minimize the possibility of conflicts with multiple Arrow
+    /// readers and writers in the same application. For example, use
+    /// `myorg.name_of_type` instead of simply `name_of_type`.
+    ///
+    /// Extension names beginning with `arrow.` are reserved for canonical
+    /// extension types, they should not be used for third-party extension
+    /// types.
+    ///
+    /// Extension names are case-sensitive.
+    ///
+    /// [`Field`]: crate::Field
+    /// [`Field::metadata`]: crate::Field::metadata
+    const NAME: &'static str;
+
+    /// The metadata type of this extension type.
+    ///
+    /// If an extension type defines no metadata it should use `()` to indicate
+    /// this.
+    type Metadata;
+
+    /// Returns a reference to the metadata of this extension type, or `&()` if
+    /// if this extension type defines no metadata (`Self::Metadata=()`).
+    fn metadata(&self) -> &Self::Metadata;
+
+    /// Returns the serialized representation of the metadata of this extension
+    /// type, or `None` if this extension type defines no metadata
+    /// (`Self::Metadata=()`).
+    ///
+    /// This is string value that is used for the
+    /// [`EXTENSION_TYPE_METADATA_KEY`] in the [`Field::metadata`] of a
+    /// [`Field`].
+    ///
+    /// [`Field`]: crate::Field
+    /// [`Field::metadata`]: crate::Field::metadata
+    fn serialize_metadata(&self) -> Option<String>;

Review Comment:
   This is interesting because it requires the type to contain all information to construct its Arrow extension metadata. This is not how I've implemented it so far in geoarrow-rs. For example, I define a `Point` type as parameterized only by its "coordinate type" (i.e. `FixedSizeList` coordinates or `Struct[x: Float, y: Float]` coordinates) and its dimension (`XY`, `XYZ`, and so on). Separately, I store [more metadata](https://github.com/geoarrow/geoarrow-rs/blob/a2cbc7d501ea4fcd4d5e8927d09bde08ccc34124/rust/geoarrow/src/array/metadata.rs#L57-L81), and each array type [stores both pieces of data](https://github.com/geoarrow/geoarrow-rs/blob/a2cbc7d501ea4fcd4d5e8927d09bde08ccc34124/rust/geoarrow/src/array/point/array.rs#L27-L28).
   
   So the data type alone is not enough information to serialize the [full GeoArrow metadata](https://github.com/geoarrow/geoarrow/blob/dd06406d11f2a498b70b6c74f0deda62ec79067b/extension-types.md#extension-metadata), which also needs the [Coordinate Reference System (CRS)](https://en.wikipedia.org/wiki/Spatial_reference_system). It's a good question whether the CRS should be a part of the type here. Perhaps it should, but then we need to support CRS equality, which is quite tricky to do, so a single logical CRS can be represented in different ways.
   
   cc @paleolimbot , this might be a question for you.



##########
arrow-schema/src/extension/mod.rs:
##########
@@ -0,0 +1,252 @@
+// 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.
+
+//! Extension types.
+
+#[cfg(feature = "canonical_extension_types")]
+mod canonical;
+#[cfg(feature = "canonical_extension_types")]
+pub use canonical::*;
+
+use crate::{ArrowError, DataType};
+
+/// The metadata key for the string name identifying an [`ExtensionType`].
+pub const EXTENSION_TYPE_NAME_KEY: &str = "ARROW:extension:name";
+
+/// The metadata key for a serialized representation of the [`ExtensionType`]
+/// necessary to reconstruct the custom type.
+pub const EXTENSION_TYPE_METADATA_KEY: &str = "ARROW:extension:metadata";
+
+/// Extension types.
+///
+/// User-defined “extension” types can be defined setting certain key value
+/// pairs in the [`Field`] metadata structure. These extension keys are:
+/// - [`EXTENSION_TYPE_NAME_KEY`]
+/// - [`EXTENSION_TYPE_METADATA_KEY`]
+///
+/// Canonical extension types support in this crate requires the
+/// `canonical_extension_types` feature.
+///
+/// Extension types may or may not use the [`EXTENSION_TYPE_METADATA_KEY`]
+/// field.
+///
+/// # Example
+///
+/// The example below demonstrates how to implement this trait for a `Uuid`
+/// type. Note this is not the canonical extension type for `Uuid`, which does
+/// not include information about the `Uuid` version.
+///
+/// ```
+/// # use arrow_schema::ArrowError;
+/// # fn main() -> Result<(), ArrowError> {
+/// use arrow_schema::{DataType, extension::ExtensionType, Field};
+/// use std::{fmt, str::FromStr};
+///
+/// /// The different Uuid versions.
+/// #[derive(Clone, Copy, Debug, PartialEq)]
+/// enum UuidVersion {
+///     V1,
+///     V2,
+///     V3,
+///     V4,
+///     V5,
+///     V6,
+///     V7,
+///     V8,
+/// }
+///
+/// // We'll use `Display` to serialize.
+/// impl fmt::Display for UuidVersion {
+///     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+///         write!(
+///             f,
+///             "{}",
+///             match self {
+///                 Self::V1 => "V1",
+///                 Self::V2 => "V2",
+///                 Self::V3 => "V3",
+///                 Self::V4 => "V4",
+///                 Self::V5 => "V5",
+///                 Self::V6 => "V6",
+///                 Self::V7 => "V7",
+///                 Self::V8 => "V8",
+///             }
+///         )
+///     }
+/// }
+///
+/// // And `FromStr` to deserialize.
+/// impl FromStr for UuidVersion {
+///     type Err = ArrowError;
+///
+///     fn from_str(s: &str) -> Result<Self, Self::Err> {
+///         match s {
+///             "V1" => Ok(Self::V1),
+///             "V2" => Ok(Self::V2),
+///             "V3" => Ok(Self::V3),
+///             "V4" => Ok(Self::V4),
+///             "V5" => Ok(Self::V5),
+///             "V6" => Ok(Self::V6),
+///             "V7" => Ok(Self::V7),
+///             "V8" => Ok(Self::V8),
+///             _ => Err(ArrowError::ParseError("Invalid UuidVersion".to_owned())),
+///         }
+///     }
+/// }
+///
+/// /// This is the extension type, not the container for Uuid values. It
+/// /// stores the Uuid version (this is the metadata of this extension type).
+/// #[derive(Clone, Copy, Debug, PartialEq)]
+/// struct Uuid(UuidVersion);
+///
+/// impl ExtensionType for Uuid {
+///     // We use a namespace as suggested by the specification.
+///     const NAME: &'static str = "myorg.example.uuid";
+///
+///     // The metadata type is the Uuid version.
+///     type Metadata = UuidVersion;
+///
+///     // We just return a reference to the Uuid version.
+///     fn metadata(&self) -> &Self::Metadata {
+///         &self.0

Review Comment:
   My first go at implementing this trait I used:
   ```rs
   pub struct PointType(CoordType, Dimension);
   
   impl ExtensionType for PointType {
       const NAME: &'static str = "geoarrow.point";
   
       type Metadata = (CoordType, Dimension);
   
       fn metadata(&self) -> &Self::Metadata {
           &(self.0, self.1)
       }
   }
   ```
   But that won't work for this because `cannot return reference to temporary value`.
   
   So essentially `type Metadata` needs to be the same as whatever is defined inside the extension type struct, and that needs to be a single item, so that it can return `Self::Metadata` without creating any new items?



-- 
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: github-unsubscribe@arrow.apache.org

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

Re: [PR] Add `ExtensionType` trait and `CanonicalExtensionType` enum [arrow-rs]

Posted by "emilk (via GitHub)" <gi...@apache.org>.

emilk commented on PR #5822:
URL: https://github.com/apache/arrow-rs/pull/5822#issuecomment-2607025210

   This looks great to me! I really like this approach


-- 
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: github-unsubscribe@arrow.apache.org

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

Re: [PR] Add `ExtensionType` trait and `CanonicalExtensionType` enum [arrow-rs]

Posted by "mbrobbel (via GitHub)" <gi...@apache.org>.

mbrobbel commented on code in PR #5822:
URL: https://github.com/apache/arrow-rs/pull/5822#discussion_r1925554195


##########
arrow-schema/src/extension/mod.rs:
##########
@@ -0,0 +1,252 @@
+// 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.
+
+//! Extension types.
+
+#[cfg(feature = "canonical-extension-types")]
+mod canonical;
+#[cfg(feature = "canonical-extension-types")]
+pub use canonical::*;
+
+use crate::{ArrowError, DataType};
+
+/// The metadata key for the string name identifying an [`ExtensionType`].
+pub const EXTENSION_TYPE_NAME_KEY: &str = "ARROW:extension:name";
+
+/// The metadata key for a serialized representation of the [`ExtensionType`]
+/// necessary to reconstruct the custom type.
+pub const EXTENSION_TYPE_METADATA_KEY: &str = "ARROW:extension:metadata";
+
+/// Extension types.
+///
+/// User-defined “extension” types can be defined setting certain key value
+/// pairs in the [`Field`] metadata structure. These extension keys are:
+/// - [`EXTENSION_TYPE_NAME_KEY`]
+/// - [`EXTENSION_TYPE_METADATA_KEY`]
+///
+/// Canonical extension types support in this crate requires the
+/// `canonical-extension-types` feature.
+///
+/// Extension types may or may not use the [`EXTENSION_TYPE_METADATA_KEY`]
+/// field.
+///
+/// # Example
+///
+/// The example below demonstrates how to implement this trait for a `Uuid`
+/// type. Note this is not the canonical extension type for `Uuid`, which does
+/// not include information about the `Uuid` version.
+///
+/// ```
+/// # use arrow_schema::ArrowError;
+/// # fn main() -> Result<(), ArrowError> {
+/// use arrow_schema::{DataType, extension::ExtensionType, Field};
+/// use std::{fmt, str::FromStr};
+///
+/// /// The different Uuid versions.
+/// #[derive(Clone, Copy, Debug, PartialEq)]
+/// enum UuidVersion {
+///     V1,
+///     V2,
+///     V3,
+///     V4,
+///     V5,
+///     V6,
+///     V7,
+///     V8,
+/// }
+///
+/// // We'll use `Display` to serialize.
+/// impl fmt::Display for UuidVersion {
+///     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+///         write!(
+///             f,
+///             "{}",
+///             match self {
+///                 Self::V1 => "V1",
+///                 Self::V2 => "V2",
+///                 Self::V3 => "V3",
+///                 Self::V4 => "V4",
+///                 Self::V5 => "V5",
+///                 Self::V6 => "V6",
+///                 Self::V7 => "V7",
+///                 Self::V8 => "V8",
+///             }
+///         )
+///     }
+/// }
+///
+/// // And `FromStr` to deserialize.
+/// impl FromStr for UuidVersion {
+///     type Err = ArrowError;
+///
+///     fn from_str(s: &str) -> Result<Self, Self::Err> {
+///         match s {
+///             "V1" => Ok(Self::V1),
+///             "V2" => Ok(Self::V2),
+///             "V3" => Ok(Self::V3),
+///             "V4" => Ok(Self::V4),
+///             "V5" => Ok(Self::V5),
+///             "V6" => Ok(Self::V6),
+///             "V7" => Ok(Self::V7),
+///             "V8" => Ok(Self::V8),
+///             _ => Err(ArrowError::ParseError("Invalid UuidVersion".to_owned())),
+///         }
+///     }
+/// }
+///
+/// /// This is the extension type, not the container for Uuid values. It
+/// /// stores the Uuid version (this is the metadata of this extension type).
+/// #[derive(Clone, Copy, Debug, PartialEq)]
+/// struct Uuid(UuidVersion);
+///
+/// impl ExtensionType for Uuid {
+///     // We use a namespace as suggested by the specification.
+///     const NAME: &'static str = "myorg.example.uuid";
+///
+///     // The metadata type is the Uuid version.
+///     type Metadata = UuidVersion;
+///
+///     // We just return a reference to the Uuid version.
+///     fn metadata(&self) -> &Self::Metadata {
+///         &self.0
+///     }
+///
+///     // We use the `Display` implementation to serialize the Uuid
+///     // version.
+///     fn serialize_metadata(&self) -> Option<String> {
+///         Some(self.0.to_string())
+///     }
+///
+///     // We use the `FromStr` implementation to deserialize the Uuid
+///     // version.
+///     fn deserialize_metadata(metadata: Option<&str>) -> Result<Self::Metadata, ArrowError> {
+///         metadata.map_or_else(
+///             || {
+///                 Err(ArrowError::InvalidArgumentError(
+///                     "Uuid extension type metadata missing".to_owned(),
+///                 ))
+///             },
+///             str::parse,
+///         )
+///     }
+///
+///     // The only supported data type is `FixedSizeBinary(16)`.
+///     fn supports_data_type(&self, data_type: &DataType) -> Result<(), ArrowError> {
+///         match data_type {
+///             DataType::FixedSizeBinary(16) => Ok(()),
+///             data_type => Err(ArrowError::InvalidArgumentError(format!(
+///                 "Uuid data type mismatch, expected FixedSizeBinary(16), found {data_type}"
+///             ))),
+///         }
+///     }
+///
+///     // We should always check if the data type is supported before
+///     // constructing the extension type.
+///     fn try_new(data_type: &DataType, metadata: Self::Metadata) -> Result<Self, ArrowError> {
+///         let uuid = Self(metadata);
+///         uuid.supports_data_type(data_type)?;
+///         Ok(uuid)
+///     }
+/// }
+///
+/// // We can now construct the extension type.
+/// let uuid_v1 = Uuid(UuidVersion::V1);
+///
+/// // And add it to a field.
+/// let mut field =
+///     Field::new("", DataType::FixedSizeBinary(16), false).with_extension_type(uuid_v1);
+///
+/// // And extract it from this field.
+/// assert_eq!(field.try_extension_type::<Uuid>()?, uuid_v1);
+///
+/// // When we try to add this to a field with an unsupported data type we
+/// // get an error.
+/// let result = Field::new("", DataType::Null, false).try_with_extension_type(uuid_v1);
+/// assert!(result.is_err());
+/// # Ok(()) }
+/// ```
+///
+/// <https://arrow.apache.org/docs/format/Columnar.html#extension-types>
+///
+/// [`Field`]: crate::Field
+pub trait ExtensionType: Sized {
+    /// The name identifying this extension type.
+    ///
+    /// This is the string value that is used for the
+    /// [`EXTENSION_TYPE_NAME_KEY`] in the [`Field::metadata`] of a [`Field`]
+    /// to identify this extension type.
+    ///
+    /// We recommend that you use a “namespace”-style prefix for extension
+    /// type names to minimize the possibility of conflicts with multiple Arrow
+    /// readers and writers in the same application. For example, use
+    /// `myorg.name_of_type` instead of simply `name_of_type`.
+    ///
+    /// Extension names beginning with `arrow.` are reserved for canonical
+    /// extension types, they should not be used for third-party extension
+    /// types.
+    ///
+    /// Extension names are case-sensitive.
+    ///
+    /// [`Field`]: crate::Field
+    /// [`Field::metadata`]: crate::Field::metadata
+    const NAME: &'static str;
+
+    /// The metadata type of this extension type.
+    ///
+    /// If an extension type defines no metadata it should use `()` to indicate
+    /// this.
+    type Metadata;
+
+    /// Returns a reference to the metadata of this extension type, or `&()` if
+    /// if this extension type defines no metadata (`Self::Metadata=()`).
+    fn metadata(&self) -> &Self::Metadata;
+
+    /// Returns the serialized representation of the metadata of this extension
+    /// type, or `None` if this extension type defines no metadata
+    /// (`Self::Metadata=()`).
+    ///
+    /// This is string value that is used for the
+    /// [`EXTENSION_TYPE_METADATA_KEY`] in the [`Field::metadata`] of a
+    /// [`Field`].
+    ///
+    /// [`Field`]: crate::Field
+    /// [`Field::metadata`]: crate::Field::metadata
+    fn serialize_metadata(&self) -> Option<String>;

Review Comment:
   > It's your call
   
   I think this should be discussed with other contributors.



-- 
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: github-unsubscribe@arrow.apache.org

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

Re: [PR] Add `ExtensionType` trait and `CanonicalExtensionType` enum [arrow-rs]

Posted by "mbrobbel (via GitHub)" <gi...@apache.org>.

mbrobbel commented on code in PR #5822:
URL: https://github.com/apache/arrow-rs/pull/5822#discussion_r1925572238


##########
arrow-schema/src/extension/mod.rs:
##########
@@ -0,0 +1,252 @@
+// 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.
+
+//! Extension types.
+
+#[cfg(feature = "canonical-extension-types")]
+mod canonical;
+#[cfg(feature = "canonical-extension-types")]
+pub use canonical::*;
+
+use crate::{ArrowError, DataType};
+
+/// The metadata key for the string name identifying an [`ExtensionType`].
+pub const EXTENSION_TYPE_NAME_KEY: &str = "ARROW:extension:name";
+
+/// The metadata key for a serialized representation of the [`ExtensionType`]
+/// necessary to reconstruct the custom type.
+pub const EXTENSION_TYPE_METADATA_KEY: &str = "ARROW:extension:metadata";
+
+/// Extension types.
+///
+/// User-defined “extension” types can be defined setting certain key value
+/// pairs in the [`Field`] metadata structure. These extension keys are:
+/// - [`EXTENSION_TYPE_NAME_KEY`]
+/// - [`EXTENSION_TYPE_METADATA_KEY`]
+///
+/// Canonical extension types support in this crate requires the
+/// `canonical-extension-types` feature.
+///
+/// Extension types may or may not use the [`EXTENSION_TYPE_METADATA_KEY`]
+/// field.
+///
+/// # Example
+///
+/// The example below demonstrates how to implement this trait for a `Uuid`
+/// type. Note this is not the canonical extension type for `Uuid`, which does
+/// not include information about the `Uuid` version.
+///
+/// ```
+/// # use arrow_schema::ArrowError;
+/// # fn main() -> Result<(), ArrowError> {
+/// use arrow_schema::{DataType, extension::ExtensionType, Field};
+/// use std::{fmt, str::FromStr};
+///
+/// /// The different Uuid versions.
+/// #[derive(Clone, Copy, Debug, PartialEq)]
+/// enum UuidVersion {
+///     V1,
+///     V2,
+///     V3,
+///     V4,
+///     V5,
+///     V6,
+///     V7,
+///     V8,
+/// }
+///
+/// // We'll use `Display` to serialize.
+/// impl fmt::Display for UuidVersion {
+///     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+///         write!(
+///             f,
+///             "{}",
+///             match self {
+///                 Self::V1 => "V1",
+///                 Self::V2 => "V2",
+///                 Self::V3 => "V3",
+///                 Self::V4 => "V4",
+///                 Self::V5 => "V5",
+///                 Self::V6 => "V6",
+///                 Self::V7 => "V7",
+///                 Self::V8 => "V8",
+///             }
+///         )
+///     }
+/// }
+///
+/// // And `FromStr` to deserialize.
+/// impl FromStr for UuidVersion {
+///     type Err = ArrowError;
+///
+///     fn from_str(s: &str) -> Result<Self, Self::Err> {
+///         match s {
+///             "V1" => Ok(Self::V1),
+///             "V2" => Ok(Self::V2),
+///             "V3" => Ok(Self::V3),
+///             "V4" => Ok(Self::V4),
+///             "V5" => Ok(Self::V5),
+///             "V6" => Ok(Self::V6),
+///             "V7" => Ok(Self::V7),
+///             "V8" => Ok(Self::V8),
+///             _ => Err(ArrowError::ParseError("Invalid UuidVersion".to_owned())),
+///         }
+///     }
+/// }
+///
+/// /// This is the extension type, not the container for Uuid values. It
+/// /// stores the Uuid version (this is the metadata of this extension type).
+/// #[derive(Clone, Copy, Debug, PartialEq)]
+/// struct Uuid(UuidVersion);
+///
+/// impl ExtensionType for Uuid {
+///     // We use a namespace as suggested by the specification.
+///     const NAME: &'static str = "myorg.example.uuid";
+///
+///     // The metadata type is the Uuid version.
+///     type Metadata = UuidVersion;
+///
+///     // We just return a reference to the Uuid version.
+///     fn metadata(&self) -> &Self::Metadata {
+///         &self.0
+///     }
+///
+///     // We use the `Display` implementation to serialize the Uuid
+///     // version.
+///     fn serialize_metadata(&self) -> Option<String> {
+///         Some(self.0.to_string())
+///     }
+///
+///     // We use the `FromStr` implementation to deserialize the Uuid
+///     // version.
+///     fn deserialize_metadata(metadata: Option<&str>) -> Result<Self::Metadata, ArrowError> {
+///         metadata.map_or_else(
+///             || {
+///                 Err(ArrowError::InvalidArgumentError(
+///                     "Uuid extension type metadata missing".to_owned(),
+///                 ))
+///             },
+///             str::parse,
+///         )
+///     }
+///
+///     // The only supported data type is `FixedSizeBinary(16)`.
+///     fn supports_data_type(&self, data_type: &DataType) -> Result<(), ArrowError> {
+///         match data_type {
+///             DataType::FixedSizeBinary(16) => Ok(()),
+///             data_type => Err(ArrowError::InvalidArgumentError(format!(
+///                 "Uuid data type mismatch, expected FixedSizeBinary(16), found {data_type}"
+///             ))),
+///         }
+///     }
+///
+///     // We should always check if the data type is supported before
+///     // constructing the extension type.
+///     fn try_new(data_type: &DataType, metadata: Self::Metadata) -> Result<Self, ArrowError> {
+///         let uuid = Self(metadata);
+///         uuid.supports_data_type(data_type)?;
+///         Ok(uuid)
+///     }
+/// }
+///
+/// // We can now construct the extension type.
+/// let uuid_v1 = Uuid(UuidVersion::V1);
+///
+/// // And add it to a field.
+/// let mut field =
+///     Field::new("", DataType::FixedSizeBinary(16), false).with_extension_type(uuid_v1);
+///
+/// // And extract it from this field.
+/// assert_eq!(field.try_extension_type::<Uuid>()?, uuid_v1);
+///
+/// // When we try to add this to a field with an unsupported data type we
+/// // get an error.
+/// let result = Field::new("", DataType::Null, false).try_with_extension_type(uuid_v1);
+/// assert!(result.is_err());
+/// # Ok(()) }
+/// ```
+///
+/// <https://arrow.apache.org/docs/format/Columnar.html#extension-types>
+///
+/// [`Field`]: crate::Field
+pub trait ExtensionType: Sized {
+    /// The name identifying this extension type.
+    ///
+    /// This is the string value that is used for the
+    /// [`EXTENSION_TYPE_NAME_KEY`] in the [`Field::metadata`] of a [`Field`]
+    /// to identify this extension type.
+    ///
+    /// We recommend that you use a “namespace”-style prefix for extension
+    /// type names to minimize the possibility of conflicts with multiple Arrow
+    /// readers and writers in the same application. For example, use
+    /// `myorg.name_of_type` instead of simply `name_of_type`.
+    ///
+    /// Extension names beginning with `arrow.` are reserved for canonical
+    /// extension types, they should not be used for third-party extension
+    /// types.
+    ///
+    /// Extension names are case-sensitive.
+    ///
+    /// [`Field`]: crate::Field
+    /// [`Field::metadata`]: crate::Field::metadata
+    const NAME: &'static str;
+
+    /// The metadata type of this extension type.
+    ///
+    /// If an extension type defines no metadata it should use `()` to indicate
+    /// this.
+    type Metadata;
+
+    /// Returns a reference to the metadata of this extension type, or `&()` if
+    /// if this extension type defines no metadata (`Self::Metadata=()`).
+    fn metadata(&self) -> &Self::Metadata;
+
+    /// Returns the serialized representation of the metadata of this extension
+    /// type, or `None` if this extension type defines no metadata
+    /// (`Self::Metadata=()`).
+    ///
+    /// This is string value that is used for the
+    /// [`EXTENSION_TYPE_METADATA_KEY`] in the [`Field::metadata`] of a
+    /// [`Field`].
+    ///
+    /// [`Field`]: crate::Field
+    /// [`Field::metadata`]: crate::Field::metadata
+    fn serialize_metadata(&self) -> Option<String>;

Review Comment:
   Yes, I can imagine. Thanks for the suggestion.



-- 
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: github-unsubscribe@arrow.apache.org

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

Re: [PR] Add `ExtensionType` trait and `CanonicalExtensionType` enum [arrow-rs]

Posted by "alamb (via GitHub)" <gi...@apache.org>.

alamb commented on code in PR #5822:
URL: https://github.com/apache/arrow-rs/pull/5822#discussion_r1925930485


##########
arrow-schema/src/extension/mod.rs:
##########
@@ -0,0 +1,252 @@
+// 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.
+
+//! Extension types.
+
+#[cfg(feature = "canonical-extension-types")]
+mod canonical;
+#[cfg(feature = "canonical-extension-types")]
+pub use canonical::*;
+
+use crate::{ArrowError, DataType};
+
+/// The metadata key for the string name identifying an [`ExtensionType`].
+pub const EXTENSION_TYPE_NAME_KEY: &str = "ARROW:extension:name";
+
+/// The metadata key for a serialized representation of the [`ExtensionType`]
+/// necessary to reconstruct the custom type.
+pub const EXTENSION_TYPE_METADATA_KEY: &str = "ARROW:extension:metadata";
+
+/// Extension types.
+///
+/// User-defined “extension” types can be defined setting certain key value
+/// pairs in the [`Field`] metadata structure. These extension keys are:
+/// - [`EXTENSION_TYPE_NAME_KEY`]
+/// - [`EXTENSION_TYPE_METADATA_KEY`]
+///
+/// Canonical extension types support in this crate requires the
+/// `canonical-extension-types` feature.
+///
+/// Extension types may or may not use the [`EXTENSION_TYPE_METADATA_KEY`]
+/// field.
+///
+/// # Example
+///
+/// The example below demonstrates how to implement this trait for a `Uuid`
+/// type. Note this is not the canonical extension type for `Uuid`, which does
+/// not include information about the `Uuid` version.
+///
+/// ```
+/// # use arrow_schema::ArrowError;
+/// # fn main() -> Result<(), ArrowError> {
+/// use arrow_schema::{DataType, extension::ExtensionType, Field};
+/// use std::{fmt, str::FromStr};
+///
+/// /// The different Uuid versions.
+/// #[derive(Clone, Copy, Debug, PartialEq)]
+/// enum UuidVersion {
+///     V1,
+///     V2,
+///     V3,
+///     V4,
+///     V5,
+///     V6,
+///     V7,
+///     V8,
+/// }
+///
+/// // We'll use `Display` to serialize.
+/// impl fmt::Display for UuidVersion {
+///     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+///         write!(
+///             f,
+///             "{}",
+///             match self {
+///                 Self::V1 => "V1",
+///                 Self::V2 => "V2",
+///                 Self::V3 => "V3",
+///                 Self::V4 => "V4",
+///                 Self::V5 => "V5",
+///                 Self::V6 => "V6",
+///                 Self::V7 => "V7",
+///                 Self::V8 => "V8",
+///             }
+///         )
+///     }
+/// }
+///
+/// // And `FromStr` to deserialize.
+/// impl FromStr for UuidVersion {
+///     type Err = ArrowError;
+///
+///     fn from_str(s: &str) -> Result<Self, Self::Err> {
+///         match s {
+///             "V1" => Ok(Self::V1),
+///             "V2" => Ok(Self::V2),
+///             "V3" => Ok(Self::V3),
+///             "V4" => Ok(Self::V4),
+///             "V5" => Ok(Self::V5),
+///             "V6" => Ok(Self::V6),
+///             "V7" => Ok(Self::V7),
+///             "V8" => Ok(Self::V8),
+///             _ => Err(ArrowError::ParseError("Invalid UuidVersion".to_owned())),
+///         }
+///     }
+/// }
+///
+/// /// This is the extension type, not the container for Uuid values. It
+/// /// stores the Uuid version (this is the metadata of this extension type).
+/// #[derive(Clone, Copy, Debug, PartialEq)]
+/// struct Uuid(UuidVersion);
+///
+/// impl ExtensionType for Uuid {
+///     // We use a namespace as suggested by the specification.
+///     const NAME: &'static str = "myorg.example.uuid";
+///
+///     // The metadata type is the Uuid version.
+///     type Metadata = UuidVersion;
+///
+///     // We just return a reference to the Uuid version.
+///     fn metadata(&self) -> &Self::Metadata {
+///         &self.0
+///     }
+///
+///     // We use the `Display` implementation to serialize the Uuid
+///     // version.
+///     fn serialize_metadata(&self) -> Option<String> {
+///         Some(self.0.to_string())
+///     }
+///
+///     // We use the `FromStr` implementation to deserialize the Uuid
+///     // version.
+///     fn deserialize_metadata(metadata: Option<&str>) -> Result<Self::Metadata, ArrowError> {
+///         metadata.map_or_else(
+///             || {
+///                 Err(ArrowError::InvalidArgumentError(
+///                     "Uuid extension type metadata missing".to_owned(),
+///                 ))
+///             },
+///             str::parse,
+///         )
+///     }
+///
+///     // The only supported data type is `FixedSizeBinary(16)`.
+///     fn supports_data_type(&self, data_type: &DataType) -> Result<(), ArrowError> {
+///         match data_type {
+///             DataType::FixedSizeBinary(16) => Ok(()),
+///             data_type => Err(ArrowError::InvalidArgumentError(format!(
+///                 "Uuid data type mismatch, expected FixedSizeBinary(16), found {data_type}"
+///             ))),
+///         }
+///     }
+///
+///     // We should always check if the data type is supported before
+///     // constructing the extension type.
+///     fn try_new(data_type: &DataType, metadata: Self::Metadata) -> Result<Self, ArrowError> {
+///         let uuid = Self(metadata);
+///         uuid.supports_data_type(data_type)?;
+///         Ok(uuid)
+///     }
+/// }
+///
+/// // We can now construct the extension type.
+/// let uuid_v1 = Uuid(UuidVersion::V1);
+///
+/// // And add it to a field.
+/// let mut field =
+///     Field::new("", DataType::FixedSizeBinary(16), false).with_extension_type(uuid_v1);
+///
+/// // And extract it from this field.
+/// assert_eq!(field.try_extension_type::<Uuid>()?, uuid_v1);
+///
+/// // When we try to add this to a field with an unsupported data type we
+/// // get an error.
+/// let result = Field::new("", DataType::Null, false).try_with_extension_type(uuid_v1);
+/// assert!(result.is_err());
+/// # Ok(()) }
+/// ```
+///
+/// <https://arrow.apache.org/docs/format/Columnar.html#extension-types>
+///
+/// [`Field`]: crate::Field
+pub trait ExtensionType: Sized {
+    /// The name identifying this extension type.
+    ///
+    /// This is the string value that is used for the
+    /// [`EXTENSION_TYPE_NAME_KEY`] in the [`Field::metadata`] of a [`Field`]
+    /// to identify this extension type.
+    ///
+    /// We recommend that you use a “namespace”-style prefix for extension
+    /// type names to minimize the possibility of conflicts with multiple Arrow
+    /// readers and writers in the same application. For example, use
+    /// `myorg.name_of_type` instead of simply `name_of_type`.
+    ///
+    /// Extension names beginning with `arrow.` are reserved for canonical
+    /// extension types, they should not be used for third-party extension
+    /// types.
+    ///
+    /// Extension names are case-sensitive.
+    ///
+    /// [`Field`]: crate::Field
+    /// [`Field::metadata`]: crate::Field::metadata
+    const NAME: &'static str;
+
+    /// The metadata type of this extension type.
+    ///
+    /// If an extension type defines no metadata it should use `()` to indicate
+    /// this.
+    type Metadata;
+
+    /// Returns a reference to the metadata of this extension type, or `&()` if
+    /// if this extension type defines no metadata (`Self::Metadata=()`).
+    fn metadata(&self) -> &Self::Metadata;
+
+    /// Returns the serialized representation of the metadata of this extension
+    /// type, or `None` if this extension type defines no metadata
+    /// (`Self::Metadata=()`).
+    ///
+    /// This is string value that is used for the
+    /// [`EXTENSION_TYPE_METADATA_KEY`] in the [`Field::metadata`] of a
+    /// [`Field`].
+    ///
+    /// [`Field`]: crate::Field
+    /// [`Field::metadata`]: crate::Field::metadata
+    fn serialize_metadata(&self) -> Option<String>;

Review Comment:
   Maybe we can add a note in the comments and suggest for maximum compatibility extension types should always return `Some("")`



-- 
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: github-unsubscribe@arrow.apache.org

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

Re: [PR] Add `ExtensionType` trait and `CanonicalExtensionType` enum [arrow-rs]

Posted by "mbrobbel (via GitHub)" <gi...@apache.org>.

mbrobbel commented on code in PR #5822:
URL: https://github.com/apache/arrow-rs/pull/5822#discussion_r1928521220


##########
arrow-schema/src/extension/canonical/bool8.rs:
##########
@@ -0,0 +1,143 @@
+// 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.
+
+//! 8-bit Boolean
+//!
+//! <https://arrow.apache.org/docs/format/CanonicalExtensions.html#bit-boolean>
+
+use crate::{extension::ExtensionType, ArrowError, DataType};
+
+/// The extension type for `8-bit Boolean`.
+///
+/// Extension name: `arrow.bool8`.
+///
+/// The storage type of the extension is `Int8` where:
+/// - false is denoted by the value 0.
+/// - true can be specified using any non-zero value. Preferably 1.
+///
+/// <https://arrow.apache.org/docs/format/CanonicalExtensions.html#bit-boolean>
+#[derive(Debug, Default, Clone, Copy, PartialEq)]
+pub struct Bool8;
+
+impl ExtensionType for Bool8 {
+    const NAME: &'static str = "arrow.bool8";
+
+    type Metadata = &'static str;

Review Comment:
   With the proposed definition of the trait, `()` would indicate no metadata and results in the metadata key to be in unset in the field custom metadata map. Note that there is no actual storage for the metadata in this extension type. Both serialization and deserialization make sure the metadata is always an empty string.



-- 
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: github-unsubscribe@arrow.apache.org

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

Re: [PR] Add `ExtensionType` trait and `CanonicalExtensionType` enum [arrow-rs]

Posted by "alamb (via GitHub)" <gi...@apache.org>.

alamb commented on PR #5822:
URL: https://github.com/apache/arrow-rs/pull/5822#issuecomment-2626049967

   I'll merge this PR in the next day or two unless anyone else has any additional comments or wants more time to comment


-- 
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: github-unsubscribe@arrow.apache.org

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

Re: [PR] Add `ExtensionType` for `uuid` and map to parquet logical type [arrow-rs]

Posted by "mbrobbel (via GitHub)" <gi...@apache.org>.

mbrobbel commented on PR #5822:
URL: https://github.com/apache/arrow-rs/pull/5822#issuecomment-2176116551

   I haven't had time to work on this, but I'm planning to pick this up later.


-- 
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: github-unsubscribe@arrow.apache.org

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

Re: [PR] Add `ExtensionType` trait and `CanonicalExtensionType` enum [arrow-rs]

Posted by "alamb (via GitHub)" <gi...@apache.org>.

alamb merged PR #5822:
URL: https://github.com/apache/arrow-rs/pull/5822


-- 
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: github-unsubscribe@arrow.apache.org

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

Re: [PR] Add `ExtensionType` trait and `CanonicalExtensionType` enum [arrow-rs]

Posted by "alamb (via GitHub)" <gi...@apache.org>.

alamb commented on code in PR #5822:
URL: https://github.com/apache/arrow-rs/pull/5822#discussion_r1925125651


##########
arrow-schema/src/extension/mod.rs:
##########
@@ -0,0 +1,252 @@
+// 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.
+
+//! Extension types.
+
+#[cfg(feature = "canonical-extension-types")]
+mod canonical;
+#[cfg(feature = "canonical-extension-types")]
+pub use canonical::*;
+
+use crate::{ArrowError, DataType};
+
+/// The metadata key for the string name identifying an [`ExtensionType`].
+pub const EXTENSION_TYPE_NAME_KEY: &str = "ARROW:extension:name";
+
+/// The metadata key for a serialized representation of the [`ExtensionType`]
+/// necessary to reconstruct the custom type.
+pub const EXTENSION_TYPE_METADATA_KEY: &str = "ARROW:extension:metadata";
+
+/// Extension types.
+///
+/// User-defined “extension” types can be defined setting certain key value
+/// pairs in the [`Field`] metadata structure. These extension keys are:
+/// - [`EXTENSION_TYPE_NAME_KEY`]
+/// - [`EXTENSION_TYPE_METADATA_KEY`]
+///
+/// Canonical extension types support in this crate requires the
+/// `canonical-extension-types` feature.
+///
+/// Extension types may or may not use the [`EXTENSION_TYPE_METADATA_KEY`]
+/// field.
+///
+/// # Example
+///
+/// The example below demonstrates how to implement this trait for a `Uuid`

Review Comment:
   this is a really nice example and illustrates how this API works



##########
arrow-schema/src/extension/mod.rs:
##########
@@ -0,0 +1,252 @@
+// 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.
+
+//! Extension types.
+
+#[cfg(feature = "canonical-extension-types")]

Review Comment:
   💯 for making this behind a feature flag
   
   It would be nice to add this to the documentation: 
   - https://github.com/apache/arrow-rs/tree/main/arrow#feature-flags
   
   Perhaps we could also note it is "experimental" and say we reserve the right to make breaking changes between even minor version releases -- that would give us flexibilty to iterate in the short term if necessary



##########
arrow-schema/src/extension/canonical/mod.rs:
##########
@@ -0,0 +1,142 @@
+// 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.
+
+//! Canonical extension types.
+//!
+//! The Arrow columnar format allows defining extension types so as to extend
+//! standard Arrow data types with custom semantics. Often these semantics will
+//! be specific to a system or application. However, it is beneficial to share
+//! the definitions of well-known extension types so as to improve
+//! interoperability between different systems integrating Arrow columnar data.
+//!
+//! <https://arrow.apache.org/docs/format/CanonicalExtensions.html#format-canonical-extensions>
+
+mod bool8;
+pub use bool8::Bool8;
+mod fixed_shape_tensor;
+pub use fixed_shape_tensor::{FixedShapeTensor, FixedShapeTensorMetadata};
+mod json;
+pub use json::{Json, JsonMetadata};
+mod opaque;
+pub use opaque::{Opaque, OpaqueMetadata};
+mod uuid;
+pub use uuid::Uuid;
+mod variable_shape_tensor;
+pub use variable_shape_tensor::{VariableShapeTensor, VariableShapeTensorMetadata};
+
+use crate::{ArrowError, Field};
+
+use super::ExtensionType;
+
+/// Canonical extension types.
+///
+/// <https://arrow.apache.org/docs/format/CanonicalExtensions.html#format-canonical-extensions>
+#[non_exhaustive]

Review Comment:
   👍  for non exhaustive so we can add new extension types without breaking API changes



##########
arrow-schema/src/extension/mod.rs:
##########
@@ -0,0 +1,252 @@
+// 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.
+
+//! Extension types.
+
+#[cfg(feature = "canonical-extension-types")]
+mod canonical;
+#[cfg(feature = "canonical-extension-types")]
+pub use canonical::*;
+
+use crate::{ArrowError, DataType};
+
+/// The metadata key for the string name identifying an [`ExtensionType`].
+pub const EXTENSION_TYPE_NAME_KEY: &str = "ARROW:extension:name";
+
+/// The metadata key for a serialized representation of the [`ExtensionType`]
+/// necessary to reconstruct the custom type.
+pub const EXTENSION_TYPE_METADATA_KEY: &str = "ARROW:extension:metadata";
+
+/// Extension types.
+///
+/// User-defined “extension” types can be defined setting certain key value
+/// pairs in the [`Field`] metadata structure. These extension keys are:
+/// - [`EXTENSION_TYPE_NAME_KEY`]
+/// - [`EXTENSION_TYPE_METADATA_KEY`]
+///
+/// Canonical extension types support in this crate requires the
+/// `canonical-extension-types` feature.
+///
+/// Extension types may or may not use the [`EXTENSION_TYPE_METADATA_KEY`]
+/// field.
+///
+/// # Example
+///
+/// The example below demonstrates how to implement this trait for a `Uuid`
+/// type. Note this is not the canonical extension type for `Uuid`, which does
+/// not include information about the `Uuid` version.
+///
+/// ```
+/// # use arrow_schema::ArrowError;
+/// # fn main() -> Result<(), ArrowError> {
+/// use arrow_schema::{DataType, extension::ExtensionType, Field};
+/// use std::{fmt, str::FromStr};
+///
+/// /// The different Uuid versions.
+/// #[derive(Clone, Copy, Debug, PartialEq)]
+/// enum UuidVersion {
+///     V1,
+///     V2,
+///     V3,
+///     V4,
+///     V5,
+///     V6,
+///     V7,
+///     V8,
+/// }
+///
+/// // We'll use `Display` to serialize.
+/// impl fmt::Display for UuidVersion {
+///     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+///         write!(
+///             f,
+///             "{}",
+///             match self {
+///                 Self::V1 => "V1",
+///                 Self::V2 => "V2",
+///                 Self::V3 => "V3",
+///                 Self::V4 => "V4",
+///                 Self::V5 => "V5",
+///                 Self::V6 => "V6",
+///                 Self::V7 => "V7",
+///                 Self::V8 => "V8",
+///             }
+///         )
+///     }
+/// }
+///
+/// // And `FromStr` to deserialize.
+/// impl FromStr for UuidVersion {
+///     type Err = ArrowError;
+///
+///     fn from_str(s: &str) -> Result<Self, Self::Err> {
+///         match s {
+///             "V1" => Ok(Self::V1),
+///             "V2" => Ok(Self::V2),
+///             "V3" => Ok(Self::V3),
+///             "V4" => Ok(Self::V4),
+///             "V5" => Ok(Self::V5),
+///             "V6" => Ok(Self::V6),
+///             "V7" => Ok(Self::V7),
+///             "V8" => Ok(Self::V8),
+///             _ => Err(ArrowError::ParseError("Invalid UuidVersion".to_owned())),
+///         }
+///     }
+/// }
+///
+/// /// This is the extension type, not the container for Uuid values. It
+/// /// stores the Uuid version (this is the metadata of this extension type).
+/// #[derive(Clone, Copy, Debug, PartialEq)]
+/// struct Uuid(UuidVersion);
+///
+/// impl ExtensionType for Uuid {
+///     // We use a namespace as suggested by the specification.
+///     const NAME: &'static str = "myorg.example.uuid";
+///
+///     // The metadata type is the Uuid version.
+///     type Metadata = UuidVersion;
+///
+///     // We just return a reference to the Uuid version.
+///     fn metadata(&self) -> &Self::Metadata {
+///         &self.0
+///     }
+///
+///     // We use the `Display` implementation to serialize the Uuid
+///     // version.
+///     fn serialize_metadata(&self) -> Option<String> {
+///         Some(self.0.to_string())
+///     }
+///
+///     // We use the `FromStr` implementation to deserialize the Uuid
+///     // version.
+///     fn deserialize_metadata(metadata: Option<&str>) -> Result<Self::Metadata, ArrowError> {
+///         metadata.map_or_else(
+///             || {
+///                 Err(ArrowError::InvalidArgumentError(
+///                     "Uuid extension type metadata missing".to_owned(),
+///                 ))
+///             },
+///             str::parse,
+///         )
+///     }
+///
+///     // The only supported data type is `FixedSizeBinary(16)`.
+///     fn supports_data_type(&self, data_type: &DataType) -> Result<(), ArrowError> {
+///         match data_type {
+///             DataType::FixedSizeBinary(16) => Ok(()),
+///             data_type => Err(ArrowError::InvalidArgumentError(format!(
+///                 "Uuid data type mismatch, expected FixedSizeBinary(16), found {data_type}"
+///             ))),
+///         }
+///     }
+///
+///     // We should always check if the data type is supported before
+///     // constructing the extension type.
+///     fn try_new(data_type: &DataType, metadata: Self::Metadata) -> Result<Self, ArrowError> {
+///         let uuid = Self(metadata);
+///         uuid.supports_data_type(data_type)?;
+///         Ok(uuid)
+///     }
+/// }
+///
+/// // We can now construct the extension type.
+/// let uuid_v1 = Uuid(UuidVersion::V1);
+///
+/// // And add it to a field.
+/// let mut field =
+///     Field::new("", DataType::FixedSizeBinary(16), false).with_extension_type(uuid_v1);
+///
+/// // And extract it from this field.
+/// assert_eq!(field.try_extension_type::<Uuid>()?, uuid_v1);
+///
+/// // When we try to add this to a field with an unsupported data type we
+/// // get an error.
+/// let result = Field::new("", DataType::Null, false).try_with_extension_type(uuid_v1);
+/// assert!(result.is_err());
+/// # Ok(()) }
+/// ```
+///
+/// <https://arrow.apache.org/docs/format/Columnar.html#extension-types>
+///
+/// [`Field`]: crate::Field
+pub trait ExtensionType: Sized {
+    /// The name identifying this extension type.
+    ///
+    /// This is the string value that is used for the
+    /// [`EXTENSION_TYPE_NAME_KEY`] in the [`Field::metadata`] of a [`Field`]
+    /// to identify this extension type.
+    ///
+    /// We recommend that you use a “namespace”-style prefix for extension
+    /// type names to minimize the possibility of conflicts with multiple Arrow
+    /// readers and writers in the same application. For example, use
+    /// `myorg.name_of_type` instead of simply `name_of_type`.
+    ///
+    /// Extension names beginning with `arrow.` are reserved for canonical
+    /// extension types, they should not be used for third-party extension
+    /// types.
+    ///
+    /// Extension names are case-sensitive.
+    ///
+    /// [`Field`]: crate::Field
+    /// [`Field::metadata`]: crate::Field::metadata
+    const NAME: &'static str;
+
+    /// The metadata type of this extension type.
+    ///
+    /// If an extension type defines no metadata it should use `()` to indicate
+    /// this.
+    type Metadata;

Review Comment:
   I wonder what is the usecase for the Metadata indirection? 
   
   For instance, the example above seems like it would be simpler if it simply serialized / deserialized `UuidVersion` directly
   
   I think that would mean the API would look like
   ```rust
   trait ExtensionType {
     // serialize self to a string
     fn serialize(&self) -> Option<String>;
     // deserialize into self
     fn deserialize(metadata: Option<&str>) -> Result<Self, ArrowError>;
   }
   ```



##########
arrow-schema/src/extension/mod.rs:
##########
@@ -0,0 +1,252 @@
+// 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.
+
+//! Extension types.
+
+#[cfg(feature = "canonical-extension-types")]
+mod canonical;
+#[cfg(feature = "canonical-extension-types")]
+pub use canonical::*;
+
+use crate::{ArrowError, DataType};
+
+/// The metadata key for the string name identifying an [`ExtensionType`].
+pub const EXTENSION_TYPE_NAME_KEY: &str = "ARROW:extension:name";
+
+/// The metadata key for a serialized representation of the [`ExtensionType`]
+/// necessary to reconstruct the custom type.
+pub const EXTENSION_TYPE_METADATA_KEY: &str = "ARROW:extension:metadata";
+
+/// Extension types.
+///
+/// User-defined “extension” types can be defined setting certain key value
+/// pairs in the [`Field`] metadata structure. These extension keys are:
+/// - [`EXTENSION_TYPE_NAME_KEY`]
+/// - [`EXTENSION_TYPE_METADATA_KEY`]
+///
+/// Canonical extension types support in this crate requires the
+/// `canonical-extension-types` feature.
+///
+/// Extension types may or may not use the [`EXTENSION_TYPE_METADATA_KEY`]
+/// field.
+///
+/// # Example
+///
+/// The example below demonstrates how to implement this trait for a `Uuid`
+/// type. Note this is not the canonical extension type for `Uuid`, which does
+/// not include information about the `Uuid` version.
+///
+/// ```
+/// # use arrow_schema::ArrowError;
+/// # fn main() -> Result<(), ArrowError> {
+/// use arrow_schema::{DataType, extension::ExtensionType, Field};
+/// use std::{fmt, str::FromStr};
+///
+/// /// The different Uuid versions.
+/// #[derive(Clone, Copy, Debug, PartialEq)]
+/// enum UuidVersion {
+///     V1,
+///     V2,
+///     V3,
+///     V4,
+///     V5,
+///     V6,
+///     V7,
+///     V8,
+/// }
+///
+/// // We'll use `Display` to serialize.
+/// impl fmt::Display for UuidVersion {
+///     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+///         write!(
+///             f,
+///             "{}",
+///             match self {
+///                 Self::V1 => "V1",
+///                 Self::V2 => "V2",
+///                 Self::V3 => "V3",
+///                 Self::V4 => "V4",
+///                 Self::V5 => "V5",
+///                 Self::V6 => "V6",
+///                 Self::V7 => "V7",
+///                 Self::V8 => "V8",
+///             }
+///         )
+///     }
+/// }
+///
+/// // And `FromStr` to deserialize.
+/// impl FromStr for UuidVersion {
+///     type Err = ArrowError;
+///
+///     fn from_str(s: &str) -> Result<Self, Self::Err> {
+///         match s {
+///             "V1" => Ok(Self::V1),
+///             "V2" => Ok(Self::V2),
+///             "V3" => Ok(Self::V3),
+///             "V4" => Ok(Self::V4),
+///             "V5" => Ok(Self::V5),
+///             "V6" => Ok(Self::V6),
+///             "V7" => Ok(Self::V7),
+///             "V8" => Ok(Self::V8),
+///             _ => Err(ArrowError::ParseError("Invalid UuidVersion".to_owned())),
+///         }
+///     }
+/// }
+///
+/// /// This is the extension type, not the container for Uuid values. It
+/// /// stores the Uuid version (this is the metadata of this extension type).
+/// #[derive(Clone, Copy, Debug, PartialEq)]
+/// struct Uuid(UuidVersion);
+///
+/// impl ExtensionType for Uuid {
+///     // We use a namespace as suggested by the specification.
+///     const NAME: &'static str = "myorg.example.uuid";
+///
+///     // The metadata type is the Uuid version.
+///     type Metadata = UuidVersion;
+///
+///     // We just return a reference to the Uuid version.
+///     fn metadata(&self) -> &Self::Metadata {
+///         &self.0
+///     }
+///
+///     // We use the `Display` implementation to serialize the Uuid
+///     // version.
+///     fn serialize_metadata(&self) -> Option<String> {
+///         Some(self.0.to_string())
+///     }
+///
+///     // We use the `FromStr` implementation to deserialize the Uuid
+///     // version.
+///     fn deserialize_metadata(metadata: Option<&str>) -> Result<Self::Metadata, ArrowError> {
+///         metadata.map_or_else(
+///             || {
+///                 Err(ArrowError::InvalidArgumentError(
+///                     "Uuid extension type metadata missing".to_owned(),
+///                 ))
+///             },
+///             str::parse,
+///         )
+///     }
+///
+///     // The only supported data type is `FixedSizeBinary(16)`.
+///     fn supports_data_type(&self, data_type: &DataType) -> Result<(), ArrowError> {
+///         match data_type {
+///             DataType::FixedSizeBinary(16) => Ok(()),
+///             data_type => Err(ArrowError::InvalidArgumentError(format!(
+///                 "Uuid data type mismatch, expected FixedSizeBinary(16), found {data_type}"
+///             ))),
+///         }
+///     }
+///
+///     // We should always check if the data type is supported before
+///     // constructing the extension type.
+///     fn try_new(data_type: &DataType, metadata: Self::Metadata) -> Result<Self, ArrowError> {
+///         let uuid = Self(metadata);
+///         uuid.supports_data_type(data_type)?;
+///         Ok(uuid)
+///     }
+/// }
+///
+/// // We can now construct the extension type.
+/// let uuid_v1 = Uuid(UuidVersion::V1);
+///
+/// // And add it to a field.
+/// let mut field =
+///     Field::new("", DataType::FixedSizeBinary(16), false).with_extension_type(uuid_v1);
+///
+/// // And extract it from this field.
+/// assert_eq!(field.try_extension_type::<Uuid>()?, uuid_v1);
+///
+/// // When we try to add this to a field with an unsupported data type we
+/// // get an error.
+/// let result = Field::new("", DataType::Null, false).try_with_extension_type(uuid_v1);
+/// assert!(result.is_err());
+/// # Ok(()) }
+/// ```
+///
+/// <https://arrow.apache.org/docs/format/Columnar.html#extension-types>
+///
+/// [`Field`]: crate::Field
+pub trait ExtensionType: Sized {
+    /// The name identifying this extension type.
+    ///
+    /// This is the string value that is used for the
+    /// [`EXTENSION_TYPE_NAME_KEY`] in the [`Field::metadata`] of a [`Field`]
+    /// to identify this extension type.
+    ///
+    /// We recommend that you use a “namespace”-style prefix for extension
+    /// type names to minimize the possibility of conflicts with multiple Arrow
+    /// readers and writers in the same application. For example, use
+    /// `myorg.name_of_type` instead of simply `name_of_type`.
+    ///
+    /// Extension names beginning with `arrow.` are reserved for canonical
+    /// extension types, they should not be used for third-party extension
+    /// types.
+    ///
+    /// Extension names are case-sensitive.
+    ///
+    /// [`Field`]: crate::Field
+    /// [`Field::metadata`]: crate::Field::metadata
+    const NAME: &'static str;
+
+    /// The metadata type of this extension type.
+    ///
+    /// If an extension type defines no metadata it should use `()` to indicate
+    /// this.
+    type Metadata;
+
+    /// Returns a reference to the metadata of this extension type, or `&()` if
+    /// if this extension type defines no metadata (`Self::Metadata=()`).
+    fn metadata(&self) -> &Self::Metadata;
+
+    /// Returns the serialized representation of the metadata of this extension
+    /// type, or `None` if this extension type defines no metadata
+    /// (`Self::Metadata=()`).
+    ///
+    /// This is string value that is used for the
+    /// [`EXTENSION_TYPE_METADATA_KEY`] in the [`Field::metadata`] of a
+    /// [`Field`].
+    ///
+    /// [`Field`]: crate::Field
+    /// [`Field::metadata`]: crate::Field::metadata
+    fn serialize_metadata(&self) -> Option<String>;

Review Comment:
   Is it ever possible to have an error serializialzing the metadata? If so perhaps this should return `Result<Option<String>>`



-- 
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: github-unsubscribe@arrow.apache.org

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

Re: [PR] Add `ExtensionType` trait and `CanonicalExtensionType` enum [arrow-rs]

Posted by "mbrobbel (via GitHub)" <gi...@apache.org>.

mbrobbel commented on code in PR #5822:
URL: https://github.com/apache/arrow-rs/pull/5822#discussion_r1925449753


##########
arrow-schema/src/extension/mod.rs:
##########
@@ -0,0 +1,252 @@
+// 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.
+
+//! Extension types.
+
+#[cfg(feature = "canonical-extension-types")]
+mod canonical;
+#[cfg(feature = "canonical-extension-types")]
+pub use canonical::*;
+
+use crate::{ArrowError, DataType};
+
+/// The metadata key for the string name identifying an [`ExtensionType`].
+pub const EXTENSION_TYPE_NAME_KEY: &str = "ARROW:extension:name";
+
+/// The metadata key for a serialized representation of the [`ExtensionType`]
+/// necessary to reconstruct the custom type.
+pub const EXTENSION_TYPE_METADATA_KEY: &str = "ARROW:extension:metadata";
+
+/// Extension types.
+///
+/// User-defined “extension” types can be defined setting certain key value
+/// pairs in the [`Field`] metadata structure. These extension keys are:
+/// - [`EXTENSION_TYPE_NAME_KEY`]
+/// - [`EXTENSION_TYPE_METADATA_KEY`]
+///
+/// Canonical extension types support in this crate requires the
+/// `canonical-extension-types` feature.
+///
+/// Extension types may or may not use the [`EXTENSION_TYPE_METADATA_KEY`]
+/// field.
+///
+/// # Example
+///
+/// The example below demonstrates how to implement this trait for a `Uuid`
+/// type. Note this is not the canonical extension type for `Uuid`, which does
+/// not include information about the `Uuid` version.
+///
+/// ```
+/// # use arrow_schema::ArrowError;
+/// # fn main() -> Result<(), ArrowError> {
+/// use arrow_schema::{DataType, extension::ExtensionType, Field};
+/// use std::{fmt, str::FromStr};
+///
+/// /// The different Uuid versions.
+/// #[derive(Clone, Copy, Debug, PartialEq)]
+/// enum UuidVersion {
+///     V1,
+///     V2,
+///     V3,
+///     V4,
+///     V5,
+///     V6,
+///     V7,
+///     V8,
+/// }
+///
+/// // We'll use `Display` to serialize.
+/// impl fmt::Display for UuidVersion {
+///     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+///         write!(
+///             f,
+///             "{}",
+///             match self {
+///                 Self::V1 => "V1",
+///                 Self::V2 => "V2",
+///                 Self::V3 => "V3",
+///                 Self::V4 => "V4",
+///                 Self::V5 => "V5",
+///                 Self::V6 => "V6",
+///                 Self::V7 => "V7",
+///                 Self::V8 => "V8",
+///             }
+///         )
+///     }
+/// }
+///
+/// // And `FromStr` to deserialize.
+/// impl FromStr for UuidVersion {
+///     type Err = ArrowError;
+///
+///     fn from_str(s: &str) -> Result<Self, Self::Err> {
+///         match s {
+///             "V1" => Ok(Self::V1),
+///             "V2" => Ok(Self::V2),
+///             "V3" => Ok(Self::V3),
+///             "V4" => Ok(Self::V4),
+///             "V5" => Ok(Self::V5),
+///             "V6" => Ok(Self::V6),
+///             "V7" => Ok(Self::V7),
+///             "V8" => Ok(Self::V8),
+///             _ => Err(ArrowError::ParseError("Invalid UuidVersion".to_owned())),
+///         }
+///     }
+/// }
+///
+/// /// This is the extension type, not the container for Uuid values. It
+/// /// stores the Uuid version (this is the metadata of this extension type).
+/// #[derive(Clone, Copy, Debug, PartialEq)]
+/// struct Uuid(UuidVersion);
+///
+/// impl ExtensionType for Uuid {
+///     // We use a namespace as suggested by the specification.
+///     const NAME: &'static str = "myorg.example.uuid";
+///
+///     // The metadata type is the Uuid version.
+///     type Metadata = UuidVersion;
+///
+///     // We just return a reference to the Uuid version.
+///     fn metadata(&self) -> &Self::Metadata {
+///         &self.0
+///     }
+///
+///     // We use the `Display` implementation to serialize the Uuid
+///     // version.
+///     fn serialize_metadata(&self) -> Option<String> {
+///         Some(self.0.to_string())
+///     }
+///
+///     // We use the `FromStr` implementation to deserialize the Uuid
+///     // version.
+///     fn deserialize_metadata(metadata: Option<&str>) -> Result<Self::Metadata, ArrowError> {
+///         metadata.map_or_else(
+///             || {
+///                 Err(ArrowError::InvalidArgumentError(
+///                     "Uuid extension type metadata missing".to_owned(),
+///                 ))
+///             },
+///             str::parse,
+///         )
+///     }
+///
+///     // The only supported data type is `FixedSizeBinary(16)`.
+///     fn supports_data_type(&self, data_type: &DataType) -> Result<(), ArrowError> {
+///         match data_type {
+///             DataType::FixedSizeBinary(16) => Ok(()),
+///             data_type => Err(ArrowError::InvalidArgumentError(format!(
+///                 "Uuid data type mismatch, expected FixedSizeBinary(16), found {data_type}"
+///             ))),
+///         }
+///     }
+///
+///     // We should always check if the data type is supported before
+///     // constructing the extension type.
+///     fn try_new(data_type: &DataType, metadata: Self::Metadata) -> Result<Self, ArrowError> {
+///         let uuid = Self(metadata);
+///         uuid.supports_data_type(data_type)?;
+///         Ok(uuid)
+///     }
+/// }
+///
+/// // We can now construct the extension type.
+/// let uuid_v1 = Uuid(UuidVersion::V1);
+///
+/// // And add it to a field.
+/// let mut field =
+///     Field::new("", DataType::FixedSizeBinary(16), false).with_extension_type(uuid_v1);
+///
+/// // And extract it from this field.
+/// assert_eq!(field.try_extension_type::<Uuid>()?, uuid_v1);
+///
+/// // When we try to add this to a field with an unsupported data type we
+/// // get an error.
+/// let result = Field::new("", DataType::Null, false).try_with_extension_type(uuid_v1);
+/// assert!(result.is_err());
+/// # Ok(()) }
+/// ```
+///
+/// <https://arrow.apache.org/docs/format/Columnar.html#extension-types>
+///
+/// [`Field`]: crate::Field
+pub trait ExtensionType: Sized {
+    /// The name identifying this extension type.
+    ///
+    /// This is the string value that is used for the
+    /// [`EXTENSION_TYPE_NAME_KEY`] in the [`Field::metadata`] of a [`Field`]
+    /// to identify this extension type.
+    ///
+    /// We recommend that you use a “namespace”-style prefix for extension
+    /// type names to minimize the possibility of conflicts with multiple Arrow
+    /// readers and writers in the same application. For example, use
+    /// `myorg.name_of_type` instead of simply `name_of_type`.
+    ///
+    /// Extension names beginning with `arrow.` are reserved for canonical
+    /// extension types, they should not be used for third-party extension
+    /// types.
+    ///
+    /// Extension names are case-sensitive.
+    ///
+    /// [`Field`]: crate::Field
+    /// [`Field::metadata`]: crate::Field::metadata
+    const NAME: &'static str;
+
+    /// The metadata type of this extension type.
+    ///
+    /// If an extension type defines no metadata it should use `()` to indicate
+    /// this.
+    type Metadata;
+
+    /// Returns a reference to the metadata of this extension type, or `&()` if
+    /// if this extension type defines no metadata (`Self::Metadata=()`).
+    fn metadata(&self) -> &Self::Metadata;
+
+    /// Returns the serialized representation of the metadata of this extension
+    /// type, or `None` if this extension type defines no metadata
+    /// (`Self::Metadata=()`).
+    ///
+    /// This is string value that is used for the
+    /// [`EXTENSION_TYPE_METADATA_KEY`] in the [`Field::metadata`] of a
+    /// [`Field`].
+    ///
+    /// [`Field`]: crate::Field
+    /// [`Field::metadata`]: crate::Field::metadata
+    fn serialize_metadata(&self) -> Option<String>;

Review Comment:
   I can't think of a practical situation where serialization of the extension type metadata would error, but I'm fine with adding a `Result` here.
   
   The `Option` is required to support extension types without metadata (they should return `None` here). Note that this is different from `""`, because in that case the extension metadata key is added to the field custom metadata, whereas for `None` this key is skipped.
   
   So with support for serialization errors this would return: `Option<Result<String, ArrowError>>`.



-- 
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: github-unsubscribe@arrow.apache.org

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

Re: [PR] Add `ExtensionType` trait and `CanonicalExtensionType` enum [arrow-rs]

Posted by "paleolimbot (via GitHub)" <gi...@apache.org>.

paleolimbot commented on code in PR #5822:
URL: https://github.com/apache/arrow-rs/pull/5822#discussion_r1925561133


##########
arrow-schema/src/extension/mod.rs:
##########
@@ -0,0 +1,252 @@
+// 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.
+
+//! Extension types.
+
+#[cfg(feature = "canonical-extension-types")]
+mod canonical;
+#[cfg(feature = "canonical-extension-types")]
+pub use canonical::*;
+
+use crate::{ArrowError, DataType};
+
+/// The metadata key for the string name identifying an [`ExtensionType`].
+pub const EXTENSION_TYPE_NAME_KEY: &str = "ARROW:extension:name";
+
+/// The metadata key for a serialized representation of the [`ExtensionType`]
+/// necessary to reconstruct the custom type.
+pub const EXTENSION_TYPE_METADATA_KEY: &str = "ARROW:extension:metadata";
+
+/// Extension types.
+///
+/// User-defined “extension” types can be defined setting certain key value
+/// pairs in the [`Field`] metadata structure. These extension keys are:
+/// - [`EXTENSION_TYPE_NAME_KEY`]
+/// - [`EXTENSION_TYPE_METADATA_KEY`]
+///
+/// Canonical extension types support in this crate requires the
+/// `canonical-extension-types` feature.
+///
+/// Extension types may or may not use the [`EXTENSION_TYPE_METADATA_KEY`]
+/// field.
+///
+/// # Example
+///
+/// The example below demonstrates how to implement this trait for a `Uuid`
+/// type. Note this is not the canonical extension type for `Uuid`, which does
+/// not include information about the `Uuid` version.
+///
+/// ```
+/// # use arrow_schema::ArrowError;
+/// # fn main() -> Result<(), ArrowError> {
+/// use arrow_schema::{DataType, extension::ExtensionType, Field};
+/// use std::{fmt, str::FromStr};
+///
+/// /// The different Uuid versions.
+/// #[derive(Clone, Copy, Debug, PartialEq)]
+/// enum UuidVersion {
+///     V1,
+///     V2,
+///     V3,
+///     V4,
+///     V5,
+///     V6,
+///     V7,
+///     V8,
+/// }
+///
+/// // We'll use `Display` to serialize.
+/// impl fmt::Display for UuidVersion {
+///     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+///         write!(
+///             f,
+///             "{}",
+///             match self {
+///                 Self::V1 => "V1",
+///                 Self::V2 => "V2",
+///                 Self::V3 => "V3",
+///                 Self::V4 => "V4",
+///                 Self::V5 => "V5",
+///                 Self::V6 => "V6",
+///                 Self::V7 => "V7",
+///                 Self::V8 => "V8",
+///             }
+///         )
+///     }
+/// }
+///
+/// // And `FromStr` to deserialize.
+/// impl FromStr for UuidVersion {
+///     type Err = ArrowError;
+///
+///     fn from_str(s: &str) -> Result<Self, Self::Err> {
+///         match s {
+///             "V1" => Ok(Self::V1),
+///             "V2" => Ok(Self::V2),
+///             "V3" => Ok(Self::V3),
+///             "V4" => Ok(Self::V4),
+///             "V5" => Ok(Self::V5),
+///             "V6" => Ok(Self::V6),
+///             "V7" => Ok(Self::V7),
+///             "V8" => Ok(Self::V8),
+///             _ => Err(ArrowError::ParseError("Invalid UuidVersion".to_owned())),
+///         }
+///     }
+/// }
+///
+/// /// This is the extension type, not the container for Uuid values. It
+/// /// stores the Uuid version (this is the metadata of this extension type).
+/// #[derive(Clone, Copy, Debug, PartialEq)]
+/// struct Uuid(UuidVersion);
+///
+/// impl ExtensionType for Uuid {
+///     // We use a namespace as suggested by the specification.
+///     const NAME: &'static str = "myorg.example.uuid";
+///
+///     // The metadata type is the Uuid version.
+///     type Metadata = UuidVersion;
+///
+///     // We just return a reference to the Uuid version.
+///     fn metadata(&self) -> &Self::Metadata {
+///         &self.0
+///     }
+///
+///     // We use the `Display` implementation to serialize the Uuid
+///     // version.
+///     fn serialize_metadata(&self) -> Option<String> {
+///         Some(self.0.to_string())
+///     }
+///
+///     // We use the `FromStr` implementation to deserialize the Uuid
+///     // version.
+///     fn deserialize_metadata(metadata: Option<&str>) -> Result<Self::Metadata, ArrowError> {
+///         metadata.map_or_else(
+///             || {
+///                 Err(ArrowError::InvalidArgumentError(
+///                     "Uuid extension type metadata missing".to_owned(),
+///                 ))
+///             },
+///             str::parse,
+///         )
+///     }
+///
+///     // The only supported data type is `FixedSizeBinary(16)`.
+///     fn supports_data_type(&self, data_type: &DataType) -> Result<(), ArrowError> {
+///         match data_type {
+///             DataType::FixedSizeBinary(16) => Ok(()),
+///             data_type => Err(ArrowError::InvalidArgumentError(format!(
+///                 "Uuid data type mismatch, expected FixedSizeBinary(16), found {data_type}"
+///             ))),
+///         }
+///     }
+///
+///     // We should always check if the data type is supported before
+///     // constructing the extension type.
+///     fn try_new(data_type: &DataType, metadata: Self::Metadata) -> Result<Self, ArrowError> {
+///         let uuid = Self(metadata);
+///         uuid.supports_data_type(data_type)?;
+///         Ok(uuid)
+///     }
+/// }
+///
+/// // We can now construct the extension type.
+/// let uuid_v1 = Uuid(UuidVersion::V1);
+///
+/// // And add it to a field.
+/// let mut field =
+///     Field::new("", DataType::FixedSizeBinary(16), false).with_extension_type(uuid_v1);
+///
+/// // And extract it from this field.
+/// assert_eq!(field.try_extension_type::<Uuid>()?, uuid_v1);
+///
+/// // When we try to add this to a field with an unsupported data type we
+/// // get an error.
+/// let result = Field::new("", DataType::Null, false).try_with_extension_type(uuid_v1);
+/// assert!(result.is_err());
+/// # Ok(()) }
+/// ```
+///
+/// <https://arrow.apache.org/docs/format/Columnar.html#extension-types>
+///
+/// [`Field`]: crate::Field
+pub trait ExtensionType: Sized {
+    /// The name identifying this extension type.
+    ///
+    /// This is the string value that is used for the
+    /// [`EXTENSION_TYPE_NAME_KEY`] in the [`Field::metadata`] of a [`Field`]
+    /// to identify this extension type.
+    ///
+    /// We recommend that you use a “namespace”-style prefix for extension
+    /// type names to minimize the possibility of conflicts with multiple Arrow
+    /// readers and writers in the same application. For example, use
+    /// `myorg.name_of_type` instead of simply `name_of_type`.
+    ///
+    /// Extension names beginning with `arrow.` are reserved for canonical
+    /// extension types, they should not be used for third-party extension
+    /// types.
+    ///
+    /// Extension names are case-sensitive.
+    ///
+    /// [`Field`]: crate::Field
+    /// [`Field::metadata`]: crate::Field::metadata
+    const NAME: &'static str;
+
+    /// The metadata type of this extension type.
+    ///
+    /// If an extension type defines no metadata it should use `()` to indicate
+    /// this.
+    type Metadata;
+
+    /// Returns a reference to the metadata of this extension type, or `&()` if
+    /// if this extension type defines no metadata (`Self::Metadata=()`).
+    fn metadata(&self) -> &Self::Metadata;
+
+    /// Returns the serialized representation of the metadata of this extension
+    /// type, or `None` if this extension type defines no metadata
+    /// (`Self::Metadata=()`).
+    ///
+    /// This is string value that is used for the
+    /// [`EXTENSION_TYPE_METADATA_KEY`] in the [`Field::metadata`] of a
+    /// [`Field`].
+    ///
+    /// [`Field`]: crate::Field
+    /// [`Field::metadata`]: crate::Field::metadata
+    fn serialize_metadata(&self) -> Option<String>;

Review Comment:
   > It's your call
   
   Probably a better way to put that would have been: I don't have a strong feeling about it, but it did take me quite a long time to figure out why geoarrow-c was crashing causing pyarrow to crash 🙂 



-- 
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: github-unsubscribe@arrow.apache.org

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

Re: [PR] Add `ExtensionType` trait and `CanonicalExtensionType` enum [arrow-rs]

Posted by "alamb (via GitHub)" <gi...@apache.org>.

alamb commented on PR #5822:
URL: https://github.com/apache/arrow-rs/pull/5822#issuecomment-2654070019

   For some reason this PR isn't showing up in the release notes  🤔 
   - https://github.com/apache/arrow-rs/pull/7110
   
   It is in the release though


-- 
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: github-unsubscribe@arrow.apache.org

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

Re: [PR] Add `ExtensionType` trait and `CanonicalExtensionType` enum [arrow-rs]

Posted by "mbrobbel (via GitHub)" <gi...@apache.org>.

mbrobbel commented on code in PR #5822:
URL: https://github.com/apache/arrow-rs/pull/5822#discussion_r2080368797


##########
arrow-schema/src/field.rs:
##########
@@ -350,6 +355,167 @@ impl Field {
         self
     }
 
+    /// Returns the extension type name of this [`Field`], if set.
+    ///
+    /// This returns the value of [`EXTENSION_TYPE_NAME_KEY`], if set in
+    /// [`Field::metadata`]. If the key is missing, there is no extension type
+    /// name and this returns `None`.
+    ///
+    /// # Example
+    ///
+    /// ```
+    /// # use arrow_schema::{DataType, extension::EXTENSION_TYPE_NAME_KEY, Field};
+    ///
+    /// let field = Field::new("", DataType::Null, false);
+    /// assert_eq!(field.extension_type_name(), None);
+    ///
+    /// let field = Field::new("", DataType::Null, false).with_metadata(
+    ///    [(EXTENSION_TYPE_NAME_KEY.to_owned(), "example".to_owned())]
+    ///        .into_iter()
+    ///        .collect(),
+    /// );
+    /// assert_eq!(field.extension_type_name(), Some("example"));
+    /// ```
+    pub fn extension_type_name(&self) -> Option<&str> {
+        self.metadata()
+            .get(EXTENSION_TYPE_NAME_KEY)
+            .map(String::as_ref)
+    }
+
+    /// Returns the extension type metadata of this [`Field`], if set.
+    ///
+    /// This returns the value of [`EXTENSION_TYPE_METADATA_KEY`], if set in
+    /// [`Field::metadata`]. If the key is missing, there is no extension type
+    /// metadata and this returns `None`.
+    ///
+    /// # Example
+    ///
+    /// ```
+    /// # use arrow_schema::{DataType, extension::EXTENSION_TYPE_METADATA_KEY, Field};
+    ///
+    /// let field = Field::new("", DataType::Null, false);
+    /// assert_eq!(field.extension_type_metadata(), None);
+    ///
+    /// let field = Field::new("", DataType::Null, false).with_metadata(
+    ///    [(EXTENSION_TYPE_METADATA_KEY.to_owned(), "example".to_owned())]
+    ///        .into_iter()
+    ///        .collect(),
+    /// );
+    /// assert_eq!(field.extension_type_metadata(), Some("example"));
+    /// ```
+    pub fn extension_type_metadata(&self) -> Option<&str> {
+        self.metadata()
+            .get(EXTENSION_TYPE_METADATA_KEY)
+            .map(String::as_ref)
+    }
+
+    /// Returns an instance of the given [`ExtensionType`] of this [`Field`],
+    /// if set in the [`Field::metadata`].
+    ///
+    /// # Error
+    ///
+    /// Returns an error if
+    /// - this field does not have the name of this extension type
+    ///   ([`ExtensionType::NAME`]) in the [`Field::metadata`] (mismatch or
+    ///   missing)
+    /// - the deserialization of the metadata
+    ///   ([`ExtensionType::deserialize_metadata`]) fails
+    /// - the construction of the extension type ([`ExtensionType::try_new`])
+    ///   fail (for example when the [`Field::data_type`] is not supported by
+    ///   the extension type ([`ExtensionType::supports_data_type`]))
+    pub fn try_extension_type<E: ExtensionType>(&self) -> Result<E, ArrowError> {
+        // Check the extension name in the metadata
+        match self.extension_type_name() {
+            // It should match the name of the given extension type
+            Some(name) if name == E::NAME => {
+                // Deserialize the metadata and try to construct the extension
+                // type
+                E::deserialize_metadata(self.extension_type_metadata())
+                    .and_then(|metadata| E::try_new(self.data_type(), metadata))
+            }
+            // Name mismatch
+            Some(name) => Err(ArrowError::InvalidArgumentError(format!(
+                "Field extension type name mismatch, expected {}, found {name}",
+                E::NAME
+            ))),
+            // Name missing
+            None => Err(ArrowError::InvalidArgumentError(
+                "Field extension type name missing".to_owned(),
+            )),
+        }
+    }
+
+    /// Returns an instance of the given [`ExtensionType`] of this [`Field`],
+    /// panics if this [`Field`] does not have this extension type.
+    ///
+    /// # Panic
+    ///
+    /// This calls [`Field::try_extension_type`] and panics when it returns an
+    /// error.
+    pub fn extension_type<E: ExtensionType>(&self) -> E {
+        self.try_extension_type::<E>()
+            .unwrap_or_else(|e| panic!("{e}"))
+    }
+
+    /// Updates the metadata of this [`Field`] with the [`ExtensionType::NAME`]
+    /// and [`ExtensionType::metadata`] of the given [`ExtensionType`], if the
+    /// given extension type supports the [`Field::data_type`] of this field
+    /// ([`ExtensionType::supports_data_type`]).
+    ///
+    /// If the given extension type defines no metadata, a previously set
+    /// value of [`EXTENSION_TYPE_METADATA_KEY`] is cleared.
+    ///
+    /// # Error
+    ///
+    /// This functions returns an error if the data type of this field does not
+    /// match any of the supported storage types of the given extension type.
+    pub fn try_with_extension_type<E: ExtensionType>(
+        &mut self,
+        extension_type: E,
+    ) -> Result<(), ArrowError> {
+        // Make sure the data type of this field is supported
+        extension_type.supports_data_type(&self.data_type)?;
+
+        self.metadata
+            .insert(EXTENSION_TYPE_NAME_KEY.to_owned(), E::NAME.to_owned());
+        match extension_type.serialize_metadata() {
+            Some(metadata) => self
+                .metadata
+                .insert(EXTENSION_TYPE_METADATA_KEY.to_owned(), metadata),
+            // If this extension type has no metadata, we make sure to
+            // clear previously set metadata.
+            None => self.metadata.remove(EXTENSION_TYPE_METADATA_KEY),
+        };
+
+        Ok(())
+    }
+
+    /// Updates the metadata of this [`Field`] with the [`ExtensionType::NAME`]
+    /// and [`ExtensionType::metadata`] of the given [`ExtensionType`].
+    ///
+    /// # Panics
+    ///
+    /// This calls [`Field::try_with_extension_type`] and panics when it
+    /// returns an error.
+    pub fn with_extension_type<E: ExtensionType>(mut self, extension_type: E) -> Self {

Review Comment:
   The reasoning was: you may still want to use your field when `try_with_extension_type` returns an error.
   An alternative would be to return `Result<Self, (Self, Error)>`?



-- 
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: github-unsubscribe@arrow.apache.org

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

Re: [PR] Add `ExtensionType` for `uuid` and map to parquet logical type [arrow-rs]

Posted by "kylebarron (via GitHub)" <gi...@apache.org>.

kylebarron commented on PR #5822:
URL: https://github.com/apache/arrow-rs/pull/5822#issuecomment-2143347334

   Maybe ExtensionType could be a trait to be externally implementable and not limited to canonical extension types?


-- 
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: github-unsubscribe@arrow.apache.org

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

Re: [PR] Add `ExtensionType` trait and `CanonicalExtensionType` enum [arrow-rs]

Posted by "alamb (via GitHub)" <gi...@apache.org>.

alamb commented on PR #5822:
URL: https://github.com/apache/arrow-rs/pull/5822#issuecomment-2629356655

   Now that we hare release 54.1.0 let's do it
   - https://github.com/apache/arrow-rs/issues/6929
   
   🚀 Thanks again @mbrobbel for seeing this through. 


-- 
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: github-unsubscribe@arrow.apache.org

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

Re: [PR] Add `ExtensionType` trait and `CanonicalExtensionType` enum [arrow-rs]

Posted by "jayzhan211 (via GitHub)" <gi...@apache.org>.

jayzhan211 commented on code in PR #5822:
URL: https://github.com/apache/arrow-rs/pull/5822#discussion_r1925369634


##########
arrow-schema/src/extension/mod.rs:
##########
@@ -0,0 +1,252 @@
+// 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.
+
+//! Extension types.
+
+#[cfg(feature = "canonical-extension-types")]
+mod canonical;
+#[cfg(feature = "canonical-extension-types")]
+pub use canonical::*;
+
+use crate::{ArrowError, DataType};
+
+/// The metadata key for the string name identifying an [`ExtensionType`].
+pub const EXTENSION_TYPE_NAME_KEY: &str = "ARROW:extension:name";
+
+/// The metadata key for a serialized representation of the [`ExtensionType`]
+/// necessary to reconstruct the custom type.
+pub const EXTENSION_TYPE_METADATA_KEY: &str = "ARROW:extension:metadata";
+
+/// Extension types.
+///
+/// User-defined “extension” types can be defined setting certain key value
+/// pairs in the [`Field`] metadata structure. These extension keys are:
+/// - [`EXTENSION_TYPE_NAME_KEY`]
+/// - [`EXTENSION_TYPE_METADATA_KEY`]
+///
+/// Canonical extension types support in this crate requires the
+/// `canonical-extension-types` feature.
+///
+/// Extension types may or may not use the [`EXTENSION_TYPE_METADATA_KEY`]
+/// field.
+///
+/// # Example
+///
+/// The example below demonstrates how to implement this trait for a `Uuid`
+/// type. Note this is not the canonical extension type for `Uuid`, which does
+/// not include information about the `Uuid` version.
+///
+/// ```
+/// # use arrow_schema::ArrowError;
+/// # fn main() -> Result<(), ArrowError> {
+/// use arrow_schema::{DataType, extension::ExtensionType, Field};
+/// use std::{fmt, str::FromStr};
+///
+/// /// The different Uuid versions.
+/// #[derive(Clone, Copy, Debug, PartialEq)]
+/// enum UuidVersion {
+///     V1,
+///     V2,
+///     V3,
+///     V4,
+///     V5,
+///     V6,
+///     V7,
+///     V8,
+/// }
+///
+/// // We'll use `Display` to serialize.
+/// impl fmt::Display for UuidVersion {
+///     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+///         write!(
+///             f,
+///             "{}",
+///             match self {
+///                 Self::V1 => "V1",
+///                 Self::V2 => "V2",
+///                 Self::V3 => "V3",
+///                 Self::V4 => "V4",
+///                 Self::V5 => "V5",
+///                 Self::V6 => "V6",
+///                 Self::V7 => "V7",
+///                 Self::V8 => "V8",
+///             }
+///         )
+///     }
+/// }
+///
+/// // And `FromStr` to deserialize.
+/// impl FromStr for UuidVersion {
+///     type Err = ArrowError;
+///
+///     fn from_str(s: &str) -> Result<Self, Self::Err> {
+///         match s {
+///             "V1" => Ok(Self::V1),
+///             "V2" => Ok(Self::V2),
+///             "V3" => Ok(Self::V3),
+///             "V4" => Ok(Self::V4),
+///             "V5" => Ok(Self::V5),
+///             "V6" => Ok(Self::V6),
+///             "V7" => Ok(Self::V7),
+///             "V8" => Ok(Self::V8),
+///             _ => Err(ArrowError::ParseError("Invalid UuidVersion".to_owned())),
+///         }
+///     }
+/// }
+///
+/// /// This is the extension type, not the container for Uuid values. It
+/// /// stores the Uuid version (this is the metadata of this extension type).
+/// #[derive(Clone, Copy, Debug, PartialEq)]
+/// struct Uuid(UuidVersion);
+///
+/// impl ExtensionType for Uuid {
+///     // We use a namespace as suggested by the specification.
+///     const NAME: &'static str = "myorg.example.uuid";
+///
+///     // The metadata type is the Uuid version.
+///     type Metadata = UuidVersion;
+///
+///     // We just return a reference to the Uuid version.
+///     fn metadata(&self) -> &Self::Metadata {
+///         &self.0
+///     }
+///
+///     // We use the `Display` implementation to serialize the Uuid
+///     // version.
+///     fn serialize_metadata(&self) -> Option<String> {
+///         Some(self.0.to_string())
+///     }
+///
+///     // We use the `FromStr` implementation to deserialize the Uuid
+///     // version.
+///     fn deserialize_metadata(metadata: Option<&str>) -> Result<Self::Metadata, ArrowError> {
+///         metadata.map_or_else(
+///             || {
+///                 Err(ArrowError::InvalidArgumentError(
+///                     "Uuid extension type metadata missing".to_owned(),
+///                 ))
+///             },
+///             str::parse,
+///         )
+///     }
+///
+///     // The only supported data type is `FixedSizeBinary(16)`.
+///     fn supports_data_type(&self, data_type: &DataType) -> Result<(), ArrowError> {
+///         match data_type {
+///             DataType::FixedSizeBinary(16) => Ok(()),
+///             data_type => Err(ArrowError::InvalidArgumentError(format!(
+///                 "Uuid data type mismatch, expected FixedSizeBinary(16), found {data_type}"
+///             ))),
+///         }
+///     }
+///
+///     // We should always check if the data type is supported before
+///     // constructing the extension type.
+///     fn try_new(data_type: &DataType, metadata: Self::Metadata) -> Result<Self, ArrowError> {
+///         let uuid = Self(metadata);
+///         uuid.supports_data_type(data_type)?;
+///         Ok(uuid)
+///     }
+/// }
+///
+/// // We can now construct the extension type.
+/// let uuid_v1 = Uuid(UuidVersion::V1);
+///
+/// // And add it to a field.
+/// let mut field =
+///     Field::new("", DataType::FixedSizeBinary(16), false).with_extension_type(uuid_v1);
+///
+/// // And extract it from this field.
+/// assert_eq!(field.try_extension_type::<Uuid>()?, uuid_v1);
+///
+/// // When we try to add this to a field with an unsupported data type we
+/// // get an error.
+/// let result = Field::new("", DataType::Null, false).try_with_extension_type(uuid_v1);
+/// assert!(result.is_err());
+/// # Ok(()) }
+/// ```
+///
+/// <https://arrow.apache.org/docs/format/Columnar.html#extension-types>
+///
+/// [`Field`]: crate::Field
+pub trait ExtensionType: Sized {
+    /// The name identifying this extension type.
+    ///
+    /// This is the string value that is used for the
+    /// [`EXTENSION_TYPE_NAME_KEY`] in the [`Field::metadata`] of a [`Field`]
+    /// to identify this extension type.
+    ///
+    /// We recommend that you use a “namespace”-style prefix for extension
+    /// type names to minimize the possibility of conflicts with multiple Arrow
+    /// readers and writers in the same application. For example, use
+    /// `myorg.name_of_type` instead of simply `name_of_type`.
+    ///
+    /// Extension names beginning with `arrow.` are reserved for canonical
+    /// extension types, they should not be used for third-party extension
+    /// types.
+    ///
+    /// Extension names are case-sensitive.
+    ///
+    /// [`Field`]: crate::Field
+    /// [`Field::metadata`]: crate::Field::metadata
+    const NAME: &'static str;
+
+    /// The metadata type of this extension type.
+    ///
+    /// If an extension type defines no metadata it should use `()` to indicate
+    /// this.
+    type Metadata;

Review Comment:
   I see. The only benefit is that we can have a separate clean struct IF the metadata becomes pretty complex. For usual case, simply se/de without `type Metadata` is enough.



-- 
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: github-unsubscribe@arrow.apache.org

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

Re: [PR] Add `ExtensionType` trait and `CanonicalExtensionType` enum [arrow-rs]

Posted by "mbrobbel (via GitHub)" <gi...@apache.org>.

mbrobbel commented on code in PR #5822:
URL: https://github.com/apache/arrow-rs/pull/5822#discussion_r1925973798


##########
arrow-schema/src/extension/mod.rs:
##########
@@ -0,0 +1,252 @@
+// 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.
+
+//! Extension types.
+
+#[cfg(feature = "canonical_extension_types")]
+mod canonical;
+#[cfg(feature = "canonical_extension_types")]
+pub use canonical::*;
+
+use crate::{ArrowError, DataType};
+
+/// The metadata key for the string name identifying an [`ExtensionType`].
+pub const EXTENSION_TYPE_NAME_KEY: &str = "ARROW:extension:name";
+
+/// The metadata key for a serialized representation of the [`ExtensionType`]
+/// necessary to reconstruct the custom type.
+pub const EXTENSION_TYPE_METADATA_KEY: &str = "ARROW:extension:metadata";
+
+/// Extension types.
+///
+/// User-defined “extension” types can be defined setting certain key value
+/// pairs in the [`Field`] metadata structure. These extension keys are:
+/// - [`EXTENSION_TYPE_NAME_KEY`]
+/// - [`EXTENSION_TYPE_METADATA_KEY`]
+///
+/// Canonical extension types support in this crate requires the
+/// `canonical_extension_types` feature.
+///
+/// Extension types may or may not use the [`EXTENSION_TYPE_METADATA_KEY`]
+/// field.
+///
+/// # Example
+///
+/// The example below demonstrates how to implement this trait for a `Uuid`
+/// type. Note this is not the canonical extension type for `Uuid`, which does
+/// not include information about the `Uuid` version.
+///
+/// ```
+/// # use arrow_schema::ArrowError;
+/// # fn main() -> Result<(), ArrowError> {
+/// use arrow_schema::{DataType, extension::ExtensionType, Field};
+/// use std::{fmt, str::FromStr};
+///
+/// /// The different Uuid versions.
+/// #[derive(Clone, Copy, Debug, PartialEq)]
+/// enum UuidVersion {
+///     V1,
+///     V2,
+///     V3,
+///     V4,
+///     V5,
+///     V6,
+///     V7,
+///     V8,
+/// }
+///
+/// // We'll use `Display` to serialize.
+/// impl fmt::Display for UuidVersion {
+///     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+///         write!(
+///             f,
+///             "{}",
+///             match self {
+///                 Self::V1 => "V1",
+///                 Self::V2 => "V2",
+///                 Self::V3 => "V3",
+///                 Self::V4 => "V4",
+///                 Self::V5 => "V5",
+///                 Self::V6 => "V6",
+///                 Self::V7 => "V7",
+///                 Self::V8 => "V8",
+///             }
+///         )
+///     }
+/// }
+///
+/// // And `FromStr` to deserialize.
+/// impl FromStr for UuidVersion {
+///     type Err = ArrowError;
+///
+///     fn from_str(s: &str) -> Result<Self, Self::Err> {
+///         match s {
+///             "V1" => Ok(Self::V1),
+///             "V2" => Ok(Self::V2),
+///             "V3" => Ok(Self::V3),
+///             "V4" => Ok(Self::V4),
+///             "V5" => Ok(Self::V5),
+///             "V6" => Ok(Self::V6),
+///             "V7" => Ok(Self::V7),
+///             "V8" => Ok(Self::V8),
+///             _ => Err(ArrowError::ParseError("Invalid UuidVersion".to_owned())),
+///         }
+///     }
+/// }
+///
+/// /// This is the extension type, not the container for Uuid values. It
+/// /// stores the Uuid version (this is the metadata of this extension type).
+/// #[derive(Clone, Copy, Debug, PartialEq)]
+/// struct Uuid(UuidVersion);
+///
+/// impl ExtensionType for Uuid {
+///     // We use a namespace as suggested by the specification.
+///     const NAME: &'static str = "myorg.example.uuid";
+///
+///     // The metadata type is the Uuid version.
+///     type Metadata = UuidVersion;
+///
+///     // We just return a reference to the Uuid version.
+///     fn metadata(&self) -> &Self::Metadata {
+///         &self.0

Review Comment:
   Yes, the current signature requires that. We could change it to return `Self::Metadata`?
   
   The pattern I used for the canonical extension types with metadata applied to your example:
   
   ```rust
   pub struct PointType(PointTypeMetadata);
   
   pub struct PointTypeMetadata(CoordType, Dimension);
   
   impl ExtensionType for PointType {
       const NAME: &'static str = "geoarrow.point";
   
       type Metadata = PointTypeMetadata;
   
       fn metadata(&self) -> &Self::Metadata {
           &self.0
       }
   }
   ```
   
   



-- 
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: github-unsubscribe@arrow.apache.org

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

Re: [PR] Add `ExtensionType` trait and `CanonicalExtensionType` enum [arrow-rs]

Posted by "paleolimbot (via GitHub)" <gi...@apache.org>.

paleolimbot commented on code in PR #5822:
URL: https://github.com/apache/arrow-rs/pull/5822#discussion_r1925535478


##########
arrow-schema/src/extension/mod.rs:
##########
@@ -0,0 +1,252 @@
+// 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.
+
+//! Extension types.
+
+#[cfg(feature = "canonical-extension-types")]
+mod canonical;
+#[cfg(feature = "canonical-extension-types")]
+pub use canonical::*;
+
+use crate::{ArrowError, DataType};
+
+/// The metadata key for the string name identifying an [`ExtensionType`].
+pub const EXTENSION_TYPE_NAME_KEY: &str = "ARROW:extension:name";
+
+/// The metadata key for a serialized representation of the [`ExtensionType`]
+/// necessary to reconstruct the custom type.
+pub const EXTENSION_TYPE_METADATA_KEY: &str = "ARROW:extension:metadata";
+
+/// Extension types.
+///
+/// User-defined “extension” types can be defined setting certain key value
+/// pairs in the [`Field`] metadata structure. These extension keys are:
+/// - [`EXTENSION_TYPE_NAME_KEY`]
+/// - [`EXTENSION_TYPE_METADATA_KEY`]
+///
+/// Canonical extension types support in this crate requires the
+/// `canonical-extension-types` feature.
+///
+/// Extension types may or may not use the [`EXTENSION_TYPE_METADATA_KEY`]
+/// field.
+///
+/// # Example
+///
+/// The example below demonstrates how to implement this trait for a `Uuid`
+/// type. Note this is not the canonical extension type for `Uuid`, which does
+/// not include information about the `Uuid` version.
+///
+/// ```
+/// # use arrow_schema::ArrowError;
+/// # fn main() -> Result<(), ArrowError> {
+/// use arrow_schema::{DataType, extension::ExtensionType, Field};
+/// use std::{fmt, str::FromStr};
+///
+/// /// The different Uuid versions.
+/// #[derive(Clone, Copy, Debug, PartialEq)]
+/// enum UuidVersion {
+///     V1,
+///     V2,
+///     V3,
+///     V4,
+///     V5,
+///     V6,
+///     V7,
+///     V8,
+/// }
+///
+/// // We'll use `Display` to serialize.
+/// impl fmt::Display for UuidVersion {
+///     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+///         write!(
+///             f,
+///             "{}",
+///             match self {
+///                 Self::V1 => "V1",
+///                 Self::V2 => "V2",
+///                 Self::V3 => "V3",
+///                 Self::V4 => "V4",
+///                 Self::V5 => "V5",
+///                 Self::V6 => "V6",
+///                 Self::V7 => "V7",
+///                 Self::V8 => "V8",
+///             }
+///         )
+///     }
+/// }
+///
+/// // And `FromStr` to deserialize.
+/// impl FromStr for UuidVersion {
+///     type Err = ArrowError;
+///
+///     fn from_str(s: &str) -> Result<Self, Self::Err> {
+///         match s {
+///             "V1" => Ok(Self::V1),
+///             "V2" => Ok(Self::V2),
+///             "V3" => Ok(Self::V3),
+///             "V4" => Ok(Self::V4),
+///             "V5" => Ok(Self::V5),
+///             "V6" => Ok(Self::V6),
+///             "V7" => Ok(Self::V7),
+///             "V8" => Ok(Self::V8),
+///             _ => Err(ArrowError::ParseError("Invalid UuidVersion".to_owned())),
+///         }
+///     }
+/// }
+///
+/// /// This is the extension type, not the container for Uuid values. It
+/// /// stores the Uuid version (this is the metadata of this extension type).
+/// #[derive(Clone, Copy, Debug, PartialEq)]
+/// struct Uuid(UuidVersion);
+///
+/// impl ExtensionType for Uuid {
+///     // We use a namespace as suggested by the specification.
+///     const NAME: &'static str = "myorg.example.uuid";
+///
+///     // The metadata type is the Uuid version.
+///     type Metadata = UuidVersion;
+///
+///     // We just return a reference to the Uuid version.
+///     fn metadata(&self) -> &Self::Metadata {
+///         &self.0
+///     }
+///
+///     // We use the `Display` implementation to serialize the Uuid
+///     // version.
+///     fn serialize_metadata(&self) -> Option<String> {
+///         Some(self.0.to_string())
+///     }
+///
+///     // We use the `FromStr` implementation to deserialize the Uuid
+///     // version.
+///     fn deserialize_metadata(metadata: Option<&str>) -> Result<Self::Metadata, ArrowError> {
+///         metadata.map_or_else(
+///             || {
+///                 Err(ArrowError::InvalidArgumentError(
+///                     "Uuid extension type metadata missing".to_owned(),
+///                 ))
+///             },
+///             str::parse,
+///         )
+///     }
+///
+///     // The only supported data type is `FixedSizeBinary(16)`.
+///     fn supports_data_type(&self, data_type: &DataType) -> Result<(), ArrowError> {
+///         match data_type {
+///             DataType::FixedSizeBinary(16) => Ok(()),
+///             data_type => Err(ArrowError::InvalidArgumentError(format!(
+///                 "Uuid data type mismatch, expected FixedSizeBinary(16), found {data_type}"
+///             ))),
+///         }
+///     }
+///
+///     // We should always check if the data type is supported before
+///     // constructing the extension type.
+///     fn try_new(data_type: &DataType, metadata: Self::Metadata) -> Result<Self, ArrowError> {
+///         let uuid = Self(metadata);
+///         uuid.supports_data_type(data_type)?;
+///         Ok(uuid)
+///     }
+/// }
+///
+/// // We can now construct the extension type.
+/// let uuid_v1 = Uuid(UuidVersion::V1);
+///
+/// // And add it to a field.
+/// let mut field =
+///     Field::new("", DataType::FixedSizeBinary(16), false).with_extension_type(uuid_v1);
+///
+/// // And extract it from this field.
+/// assert_eq!(field.try_extension_type::<Uuid>()?, uuid_v1);
+///
+/// // When we try to add this to a field with an unsupported data type we
+/// // get an error.
+/// let result = Field::new("", DataType::Null, false).try_with_extension_type(uuid_v1);
+/// assert!(result.is_err());
+/// # Ok(()) }
+/// ```
+///
+/// <https://arrow.apache.org/docs/format/Columnar.html#extension-types>
+///
+/// [`Field`]: crate::Field
+pub trait ExtensionType: Sized {
+    /// The name identifying this extension type.
+    ///
+    /// This is the string value that is used for the
+    /// [`EXTENSION_TYPE_NAME_KEY`] in the [`Field::metadata`] of a [`Field`]
+    /// to identify this extension type.
+    ///
+    /// We recommend that you use a “namespace”-style prefix for extension
+    /// type names to minimize the possibility of conflicts with multiple Arrow
+    /// readers and writers in the same application. For example, use
+    /// `myorg.name_of_type` instead of simply `name_of_type`.
+    ///
+    /// Extension names beginning with `arrow.` are reserved for canonical
+    /// extension types, they should not be used for third-party extension
+    /// types.
+    ///
+    /// Extension names are case-sensitive.
+    ///
+    /// [`Field`]: crate::Field
+    /// [`Field::metadata`]: crate::Field::metadata
+    const NAME: &'static str;
+
+    /// The metadata type of this extension type.
+    ///
+    /// If an extension type defines no metadata it should use `()` to indicate
+    /// this.
+    type Metadata;
+
+    /// Returns a reference to the metadata of this extension type, or `&()` if
+    /// if this extension type defines no metadata (`Self::Metadata=()`).
+    fn metadata(&self) -> &Self::Metadata;
+
+    /// Returns the serialized representation of the metadata of this extension
+    /// type, or `None` if this extension type defines no metadata
+    /// (`Self::Metadata=()`).
+    ///
+    /// This is string value that is used for the
+    /// [`EXTENSION_TYPE_METADATA_KEY`] in the [`Field::metadata`] of a
+    /// [`Field`].
+    ///
+    /// [`Field`]: crate::Field
+    /// [`Field::metadata`]: crate::Field::metadata
+    fn serialize_metadata(&self) -> Option<String>;

Review Comment:
   Yes, the spec has always said that (and Arrow C++ has always interpreted this correctly when importing via IPC). It's your call, but some people are stuck on very old Arrow C++/pyarrow for compatibility reasons and won't have a viable workaround.



-- 
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: github-unsubscribe@arrow.apache.org

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

Re: [PR] Add `ExtensionType` trait and `CanonicalExtensionType` enum [arrow-rs]

Posted by "mbrobbel (via GitHub)" <gi...@apache.org>.

mbrobbel commented on code in PR #5822:
URL: https://github.com/apache/arrow-rs/pull/5822#discussion_r1925528709


##########
arrow-schema/src/extension/mod.rs:
##########
@@ -0,0 +1,252 @@
+// 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.
+
+//! Extension types.
+
+#[cfg(feature = "canonical-extension-types")]
+mod canonical;
+#[cfg(feature = "canonical-extension-types")]
+pub use canonical::*;
+
+use crate::{ArrowError, DataType};
+
+/// The metadata key for the string name identifying an [`ExtensionType`].
+pub const EXTENSION_TYPE_NAME_KEY: &str = "ARROW:extension:name";
+
+/// The metadata key for a serialized representation of the [`ExtensionType`]
+/// necessary to reconstruct the custom type.
+pub const EXTENSION_TYPE_METADATA_KEY: &str = "ARROW:extension:metadata";
+
+/// Extension types.
+///
+/// User-defined “extension” types can be defined setting certain key value
+/// pairs in the [`Field`] metadata structure. These extension keys are:
+/// - [`EXTENSION_TYPE_NAME_KEY`]
+/// - [`EXTENSION_TYPE_METADATA_KEY`]
+///
+/// Canonical extension types support in this crate requires the
+/// `canonical-extension-types` feature.
+///
+/// Extension types may or may not use the [`EXTENSION_TYPE_METADATA_KEY`]
+/// field.
+///
+/// # Example
+///
+/// The example below demonstrates how to implement this trait for a `Uuid`
+/// type. Note this is not the canonical extension type for `Uuid`, which does
+/// not include information about the `Uuid` version.
+///
+/// ```
+/// # use arrow_schema::ArrowError;
+/// # fn main() -> Result<(), ArrowError> {
+/// use arrow_schema::{DataType, extension::ExtensionType, Field};
+/// use std::{fmt, str::FromStr};
+///
+/// /// The different Uuid versions.
+/// #[derive(Clone, Copy, Debug, PartialEq)]
+/// enum UuidVersion {
+///     V1,
+///     V2,
+///     V3,
+///     V4,
+///     V5,
+///     V6,
+///     V7,
+///     V8,
+/// }
+///
+/// // We'll use `Display` to serialize.
+/// impl fmt::Display for UuidVersion {
+///     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+///         write!(
+///             f,
+///             "{}",
+///             match self {
+///                 Self::V1 => "V1",
+///                 Self::V2 => "V2",
+///                 Self::V3 => "V3",
+///                 Self::V4 => "V4",
+///                 Self::V5 => "V5",
+///                 Self::V6 => "V6",
+///                 Self::V7 => "V7",
+///                 Self::V8 => "V8",
+///             }
+///         )
+///     }
+/// }
+///
+/// // And `FromStr` to deserialize.
+/// impl FromStr for UuidVersion {
+///     type Err = ArrowError;
+///
+///     fn from_str(s: &str) -> Result<Self, Self::Err> {
+///         match s {
+///             "V1" => Ok(Self::V1),
+///             "V2" => Ok(Self::V2),
+///             "V3" => Ok(Self::V3),
+///             "V4" => Ok(Self::V4),
+///             "V5" => Ok(Self::V5),
+///             "V6" => Ok(Self::V6),
+///             "V7" => Ok(Self::V7),
+///             "V8" => Ok(Self::V8),
+///             _ => Err(ArrowError::ParseError("Invalid UuidVersion".to_owned())),
+///         }
+///     }
+/// }
+///
+/// /// This is the extension type, not the container for Uuid values. It
+/// /// stores the Uuid version (this is the metadata of this extension type).
+/// #[derive(Clone, Copy, Debug, PartialEq)]
+/// struct Uuid(UuidVersion);
+///
+/// impl ExtensionType for Uuid {
+///     // We use a namespace as suggested by the specification.
+///     const NAME: &'static str = "myorg.example.uuid";
+///
+///     // The metadata type is the Uuid version.
+///     type Metadata = UuidVersion;
+///
+///     // We just return a reference to the Uuid version.
+///     fn metadata(&self) -> &Self::Metadata {
+///         &self.0
+///     }
+///
+///     // We use the `Display` implementation to serialize the Uuid
+///     // version.
+///     fn serialize_metadata(&self) -> Option<String> {
+///         Some(self.0.to_string())
+///     }
+///
+///     // We use the `FromStr` implementation to deserialize the Uuid
+///     // version.
+///     fn deserialize_metadata(metadata: Option<&str>) -> Result<Self::Metadata, ArrowError> {
+///         metadata.map_or_else(
+///             || {
+///                 Err(ArrowError::InvalidArgumentError(
+///                     "Uuid extension type metadata missing".to_owned(),
+///                 ))
+///             },
+///             str::parse,
+///         )
+///     }
+///
+///     // The only supported data type is `FixedSizeBinary(16)`.
+///     fn supports_data_type(&self, data_type: &DataType) -> Result<(), ArrowError> {
+///         match data_type {
+///             DataType::FixedSizeBinary(16) => Ok(()),
+///             data_type => Err(ArrowError::InvalidArgumentError(format!(
+///                 "Uuid data type mismatch, expected FixedSizeBinary(16), found {data_type}"
+///             ))),
+///         }
+///     }
+///
+///     // We should always check if the data type is supported before
+///     // constructing the extension type.
+///     fn try_new(data_type: &DataType, metadata: Self::Metadata) -> Result<Self, ArrowError> {
+///         let uuid = Self(metadata);
+///         uuid.supports_data_type(data_type)?;
+///         Ok(uuid)
+///     }
+/// }
+///
+/// // We can now construct the extension type.
+/// let uuid_v1 = Uuid(UuidVersion::V1);
+///
+/// // And add it to a field.
+/// let mut field =
+///     Field::new("", DataType::FixedSizeBinary(16), false).with_extension_type(uuid_v1);
+///
+/// // And extract it from this field.
+/// assert_eq!(field.try_extension_type::<Uuid>()?, uuid_v1);
+///
+/// // When we try to add this to a field with an unsupported data type we
+/// // get an error.
+/// let result = Field::new("", DataType::Null, false).try_with_extension_type(uuid_v1);
+/// assert!(result.is_err());
+/// # Ok(()) }
+/// ```
+///
+/// <https://arrow.apache.org/docs/format/Columnar.html#extension-types>
+///
+/// [`Field`]: crate::Field
+pub trait ExtensionType: Sized {
+    /// The name identifying this extension type.
+    ///
+    /// This is the string value that is used for the
+    /// [`EXTENSION_TYPE_NAME_KEY`] in the [`Field::metadata`] of a [`Field`]
+    /// to identify this extension type.
+    ///
+    /// We recommend that you use a “namespace”-style prefix for extension
+    /// type names to minimize the possibility of conflicts with multiple Arrow
+    /// readers and writers in the same application. For example, use
+    /// `myorg.name_of_type` instead of simply `name_of_type`.
+    ///
+    /// Extension names beginning with `arrow.` are reserved for canonical
+    /// extension types, they should not be used for third-party extension
+    /// types.
+    ///
+    /// Extension names are case-sensitive.
+    ///
+    /// [`Field`]: crate::Field
+    /// [`Field::metadata`]: crate::Field::metadata
+    const NAME: &'static str;
+
+    /// The metadata type of this extension type.
+    ///
+    /// If an extension type defines no metadata it should use `()` to indicate
+    /// this.
+    type Metadata;
+
+    /// Returns a reference to the metadata of this extension type, or `&()` if
+    /// if this extension type defines no metadata (`Self::Metadata=()`).
+    fn metadata(&self) -> &Self::Metadata;
+
+    /// Returns the serialized representation of the metadata of this extension
+    /// type, or `None` if this extension type defines no metadata
+    /// (`Self::Metadata=()`).
+    ///
+    /// This is string value that is used for the
+    /// [`EXTENSION_TYPE_METADATA_KEY`] in the [`Field::metadata`] of a
+    /// [`Field`].
+    ///
+    /// [`Field`]: crate::Field
+    /// [`Field::metadata`]: crate::Field::metadata
+    fn serialize_metadata(&self) -> Option<String>;

Review Comment:
   Per https://arrow.apache.org/docs/format/Columnar.html#extension-types:
   
   > Extension types may or may not use the 'ARROW:extension:metadata' field.
   
   Which I interpret as; it should also work when this key is missing in the field metadata for extension types that don't require metadata.



-- 
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: github-unsubscribe@arrow.apache.org

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

Re: [PR] Add `ExtensionType` trait and `CanonicalExtensionType` enum [arrow-rs]

Posted by "Tpt (via GitHub)" <gi...@apache.org>.

Tpt commented on code in PR #5822:
URL: https://github.com/apache/arrow-rs/pull/5822#discussion_r1928524481


##########
arrow-schema/src/extension/canonical/bool8.rs:
##########
@@ -0,0 +1,143 @@
+// 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.
+
+//! 8-bit Boolean
+//!
+//! <https://arrow.apache.org/docs/format/CanonicalExtensions.html#bit-boolean>
+
+use crate::{extension::ExtensionType, ArrowError, DataType};
+
+/// The extension type for `8-bit Boolean`.
+///
+/// Extension name: `arrow.bool8`.
+///
+/// The storage type of the extension is `Int8` where:
+/// - false is denoted by the value 0.
+/// - true can be specified using any non-zero value. Preferably 1.
+///
+/// <https://arrow.apache.org/docs/format/CanonicalExtensions.html#bit-boolean>
+#[derive(Debug, Default, Clone, Copy, PartialEq)]
+pub struct Bool8;
+
+impl ExtensionType for Bool8 {
+    const NAME: &'static str = "arrow.bool8";
+
+    type Metadata = &'static str;

Review Comment:
   Ok! Missed that! Thanks!



-- 
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: github-unsubscribe@arrow.apache.org

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

Re: [PR] Add `ExtensionType` trait and `CanonicalExtensionType` enum [arrow-rs]

Posted by "mbrobbel (via GitHub)" <gi...@apache.org>.

mbrobbel commented on code in PR #5822:
URL: https://github.com/apache/arrow-rs/pull/5822#discussion_r1928571935


##########
arrow-schema/src/extension/canonical/json.rs:
##########
@@ -0,0 +1,189 @@
+// 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.
+
+//! JSON
+//!
+//! <https://arrow.apache.org/docs/format/CanonicalExtensions.html#json>
+
+use serde_json::Value;
+
+use crate::{extension::ExtensionType, ArrowError, DataType};
+
+/// The extension type for `JSON`.
+///
+/// Extension name: `arrow.json`.
+///
+/// The storage type of this extension is `String` or `LargeString` or
+/// `StringView`. Only UTF-8 encoded JSON as specified in [rfc8259](https://datatracker.ietf.org/doc/html/rfc8259)
+/// is supported.
+///
+/// This type does not have any parameters.
+///
+/// Metadata is either an empty string or a JSON string with an empty

Review Comment:
   There is currently no metadata storage for `Bool8`, and the difference is that there is no note about future additions to the metadata serialization:
   
   > Description of the serialization:
   > Metadata is an empty string.
   
   So, if there are breaking changes in the spec for this type, it would have to be a breaking change here too anyway.
   



-- 
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: github-unsubscribe@arrow.apache.org

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

Re: [PR] Add `ExtensionType` trait and `CanonicalExtensionType` enum [arrow-rs]

Posted by "Tpt (via GitHub)" <gi...@apache.org>.

Tpt commented on code in PR #5822:
URL: https://github.com/apache/arrow-rs/pull/5822#discussion_r1928523593


##########
arrow-schema/src/extension/canonical/json.rs:
##########
@@ -0,0 +1,189 @@
+// 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.
+
+//! JSON
+//!
+//! <https://arrow.apache.org/docs/format/CanonicalExtensions.html#json>
+
+use serde_json::Value;
+
+use crate::{extension::ExtensionType, ArrowError, DataType};
+
+/// The extension type for `JSON`.
+///
+/// Extension name: `arrow.json`.
+///
+/// The storage type of this extension is `String` or `LargeString` or
+/// `StringView`. Only UTF-8 encoded JSON as specified in [rfc8259](https://datatracker.ietf.org/doc/html/rfc8259)
+/// is supported.
+///
+/// This type does not have any parameters.
+///
+/// Metadata is either an empty string or a JSON string with an empty

Review Comment:
   Maybe bad idea: instead of an `Option<Map<...>>` what about just using a `is_empty_object: bool` field or something like this to make invalid states unrepresentable?
   
   Also, you might even hack something like this to only deserialize the empty object:
   
   ```serde
   #[derive(Deserialize)]
   struct Empty {}
   
   let is_empty_struct = serde_json::from_str<Empty>(value).is_ok()
   ```
   
   and then use the `Option<Empty>` type in `JsonMetadata`



-- 
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: github-unsubscribe@arrow.apache.org

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

Re: [PR] Add `ExtensionType` trait and `CanonicalExtensionType` enum [arrow-rs]

Posted by "alamb (via GitHub)" <gi...@apache.org>.

alamb commented on PR #5822:
URL: https://github.com/apache/arrow-rs/pull/5822#issuecomment-2629368143

   Epic work -- can't wait to see how this works in the real world


-- 
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: github-unsubscribe@arrow.apache.org

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

Re: [PR] Add `ExtensionType` trait and `CanonicalExtensionType` enum [arrow-rs]

Posted by "mbrobbel (via GitHub)" <gi...@apache.org>.

mbrobbel commented on code in PR #5822:
URL: https://github.com/apache/arrow-rs/pull/5822#discussion_r1925515381


##########
arrow-schema/src/extension/mod.rs:
##########
@@ -0,0 +1,252 @@
+// 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.
+
+//! Extension types.
+
+#[cfg(feature = "canonical-extension-types")]

Review Comment:
   [069642f](https://github.com/apache/arrow-rs/pull/5822/commits/069642fde6af993dfb94d6082bddaf0933740f8b)
   
   > Perhaps we could also note it is "experimental"
   
   Do you want to add an `experimental` feature to the `arrow` crate (same as in the `parquet` crate)?
   https://github.com/apache/arrow-rs/blob/16642149b75f0b18d8e689ad9e2111edf4e429a9/parquet/Cargo.toml#L112-L113
   
   Or just a note in the module docs?
   



-- 
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: github-unsubscribe@arrow.apache.org

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

Re: [PR] Add `ExtensionType` trait and `CanonicalExtensionType` enum [arrow-rs]

Posted by "jayzhan211 (via GitHub)" <gi...@apache.org>.

jayzhan211 commented on code in PR #5822:
URL: https://github.com/apache/arrow-rs/pull/5822#discussion_r1925329991


##########
arrow-schema/src/extension/mod.rs:
##########
@@ -0,0 +1,252 @@
+// 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.
+
+//! Extension types.
+
+#[cfg(feature = "canonical-extension-types")]
+mod canonical;
+#[cfg(feature = "canonical-extension-types")]
+pub use canonical::*;
+
+use crate::{ArrowError, DataType};
+
+/// The metadata key for the string name identifying an [`ExtensionType`].
+pub const EXTENSION_TYPE_NAME_KEY: &str = "ARROW:extension:name";
+
+/// The metadata key for a serialized representation of the [`ExtensionType`]
+/// necessary to reconstruct the custom type.
+pub const EXTENSION_TYPE_METADATA_KEY: &str = "ARROW:extension:metadata";
+
+/// Extension types.
+///
+/// User-defined “extension” types can be defined setting certain key value
+/// pairs in the [`Field`] metadata structure. These extension keys are:
+/// - [`EXTENSION_TYPE_NAME_KEY`]
+/// - [`EXTENSION_TYPE_METADATA_KEY`]
+///
+/// Canonical extension types support in this crate requires the
+/// `canonical-extension-types` feature.
+///
+/// Extension types may or may not use the [`EXTENSION_TYPE_METADATA_KEY`]
+/// field.
+///
+/// # Example
+///
+/// The example below demonstrates how to implement this trait for a `Uuid`
+/// type. Note this is not the canonical extension type for `Uuid`, which does
+/// not include information about the `Uuid` version.
+///
+/// ```
+/// # use arrow_schema::ArrowError;
+/// # fn main() -> Result<(), ArrowError> {
+/// use arrow_schema::{DataType, extension::ExtensionType, Field};
+/// use std::{fmt, str::FromStr};
+///
+/// /// The different Uuid versions.
+/// #[derive(Clone, Copy, Debug, PartialEq)]
+/// enum UuidVersion {
+///     V1,
+///     V2,
+///     V3,
+///     V4,
+///     V5,
+///     V6,
+///     V7,
+///     V8,
+/// }
+///
+/// // We'll use `Display` to serialize.
+/// impl fmt::Display for UuidVersion {
+///     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+///         write!(
+///             f,
+///             "{}",
+///             match self {
+///                 Self::V1 => "V1",
+///                 Self::V2 => "V2",
+///                 Self::V3 => "V3",
+///                 Self::V4 => "V4",
+///                 Self::V5 => "V5",
+///                 Self::V6 => "V6",
+///                 Self::V7 => "V7",
+///                 Self::V8 => "V8",
+///             }
+///         )
+///     }
+/// }
+///
+/// // And `FromStr` to deserialize.
+/// impl FromStr for UuidVersion {
+///     type Err = ArrowError;
+///
+///     fn from_str(s: &str) -> Result<Self, Self::Err> {
+///         match s {
+///             "V1" => Ok(Self::V1),
+///             "V2" => Ok(Self::V2),
+///             "V3" => Ok(Self::V3),
+///             "V4" => Ok(Self::V4),
+///             "V5" => Ok(Self::V5),
+///             "V6" => Ok(Self::V6),
+///             "V7" => Ok(Self::V7),
+///             "V8" => Ok(Self::V8),
+///             _ => Err(ArrowError::ParseError("Invalid UuidVersion".to_owned())),
+///         }
+///     }
+/// }
+///
+/// /// This is the extension type, not the container for Uuid values. It
+/// /// stores the Uuid version (this is the metadata of this extension type).
+/// #[derive(Clone, Copy, Debug, PartialEq)]
+/// struct Uuid(UuidVersion);
+///
+/// impl ExtensionType for Uuid {
+///     // We use a namespace as suggested by the specification.
+///     const NAME: &'static str = "myorg.example.uuid";
+///
+///     // The metadata type is the Uuid version.
+///     type Metadata = UuidVersion;
+///
+///     // We just return a reference to the Uuid version.
+///     fn metadata(&self) -> &Self::Metadata {
+///         &self.0
+///     }
+///
+///     // We use the `Display` implementation to serialize the Uuid
+///     // version.
+///     fn serialize_metadata(&self) -> Option<String> {
+///         Some(self.0.to_string())
+///     }
+///
+///     // We use the `FromStr` implementation to deserialize the Uuid
+///     // version.
+///     fn deserialize_metadata(metadata: Option<&str>) -> Result<Self::Metadata, ArrowError> {
+///         metadata.map_or_else(
+///             || {
+///                 Err(ArrowError::InvalidArgumentError(
+///                     "Uuid extension type metadata missing".to_owned(),
+///                 ))
+///             },
+///             str::parse,
+///         )
+///     }
+///
+///     // The only supported data type is `FixedSizeBinary(16)`.
+///     fn supports_data_type(&self, data_type: &DataType) -> Result<(), ArrowError> {
+///         match data_type {
+///             DataType::FixedSizeBinary(16) => Ok(()),
+///             data_type => Err(ArrowError::InvalidArgumentError(format!(
+///                 "Uuid data type mismatch, expected FixedSizeBinary(16), found {data_type}"
+///             ))),
+///         }
+///     }
+///
+///     // We should always check if the data type is supported before
+///     // constructing the extension type.
+///     fn try_new(data_type: &DataType, metadata: Self::Metadata) -> Result<Self, ArrowError> {
+///         let uuid = Self(metadata);
+///         uuid.supports_data_type(data_type)?;
+///         Ok(uuid)
+///     }
+/// }
+///
+/// // We can now construct the extension type.
+/// let uuid_v1 = Uuid(UuidVersion::V1);
+///
+/// // And add it to a field.
+/// let mut field =
+///     Field::new("", DataType::FixedSizeBinary(16), false).with_extension_type(uuid_v1);
+///
+/// // And extract it from this field.
+/// assert_eq!(field.try_extension_type::<Uuid>()?, uuid_v1);
+///
+/// // When we try to add this to a field with an unsupported data type we
+/// // get an error.
+/// let result = Field::new("", DataType::Null, false).try_with_extension_type(uuid_v1);
+/// assert!(result.is_err());
+/// # Ok(()) }
+/// ```
+///
+/// <https://arrow.apache.org/docs/format/Columnar.html#extension-types>
+///
+/// [`Field`]: crate::Field
+pub trait ExtensionType: Sized {
+    /// The name identifying this extension type.
+    ///
+    /// This is the string value that is used for the
+    /// [`EXTENSION_TYPE_NAME_KEY`] in the [`Field::metadata`] of a [`Field`]
+    /// to identify this extension type.
+    ///
+    /// We recommend that you use a “namespace”-style prefix for extension
+    /// type names to minimize the possibility of conflicts with multiple Arrow
+    /// readers and writers in the same application. For example, use
+    /// `myorg.name_of_type` instead of simply `name_of_type`.
+    ///
+    /// Extension names beginning with `arrow.` are reserved for canonical
+    /// extension types, they should not be used for third-party extension
+    /// types.
+    ///
+    /// Extension names are case-sensitive.
+    ///
+    /// [`Field`]: crate::Field
+    /// [`Field::metadata`]: crate::Field::metadata
+    const NAME: &'static str;
+
+    /// The metadata type of this extension type.
+    ///
+    /// If an extension type defines no metadata it should use `()` to indicate
+    /// this.
+    type Metadata;
+
+    /// Returns a reference to the metadata of this extension type, or `&()` if
+    /// if this extension type defines no metadata (`Self::Metadata=()`).
+    fn metadata(&self) -> &Self::Metadata;
+
+    /// Returns the serialized representation of the metadata of this extension
+    /// type, or `None` if this extension type defines no metadata
+    /// (`Self::Metadata=()`).
+    ///
+    /// This is string value that is used for the
+    /// [`EXTENSION_TYPE_METADATA_KEY`] in the [`Field::metadata`] of a
+    /// [`Field`].
+    ///
+    /// [`Field`]: crate::Field
+    /// [`Field::metadata`]: crate::Field::metadata
+    fn serialize_metadata(&self) -> Option<String>;

Review Comment:
   Probably `Result<String>` is enough, I think None and "" in this case doesn't matter



-- 
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: github-unsubscribe@arrow.apache.org

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

Re: [PR] Add `ExtensionType` trait and `CanonicalExtensionType` enum [arrow-rs]

Posted by "alamb (via GitHub)" <gi...@apache.org>.

alamb commented on code in PR #5822:
URL: https://github.com/apache/arrow-rs/pull/5822#discussion_r1925928159


##########
arrow-schema/src/extension/mod.rs:
##########
@@ -0,0 +1,252 @@
+// 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.
+
+//! Extension types.
+
+#[cfg(feature = "canonical-extension-types")]
+mod canonical;
+#[cfg(feature = "canonical-extension-types")]
+pub use canonical::*;
+
+use crate::{ArrowError, DataType};
+
+/// The metadata key for the string name identifying an [`ExtensionType`].
+pub const EXTENSION_TYPE_NAME_KEY: &str = "ARROW:extension:name";
+
+/// The metadata key for a serialized representation of the [`ExtensionType`]
+/// necessary to reconstruct the custom type.
+pub const EXTENSION_TYPE_METADATA_KEY: &str = "ARROW:extension:metadata";
+
+/// Extension types.
+///
+/// User-defined “extension” types can be defined setting certain key value
+/// pairs in the [`Field`] metadata structure. These extension keys are:
+/// - [`EXTENSION_TYPE_NAME_KEY`]
+/// - [`EXTENSION_TYPE_METADATA_KEY`]
+///
+/// Canonical extension types support in this crate requires the
+/// `canonical-extension-types` feature.
+///
+/// Extension types may or may not use the [`EXTENSION_TYPE_METADATA_KEY`]
+/// field.
+///
+/// # Example
+///
+/// The example below demonstrates how to implement this trait for a `Uuid`
+/// type. Note this is not the canonical extension type for `Uuid`, which does
+/// not include information about the `Uuid` version.
+///
+/// ```
+/// # use arrow_schema::ArrowError;
+/// # fn main() -> Result<(), ArrowError> {
+/// use arrow_schema::{DataType, extension::ExtensionType, Field};
+/// use std::{fmt, str::FromStr};
+///
+/// /// The different Uuid versions.
+/// #[derive(Clone, Copy, Debug, PartialEq)]
+/// enum UuidVersion {
+///     V1,
+///     V2,
+///     V3,
+///     V4,
+///     V5,
+///     V6,
+///     V7,
+///     V8,
+/// }
+///
+/// // We'll use `Display` to serialize.
+/// impl fmt::Display for UuidVersion {
+///     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+///         write!(
+///             f,
+///             "{}",
+///             match self {
+///                 Self::V1 => "V1",
+///                 Self::V2 => "V2",
+///                 Self::V3 => "V3",
+///                 Self::V4 => "V4",
+///                 Self::V5 => "V5",
+///                 Self::V6 => "V6",
+///                 Self::V7 => "V7",
+///                 Self::V8 => "V8",
+///             }
+///         )
+///     }
+/// }
+///
+/// // And `FromStr` to deserialize.
+/// impl FromStr for UuidVersion {
+///     type Err = ArrowError;
+///
+///     fn from_str(s: &str) -> Result<Self, Self::Err> {
+///         match s {
+///             "V1" => Ok(Self::V1),
+///             "V2" => Ok(Self::V2),
+///             "V3" => Ok(Self::V3),
+///             "V4" => Ok(Self::V4),
+///             "V5" => Ok(Self::V5),
+///             "V6" => Ok(Self::V6),
+///             "V7" => Ok(Self::V7),
+///             "V8" => Ok(Self::V8),
+///             _ => Err(ArrowError::ParseError("Invalid UuidVersion".to_owned())),
+///         }
+///     }
+/// }
+///
+/// /// This is the extension type, not the container for Uuid values. It
+/// /// stores the Uuid version (this is the metadata of this extension type).
+/// #[derive(Clone, Copy, Debug, PartialEq)]
+/// struct Uuid(UuidVersion);
+///
+/// impl ExtensionType for Uuid {
+///     // We use a namespace as suggested by the specification.
+///     const NAME: &'static str = "myorg.example.uuid";
+///
+///     // The metadata type is the Uuid version.
+///     type Metadata = UuidVersion;
+///
+///     // We just return a reference to the Uuid version.
+///     fn metadata(&self) -> &Self::Metadata {
+///         &self.0
+///     }
+///
+///     // We use the `Display` implementation to serialize the Uuid
+///     // version.
+///     fn serialize_metadata(&self) -> Option<String> {
+///         Some(self.0.to_string())
+///     }
+///
+///     // We use the `FromStr` implementation to deserialize the Uuid
+///     // version.
+///     fn deserialize_metadata(metadata: Option<&str>) -> Result<Self::Metadata, ArrowError> {
+///         metadata.map_or_else(
+///             || {
+///                 Err(ArrowError::InvalidArgumentError(
+///                     "Uuid extension type metadata missing".to_owned(),
+///                 ))
+///             },
+///             str::parse,
+///         )
+///     }
+///
+///     // The only supported data type is `FixedSizeBinary(16)`.
+///     fn supports_data_type(&self, data_type: &DataType) -> Result<(), ArrowError> {
+///         match data_type {
+///             DataType::FixedSizeBinary(16) => Ok(()),
+///             data_type => Err(ArrowError::InvalidArgumentError(format!(
+///                 "Uuid data type mismatch, expected FixedSizeBinary(16), found {data_type}"
+///             ))),
+///         }
+///     }
+///
+///     // We should always check if the data type is supported before
+///     // constructing the extension type.
+///     fn try_new(data_type: &DataType, metadata: Self::Metadata) -> Result<Self, ArrowError> {
+///         let uuid = Self(metadata);
+///         uuid.supports_data_type(data_type)?;
+///         Ok(uuid)
+///     }
+/// }
+///
+/// // We can now construct the extension type.
+/// let uuid_v1 = Uuid(UuidVersion::V1);
+///
+/// // And add it to a field.
+/// let mut field =
+///     Field::new("", DataType::FixedSizeBinary(16), false).with_extension_type(uuid_v1);
+///
+/// // And extract it from this field.
+/// assert_eq!(field.try_extension_type::<Uuid>()?, uuid_v1);
+///
+/// // When we try to add this to a field with an unsupported data type we
+/// // get an error.
+/// let result = Field::new("", DataType::Null, false).try_with_extension_type(uuid_v1);
+/// assert!(result.is_err());
+/// # Ok(()) }
+/// ```
+///
+/// <https://arrow.apache.org/docs/format/Columnar.html#extension-types>
+///
+/// [`Field`]: crate::Field
+pub trait ExtensionType: Sized {
+    /// The name identifying this extension type.
+    ///
+    /// This is the string value that is used for the
+    /// [`EXTENSION_TYPE_NAME_KEY`] in the [`Field::metadata`] of a [`Field`]
+    /// to identify this extension type.
+    ///
+    /// We recommend that you use a “namespace”-style prefix for extension
+    /// type names to minimize the possibility of conflicts with multiple Arrow
+    /// readers and writers in the same application. For example, use
+    /// `myorg.name_of_type` instead of simply `name_of_type`.
+    ///
+    /// Extension names beginning with `arrow.` are reserved for canonical
+    /// extension types, they should not be used for third-party extension
+    /// types.
+    ///
+    /// Extension names are case-sensitive.
+    ///
+    /// [`Field`]: crate::Field
+    /// [`Field::metadata`]: crate::Field::metadata
+    const NAME: &'static str;
+
+    /// The metadata type of this extension type.
+    ///
+    /// If an extension type defines no metadata it should use `()` to indicate
+    /// this.
+    type Metadata;

Review Comment:
   Ah, got it. Maybe we can add some comments explaining the rationale a bit more. 
   
   Or maybe we can default the `Metadata` type to `Self`
   
   ```rust
   type Metadata = Self;
   ```
   
   To be clear we can do this as a follow on PR (or never). I don't think it is necessary for this PR



-- 
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: github-unsubscribe@arrow.apache.org

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

Re: [PR] Add `ExtensionType` trait and `CanonicalExtensionType` enum [arrow-rs]

Posted by "mbrobbel (via GitHub)" <gi...@apache.org>.

mbrobbel commented on code in PR #5822:
URL: https://github.com/apache/arrow-rs/pull/5822#discussion_r1925947814


##########
arrow-schema/src/extension/mod.rs:
##########
@@ -0,0 +1,252 @@
+// 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.
+
+//! Extension types.
+
+#[cfg(feature = "canonical-extension-types")]

Review Comment:
   Added a warning in 5fec56d8ff81d14902274bd45e9b41e634a9379e.



-- 
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: github-unsubscribe@arrow.apache.org

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

Re: [PR] Add `ExtensionType` for `uuid` and map to parquet logical type [arrow-rs]

Posted by "aykut-bozkurt (via GitHub)" <gi...@apache.org>.

aykut-bozkurt commented on PR #5822:
URL: https://github.com/apache/arrow-rs/pull/5822#issuecomment-2281456210

   Thanks for the PR. This seems very useful to support not yet mapped logical types. e.g. json


-- 
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: github-unsubscribe@arrow.apache.org

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

Re: [PR] Add `ExtensionType` trait and `CanonicalExtensionType` enum [arrow-rs]

Posted by "mbrobbel (via GitHub)" <gi...@apache.org>.

mbrobbel commented on code in PR #5822:
URL: https://github.com/apache/arrow-rs/pull/5822#discussion_r1925942369


##########
parquet/src/arrow/schema/mod.rs:
##########
@@ -2163,4 +2211,52 @@ mod tests {
     fn test_get_arrow_schema_from_metadata() {
         assert!(get_arrow_schema_from_metadata("").is_err());
     }
+
+    #[test]
+    #[cfg(feature = "arrow_canonical_extension_types")]
+    fn arrow_uuid_to_parquet_uuid() -> Result<()> {
+        let arrow_schema = Schema::new(vec![Field::new(
+            "uuid",
+            DataType::FixedSizeBinary(16),
+            false,
+        )
+        .with_extension_type(Uuid)]);
+
+        let parquet_schema = ArrowSchemaConverter::new().convert(&arrow_schema)?;
+
+        assert_eq!(
+            parquet_schema.column(0).logical_type(),
+            Some(LogicalType::Uuid)
+        );
+
+        // TODO: roundtrip
+        // let arrow_schema = parquet_to_arrow_schema(&parquet_schema, None)?;
+        // assert_eq!(arrow_schema.field(0).try_extension_type::<Uuid>()?, Uuid);
+
+        Ok(())
+    }
+
+    #[test]
+    #[cfg(feature = "arrow_canonical_extension_types")]
+    fn arrow_json_to_parquet_json() -> Result<()> {
+        let arrow_schema = Schema::new(vec![
+            Field::new("json", DataType::Utf8, false).with_extension_type(Json::default())
+        ]);
+
+        let parquet_schema = ArrowSchemaConverter::new().convert(&arrow_schema)?;
+
+        assert_eq!(
+            parquet_schema.column(0).logical_type(),
+            Some(LogicalType::Json)
+        );
+
+        // TODO: roundtrip
+        // let arrow_schema = parquet_to_arrow_schema(&parquet_schema, None)?;
+        // assert_eq!(
+        //     arrow_schema.field(0).try_extension_type::<Json>()?,
+        //     Json::default()
+        // );

Review Comment:
   Yes, this requires a bit more work to propagate correctly.



-- 
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: github-unsubscribe@arrow.apache.org

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

Re: [PR] Add `ExtensionType` trait and `CanonicalExtensionType` enum [arrow-rs]

Posted by "mbrobbel (via GitHub)" <gi...@apache.org>.

mbrobbel commented on PR #5822:
URL: https://github.com/apache/arrow-rs/pull/5822#issuecomment-2603111058

   This is ready for another round of reviews.
   
   I updated the trait and changed some field methods, added implementations for the current set of canonical extension types (behind a feature flag) and added more tests and docs. Maybe this PR is too big now, but I don't mind splitting it up if needed.
   
   Roundtripping through parquet is not implemented in this PR.


-- 
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: github-unsubscribe@arrow.apache.org

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

Re: [PR] Add `ExtensionType` trait and `CanonicalExtensionType` enum [arrow-rs]

Posted by "mbrobbel (via GitHub)" <gi...@apache.org>.

mbrobbel commented on code in PR #5822:
URL: https://github.com/apache/arrow-rs/pull/5822#discussion_r1928516990


##########
arrow-schema/src/extension/canonical/json.rs:
##########
@@ -0,0 +1,189 @@
+// 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.
+
+//! JSON
+//!
+//! <https://arrow.apache.org/docs/format/CanonicalExtensions.html#json>
+
+use serde_json::Value;
+
+use crate::{extension::ExtensionType, ArrowError, DataType};
+
+/// The extension type for `JSON`.
+///
+/// Extension name: `arrow.json`.
+///
+/// The storage type of this extension is `String` or `LargeString` or
+/// `StringView`. Only UTF-8 encoded JSON as specified in [rfc8259](https://datatracker.ietf.org/doc/html/rfc8259)
+/// is supported.
+///
+/// This type does not have any parameters.
+///
+/// Metadata is either an empty string or a JSON string with an empty

Review Comment:
   Good catch! [29a94cb](https://github.com/apache/arrow-rs/pull/5822/commits/29a94cbed185515f4eb8cad362f1d2c242fd8eac)



-- 
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: github-unsubscribe@arrow.apache.org

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

Re: [PR] Add `ExtensionType` trait and `CanonicalExtensionType` enum [arrow-rs]

Posted by "jayzhan211 (via GitHub)" <gi...@apache.org>.

jayzhan211 commented on code in PR #5822:
URL: https://github.com/apache/arrow-rs/pull/5822#discussion_r1925326369


##########
arrow-schema/src/extension/mod.rs:
##########
@@ -0,0 +1,252 @@
+// 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.
+
+//! Extension types.
+
+#[cfg(feature = "canonical-extension-types")]
+mod canonical;
+#[cfg(feature = "canonical-extension-types")]
+pub use canonical::*;
+
+use crate::{ArrowError, DataType};
+
+/// The metadata key for the string name identifying an [`ExtensionType`].
+pub const EXTENSION_TYPE_NAME_KEY: &str = "ARROW:extension:name";
+
+/// The metadata key for a serialized representation of the [`ExtensionType`]
+/// necessary to reconstruct the custom type.
+pub const EXTENSION_TYPE_METADATA_KEY: &str = "ARROW:extension:metadata";
+
+/// Extension types.
+///
+/// User-defined “extension” types can be defined setting certain key value
+/// pairs in the [`Field`] metadata structure. These extension keys are:
+/// - [`EXTENSION_TYPE_NAME_KEY`]
+/// - [`EXTENSION_TYPE_METADATA_KEY`]
+///
+/// Canonical extension types support in this crate requires the
+/// `canonical-extension-types` feature.
+///
+/// Extension types may or may not use the [`EXTENSION_TYPE_METADATA_KEY`]
+/// field.
+///
+/// # Example
+///
+/// The example below demonstrates how to implement this trait for a `Uuid`
+/// type. Note this is not the canonical extension type for `Uuid`, which does
+/// not include information about the `Uuid` version.
+///
+/// ```
+/// # use arrow_schema::ArrowError;
+/// # fn main() -> Result<(), ArrowError> {
+/// use arrow_schema::{DataType, extension::ExtensionType, Field};
+/// use std::{fmt, str::FromStr};
+///
+/// /// The different Uuid versions.
+/// #[derive(Clone, Copy, Debug, PartialEq)]
+/// enum UuidVersion {
+///     V1,
+///     V2,
+///     V3,
+///     V4,
+///     V5,
+///     V6,
+///     V7,
+///     V8,
+/// }
+///
+/// // We'll use `Display` to serialize.
+/// impl fmt::Display for UuidVersion {
+///     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+///         write!(
+///             f,
+///             "{}",
+///             match self {
+///                 Self::V1 => "V1",
+///                 Self::V2 => "V2",
+///                 Self::V3 => "V3",
+///                 Self::V4 => "V4",
+///                 Self::V5 => "V5",
+///                 Self::V6 => "V6",
+///                 Self::V7 => "V7",
+///                 Self::V8 => "V8",
+///             }
+///         )
+///     }
+/// }
+///
+/// // And `FromStr` to deserialize.
+/// impl FromStr for UuidVersion {
+///     type Err = ArrowError;
+///
+///     fn from_str(s: &str) -> Result<Self, Self::Err> {
+///         match s {
+///             "V1" => Ok(Self::V1),
+///             "V2" => Ok(Self::V2),
+///             "V3" => Ok(Self::V3),
+///             "V4" => Ok(Self::V4),
+///             "V5" => Ok(Self::V5),
+///             "V6" => Ok(Self::V6),
+///             "V7" => Ok(Self::V7),
+///             "V8" => Ok(Self::V8),
+///             _ => Err(ArrowError::ParseError("Invalid UuidVersion".to_owned())),
+///         }
+///     }
+/// }
+///
+/// /// This is the extension type, not the container for Uuid values. It
+/// /// stores the Uuid version (this is the metadata of this extension type).
+/// #[derive(Clone, Copy, Debug, PartialEq)]
+/// struct Uuid(UuidVersion);
+///
+/// impl ExtensionType for Uuid {
+///     // We use a namespace as suggested by the specification.
+///     const NAME: &'static str = "myorg.example.uuid";
+///
+///     // The metadata type is the Uuid version.
+///     type Metadata = UuidVersion;
+///
+///     // We just return a reference to the Uuid version.
+///     fn metadata(&self) -> &Self::Metadata {
+///         &self.0
+///     }
+///
+///     // We use the `Display` implementation to serialize the Uuid
+///     // version.
+///     fn serialize_metadata(&self) -> Option<String> {
+///         Some(self.0.to_string())
+///     }
+///
+///     // We use the `FromStr` implementation to deserialize the Uuid
+///     // version.
+///     fn deserialize_metadata(metadata: Option<&str>) -> Result<Self::Metadata, ArrowError> {
+///         metadata.map_or_else(
+///             || {
+///                 Err(ArrowError::InvalidArgumentError(
+///                     "Uuid extension type metadata missing".to_owned(),
+///                 ))
+///             },
+///             str::parse,
+///         )
+///     }
+///
+///     // The only supported data type is `FixedSizeBinary(16)`.
+///     fn supports_data_type(&self, data_type: &DataType) -> Result<(), ArrowError> {
+///         match data_type {
+///             DataType::FixedSizeBinary(16) => Ok(()),
+///             data_type => Err(ArrowError::InvalidArgumentError(format!(
+///                 "Uuid data type mismatch, expected FixedSizeBinary(16), found {data_type}"
+///             ))),
+///         }
+///     }
+///
+///     // We should always check if the data type is supported before
+///     // constructing the extension type.
+///     fn try_new(data_type: &DataType, metadata: Self::Metadata) -> Result<Self, ArrowError> {
+///         let uuid = Self(metadata);
+///         uuid.supports_data_type(data_type)?;
+///         Ok(uuid)
+///     }
+/// }
+///
+/// // We can now construct the extension type.
+/// let uuid_v1 = Uuid(UuidVersion::V1);
+///
+/// // And add it to a field.
+/// let mut field =
+///     Field::new("", DataType::FixedSizeBinary(16), false).with_extension_type(uuid_v1);
+///
+/// // And extract it from this field.
+/// assert_eq!(field.try_extension_type::<Uuid>()?, uuid_v1);
+///
+/// // When we try to add this to a field with an unsupported data type we
+/// // get an error.
+/// let result = Field::new("", DataType::Null, false).try_with_extension_type(uuid_v1);
+/// assert!(result.is_err());
+/// # Ok(()) }
+/// ```
+///
+/// <https://arrow.apache.org/docs/format/Columnar.html#extension-types>
+///
+/// [`Field`]: crate::Field
+pub trait ExtensionType: Sized {
+    /// The name identifying this extension type.
+    ///
+    /// This is the string value that is used for the
+    /// [`EXTENSION_TYPE_NAME_KEY`] in the [`Field::metadata`] of a [`Field`]
+    /// to identify this extension type.
+    ///
+    /// We recommend that you use a “namespace”-style prefix for extension
+    /// type names to minimize the possibility of conflicts with multiple Arrow
+    /// readers and writers in the same application. For example, use
+    /// `myorg.name_of_type` instead of simply `name_of_type`.
+    ///
+    /// Extension names beginning with `arrow.` are reserved for canonical
+    /// extension types, they should not be used for third-party extension
+    /// types.
+    ///
+    /// Extension names are case-sensitive.
+    ///
+    /// [`Field`]: crate::Field
+    /// [`Field::metadata`]: crate::Field::metadata
+    const NAME: &'static str;
+
+    /// The metadata type of this extension type.
+    ///
+    /// If an extension type defines no metadata it should use `()` to indicate
+    /// this.
+    type Metadata;

Review Comment:
   In Json, `serde_json::Value` is the metadata instead of String, with your design I think we are not able to store metadata other than String



-- 
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: github-unsubscribe@arrow.apache.org

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

Re: [PR] Add `ExtensionType` trait and `CanonicalExtensionType` enum [arrow-rs]

Posted by "paleolimbot (via GitHub)" <gi...@apache.org>.

paleolimbot commented on code in PR #5822:
URL: https://github.com/apache/arrow-rs/pull/5822#discussion_r1925512902


##########
arrow-schema/src/extension/mod.rs:
##########
@@ -0,0 +1,252 @@
+// 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.
+
+//! Extension types.
+
+#[cfg(feature = "canonical-extension-types")]
+mod canonical;
+#[cfg(feature = "canonical-extension-types")]
+pub use canonical::*;
+
+use crate::{ArrowError, DataType};
+
+/// The metadata key for the string name identifying an [`ExtensionType`].
+pub const EXTENSION_TYPE_NAME_KEY: &str = "ARROW:extension:name";
+
+/// The metadata key for a serialized representation of the [`ExtensionType`]
+/// necessary to reconstruct the custom type.
+pub const EXTENSION_TYPE_METADATA_KEY: &str = "ARROW:extension:metadata";
+
+/// Extension types.
+///
+/// User-defined “extension” types can be defined setting certain key value
+/// pairs in the [`Field`] metadata structure. These extension keys are:
+/// - [`EXTENSION_TYPE_NAME_KEY`]
+/// - [`EXTENSION_TYPE_METADATA_KEY`]
+///
+/// Canonical extension types support in this crate requires the
+/// `canonical-extension-types` feature.
+///
+/// Extension types may or may not use the [`EXTENSION_TYPE_METADATA_KEY`]
+/// field.
+///
+/// # Example
+///
+/// The example below demonstrates how to implement this trait for a `Uuid`
+/// type. Note this is not the canonical extension type for `Uuid`, which does
+/// not include information about the `Uuid` version.
+///
+/// ```
+/// # use arrow_schema::ArrowError;
+/// # fn main() -> Result<(), ArrowError> {
+/// use arrow_schema::{DataType, extension::ExtensionType, Field};
+/// use std::{fmt, str::FromStr};
+///
+/// /// The different Uuid versions.
+/// #[derive(Clone, Copy, Debug, PartialEq)]
+/// enum UuidVersion {
+///     V1,
+///     V2,
+///     V3,
+///     V4,
+///     V5,
+///     V6,
+///     V7,
+///     V8,
+/// }
+///
+/// // We'll use `Display` to serialize.
+/// impl fmt::Display for UuidVersion {
+///     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+///         write!(
+///             f,
+///             "{}",
+///             match self {
+///                 Self::V1 => "V1",
+///                 Self::V2 => "V2",
+///                 Self::V3 => "V3",
+///                 Self::V4 => "V4",
+///                 Self::V5 => "V5",
+///                 Self::V6 => "V6",
+///                 Self::V7 => "V7",
+///                 Self::V8 => "V8",
+///             }
+///         )
+///     }
+/// }
+///
+/// // And `FromStr` to deserialize.
+/// impl FromStr for UuidVersion {
+///     type Err = ArrowError;
+///
+///     fn from_str(s: &str) -> Result<Self, Self::Err> {
+///         match s {
+///             "V1" => Ok(Self::V1),
+///             "V2" => Ok(Self::V2),
+///             "V3" => Ok(Self::V3),
+///             "V4" => Ok(Self::V4),
+///             "V5" => Ok(Self::V5),
+///             "V6" => Ok(Self::V6),
+///             "V7" => Ok(Self::V7),
+///             "V8" => Ok(Self::V8),
+///             _ => Err(ArrowError::ParseError("Invalid UuidVersion".to_owned())),
+///         }
+///     }
+/// }
+///
+/// /// This is the extension type, not the container for Uuid values. It
+/// /// stores the Uuid version (this is the metadata of this extension type).
+/// #[derive(Clone, Copy, Debug, PartialEq)]
+/// struct Uuid(UuidVersion);
+///
+/// impl ExtensionType for Uuid {
+///     // We use a namespace as suggested by the specification.
+///     const NAME: &'static str = "myorg.example.uuid";
+///
+///     // The metadata type is the Uuid version.
+///     type Metadata = UuidVersion;
+///
+///     // We just return a reference to the Uuid version.
+///     fn metadata(&self) -> &Self::Metadata {
+///         &self.0
+///     }
+///
+///     // We use the `Display` implementation to serialize the Uuid
+///     // version.
+///     fn serialize_metadata(&self) -> Option<String> {
+///         Some(self.0.to_string())
+///     }
+///
+///     // We use the `FromStr` implementation to deserialize the Uuid
+///     // version.
+///     fn deserialize_metadata(metadata: Option<&str>) -> Result<Self::Metadata, ArrowError> {
+///         metadata.map_or_else(
+///             || {
+///                 Err(ArrowError::InvalidArgumentError(
+///                     "Uuid extension type metadata missing".to_owned(),
+///                 ))
+///             },
+///             str::parse,
+///         )
+///     }
+///
+///     // The only supported data type is `FixedSizeBinary(16)`.
+///     fn supports_data_type(&self, data_type: &DataType) -> Result<(), ArrowError> {
+///         match data_type {
+///             DataType::FixedSizeBinary(16) => Ok(()),
+///             data_type => Err(ArrowError::InvalidArgumentError(format!(
+///                 "Uuid data type mismatch, expected FixedSizeBinary(16), found {data_type}"
+///             ))),
+///         }
+///     }
+///
+///     // We should always check if the data type is supported before
+///     // constructing the extension type.
+///     fn try_new(data_type: &DataType, metadata: Self::Metadata) -> Result<Self, ArrowError> {
+///         let uuid = Self(metadata);
+///         uuid.supports_data_type(data_type)?;
+///         Ok(uuid)
+///     }
+/// }
+///
+/// // We can now construct the extension type.
+/// let uuid_v1 = Uuid(UuidVersion::V1);
+///
+/// // And add it to a field.
+/// let mut field =
+///     Field::new("", DataType::FixedSizeBinary(16), false).with_extension_type(uuid_v1);
+///
+/// // And extract it from this field.
+/// assert_eq!(field.try_extension_type::<Uuid>()?, uuid_v1);
+///
+/// // When we try to add this to a field with an unsupported data type we
+/// // get an error.
+/// let result = Field::new("", DataType::Null, false).try_with_extension_type(uuid_v1);
+/// assert!(result.is_err());
+/// # Ok(()) }
+/// ```
+///
+/// <https://arrow.apache.org/docs/format/Columnar.html#extension-types>
+///
+/// [`Field`]: crate::Field
+pub trait ExtensionType: Sized {
+    /// The name identifying this extension type.
+    ///
+    /// This is the string value that is used for the
+    /// [`EXTENSION_TYPE_NAME_KEY`] in the [`Field::metadata`] of a [`Field`]
+    /// to identify this extension type.
+    ///
+    /// We recommend that you use a “namespace”-style prefix for extension
+    /// type names to minimize the possibility of conflicts with multiple Arrow
+    /// readers and writers in the same application. For example, use
+    /// `myorg.name_of_type` instead of simply `name_of_type`.
+    ///
+    /// Extension names beginning with `arrow.` are reserved for canonical
+    /// extension types, they should not be used for third-party extension
+    /// types.
+    ///
+    /// Extension names are case-sensitive.
+    ///
+    /// [`Field`]: crate::Field
+    /// [`Field::metadata`]: crate::Field::metadata
+    const NAME: &'static str;
+
+    /// The metadata type of this extension type.
+    ///
+    /// If an extension type defines no metadata it should use `()` to indicate
+    /// this.
+    type Metadata;
+
+    /// Returns a reference to the metadata of this extension type, or `&()` if
+    /// if this extension type defines no metadata (`Self::Metadata=()`).
+    fn metadata(&self) -> &Self::Metadata;
+
+    /// Returns the serialized representation of the metadata of this extension
+    /// type, or `None` if this extension type defines no metadata
+    /// (`Self::Metadata=()`).
+    ///
+    /// This is string value that is used for the
+    /// [`EXTENSION_TYPE_METADATA_KEY`] in the [`Field::metadata`] of a
+    /// [`Field`].
+    ///
+    /// [`Field`]: crate::Field
+    /// [`Field::metadata`]: crate::Field::metadata
+    fn serialize_metadata(&self) -> Option<String>;

Review Comment:
   > because in that case the extension metadata key is added to the field custom metadata, whereas for None this key is skipped.
   
   I forget what version of Arrow C++ I fixed this in, but old versions or Arrow C++ (maybe older than ~a year) will segfault if `ARROW:extension:metadata` is missing when importing via the C Data interface, so it might be worth always serializing an empty string.



-- 
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: github-unsubscribe@arrow.apache.org

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

Re: [PR] Add `ExtensionType` trait and `CanonicalExtensionType` enum [arrow-rs]

Posted by "mbrobbel (via GitHub)" <gi...@apache.org>.

mbrobbel commented on code in PR #5822:
URL: https://github.com/apache/arrow-rs/pull/5822#discussion_r1925452582


##########
arrow-schema/src/extension/mod.rs:
##########
@@ -0,0 +1,252 @@
+// 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.
+
+//! Extension types.
+
+#[cfg(feature = "canonical-extension-types")]
+mod canonical;
+#[cfg(feature = "canonical-extension-types")]
+pub use canonical::*;
+
+use crate::{ArrowError, DataType};
+
+/// The metadata key for the string name identifying an [`ExtensionType`].
+pub const EXTENSION_TYPE_NAME_KEY: &str = "ARROW:extension:name";
+
+/// The metadata key for a serialized representation of the [`ExtensionType`]
+/// necessary to reconstruct the custom type.
+pub const EXTENSION_TYPE_METADATA_KEY: &str = "ARROW:extension:metadata";
+
+/// Extension types.
+///
+/// User-defined “extension” types can be defined setting certain key value
+/// pairs in the [`Field`] metadata structure. These extension keys are:
+/// - [`EXTENSION_TYPE_NAME_KEY`]
+/// - [`EXTENSION_TYPE_METADATA_KEY`]
+///
+/// Canonical extension types support in this crate requires the
+/// `canonical-extension-types` feature.
+///
+/// Extension types may or may not use the [`EXTENSION_TYPE_METADATA_KEY`]
+/// field.
+///
+/// # Example
+///
+/// The example below demonstrates how to implement this trait for a `Uuid`
+/// type. Note this is not the canonical extension type for `Uuid`, which does
+/// not include information about the `Uuid` version.
+///
+/// ```
+/// # use arrow_schema::ArrowError;
+/// # fn main() -> Result<(), ArrowError> {
+/// use arrow_schema::{DataType, extension::ExtensionType, Field};
+/// use std::{fmt, str::FromStr};
+///
+/// /// The different Uuid versions.
+/// #[derive(Clone, Copy, Debug, PartialEq)]
+/// enum UuidVersion {
+///     V1,
+///     V2,
+///     V3,
+///     V4,
+///     V5,
+///     V6,
+///     V7,
+///     V8,
+/// }
+///
+/// // We'll use `Display` to serialize.
+/// impl fmt::Display for UuidVersion {
+///     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+///         write!(
+///             f,
+///             "{}",
+///             match self {
+///                 Self::V1 => "V1",
+///                 Self::V2 => "V2",
+///                 Self::V3 => "V3",
+///                 Self::V4 => "V4",
+///                 Self::V5 => "V5",
+///                 Self::V6 => "V6",
+///                 Self::V7 => "V7",
+///                 Self::V8 => "V8",
+///             }
+///         )
+///     }
+/// }
+///
+/// // And `FromStr` to deserialize.
+/// impl FromStr for UuidVersion {
+///     type Err = ArrowError;
+///
+///     fn from_str(s: &str) -> Result<Self, Self::Err> {
+///         match s {
+///             "V1" => Ok(Self::V1),
+///             "V2" => Ok(Self::V2),
+///             "V3" => Ok(Self::V3),
+///             "V4" => Ok(Self::V4),
+///             "V5" => Ok(Self::V5),
+///             "V6" => Ok(Self::V6),
+///             "V7" => Ok(Self::V7),
+///             "V8" => Ok(Self::V8),
+///             _ => Err(ArrowError::ParseError("Invalid UuidVersion".to_owned())),
+///         }
+///     }
+/// }
+///
+/// /// This is the extension type, not the container for Uuid values. It
+/// /// stores the Uuid version (this is the metadata of this extension type).
+/// #[derive(Clone, Copy, Debug, PartialEq)]
+/// struct Uuid(UuidVersion);
+///
+/// impl ExtensionType for Uuid {
+///     // We use a namespace as suggested by the specification.
+///     const NAME: &'static str = "myorg.example.uuid";
+///
+///     // The metadata type is the Uuid version.
+///     type Metadata = UuidVersion;
+///
+///     // We just return a reference to the Uuid version.
+///     fn metadata(&self) -> &Self::Metadata {
+///         &self.0
+///     }
+///
+///     // We use the `Display` implementation to serialize the Uuid
+///     // version.
+///     fn serialize_metadata(&self) -> Option<String> {
+///         Some(self.0.to_string())
+///     }
+///
+///     // We use the `FromStr` implementation to deserialize the Uuid
+///     // version.
+///     fn deserialize_metadata(metadata: Option<&str>) -> Result<Self::Metadata, ArrowError> {
+///         metadata.map_or_else(
+///             || {
+///                 Err(ArrowError::InvalidArgumentError(
+///                     "Uuid extension type metadata missing".to_owned(),
+///                 ))
+///             },
+///             str::parse,
+///         )
+///     }
+///
+///     // The only supported data type is `FixedSizeBinary(16)`.
+///     fn supports_data_type(&self, data_type: &DataType) -> Result<(), ArrowError> {
+///         match data_type {
+///             DataType::FixedSizeBinary(16) => Ok(()),
+///             data_type => Err(ArrowError::InvalidArgumentError(format!(
+///                 "Uuid data type mismatch, expected FixedSizeBinary(16), found {data_type}"
+///             ))),
+///         }
+///     }
+///
+///     // We should always check if the data type is supported before
+///     // constructing the extension type.
+///     fn try_new(data_type: &DataType, metadata: Self::Metadata) -> Result<Self, ArrowError> {
+///         let uuid = Self(metadata);
+///         uuid.supports_data_type(data_type)?;
+///         Ok(uuid)
+///     }
+/// }
+///
+/// // We can now construct the extension type.
+/// let uuid_v1 = Uuid(UuidVersion::V1);
+///
+/// // And add it to a field.
+/// let mut field =
+///     Field::new("", DataType::FixedSizeBinary(16), false).with_extension_type(uuid_v1);
+///
+/// // And extract it from this field.
+/// assert_eq!(field.try_extension_type::<Uuid>()?, uuid_v1);
+///
+/// // When we try to add this to a field with an unsupported data type we
+/// // get an error.
+/// let result = Field::new("", DataType::Null, false).try_with_extension_type(uuid_v1);
+/// assert!(result.is_err());
+/// # Ok(()) }
+/// ```
+///
+/// <https://arrow.apache.org/docs/format/Columnar.html#extension-types>
+///
+/// [`Field`]: crate::Field
+pub trait ExtensionType: Sized {
+    /// The name identifying this extension type.
+    ///
+    /// This is the string value that is used for the
+    /// [`EXTENSION_TYPE_NAME_KEY`] in the [`Field::metadata`] of a [`Field`]
+    /// to identify this extension type.
+    ///
+    /// We recommend that you use a “namespace”-style prefix for extension
+    /// type names to minimize the possibility of conflicts with multiple Arrow
+    /// readers and writers in the same application. For example, use
+    /// `myorg.name_of_type` instead of simply `name_of_type`.
+    ///
+    /// Extension names beginning with `arrow.` are reserved for canonical
+    /// extension types, they should not be used for third-party extension
+    /// types.
+    ///
+    /// Extension names are case-sensitive.
+    ///
+    /// [`Field`]: crate::Field
+    /// [`Field::metadata`]: crate::Field::metadata
+    const NAME: &'static str;
+
+    /// The metadata type of this extension type.
+    ///
+    /// If an extension type defines no metadata it should use `()` to indicate
+    /// this.
+    type Metadata;

Review Comment:
   This doesn't work for all extension types. For example the `FixedShapeTensor` and `VariableShapeTensor` canonical extension types can't be constructed from just the metadata, they also need the field datatype (which explains the `ExtensionType::try_new` signature).



-- 
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: github-unsubscribe@arrow.apache.org

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

Re: [PR] Add `ExtensionType` trait and `CanonicalExtensionType` enum [arrow-rs]

Posted by "alamb (via GitHub)" <gi...@apache.org>.

alamb commented on code in PR #5822:
URL: https://github.com/apache/arrow-rs/pull/5822#discussion_r1938475298


##########
parquet/src/arrow/schema/mod.rs:
##########
@@ -2163,4 +2211,52 @@ mod tests {
     fn test_get_arrow_schema_from_metadata() {
         assert!(get_arrow_schema_from_metadata("").is_err());
     }
+
+    #[test]
+    #[cfg(feature = "arrow_canonical_extension_types")]
+    fn arrow_uuid_to_parquet_uuid() -> Result<()> {
+        let arrow_schema = Schema::new(vec![Field::new(
+            "uuid",
+            DataType::FixedSizeBinary(16),
+            false,
+        )
+        .with_extension_type(Uuid)]);
+
+        let parquet_schema = ArrowSchemaConverter::new().convert(&arrow_schema)?;
+
+        assert_eq!(
+            parquet_schema.column(0).logical_type(),
+            Some(LogicalType::Uuid)
+        );
+
+        // TODO: roundtrip
+        // let arrow_schema = parquet_to_arrow_schema(&parquet_schema, None)?;
+        // assert_eq!(arrow_schema.field(0).try_extension_type::<Uuid>()?, Uuid);
+
+        Ok(())
+    }
+
+    #[test]
+    #[cfg(feature = "arrow_canonical_extension_types")]
+    fn arrow_json_to_parquet_json() -> Result<()> {
+        let arrow_schema = Schema::new(vec![
+            Field::new("json", DataType::Utf8, false).with_extension_type(Json::default())
+        ]);
+
+        let parquet_schema = ArrowSchemaConverter::new().convert(&arrow_schema)?;
+
+        assert_eq!(
+            parquet_schema.column(0).logical_type(),
+            Some(LogicalType::Json)
+        );
+
+        // TODO: roundtrip
+        // let arrow_schema = parquet_to_arrow_schema(&parquet_schema, None)?;
+        // assert_eq!(
+        //     arrow_schema.field(0).try_extension_type::<Json>()?,
+        //     Json::default()
+        // );

Review Comment:
   - Filed https://github.com/apache/arrow-rs/issues/7063 to track



-- 
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: github-unsubscribe@arrow.apache.org

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

Re: [PR] Add `ExtensionType` trait and `CanonicalExtensionType` enum [arrow-rs]

Posted by "kylebarron (via GitHub)" <gi...@apache.org>.

kylebarron commented on code in PR #5822:
URL: https://github.com/apache/arrow-rs/pull/5822#discussion_r2080387205


##########
arrow-schema/src/field.rs:
##########
@@ -350,6 +355,167 @@ impl Field {
         self
     }
 
+    /// Returns the extension type name of this [`Field`], if set.
+    ///
+    /// This returns the value of [`EXTENSION_TYPE_NAME_KEY`], if set in
+    /// [`Field::metadata`]. If the key is missing, there is no extension type
+    /// name and this returns `None`.
+    ///
+    /// # Example
+    ///
+    /// ```
+    /// # use arrow_schema::{DataType, extension::EXTENSION_TYPE_NAME_KEY, Field};
+    ///
+    /// let field = Field::new("", DataType::Null, false);
+    /// assert_eq!(field.extension_type_name(), None);
+    ///
+    /// let field = Field::new("", DataType::Null, false).with_metadata(
+    ///    [(EXTENSION_TYPE_NAME_KEY.to_owned(), "example".to_owned())]
+    ///        .into_iter()
+    ///        .collect(),
+    /// );
+    /// assert_eq!(field.extension_type_name(), Some("example"));
+    /// ```
+    pub fn extension_type_name(&self) -> Option<&str> {
+        self.metadata()
+            .get(EXTENSION_TYPE_NAME_KEY)
+            .map(String::as_ref)
+    }
+
+    /// Returns the extension type metadata of this [`Field`], if set.
+    ///
+    /// This returns the value of [`EXTENSION_TYPE_METADATA_KEY`], if set in
+    /// [`Field::metadata`]. If the key is missing, there is no extension type
+    /// metadata and this returns `None`.
+    ///
+    /// # Example
+    ///
+    /// ```
+    /// # use arrow_schema::{DataType, extension::EXTENSION_TYPE_METADATA_KEY, Field};
+    ///
+    /// let field = Field::new("", DataType::Null, false);
+    /// assert_eq!(field.extension_type_metadata(), None);
+    ///
+    /// let field = Field::new("", DataType::Null, false).with_metadata(
+    ///    [(EXTENSION_TYPE_METADATA_KEY.to_owned(), "example".to_owned())]
+    ///        .into_iter()
+    ///        .collect(),
+    /// );
+    /// assert_eq!(field.extension_type_metadata(), Some("example"));
+    /// ```
+    pub fn extension_type_metadata(&self) -> Option<&str> {
+        self.metadata()
+            .get(EXTENSION_TYPE_METADATA_KEY)
+            .map(String::as_ref)
+    }
+
+    /// Returns an instance of the given [`ExtensionType`] of this [`Field`],
+    /// if set in the [`Field::metadata`].
+    ///
+    /// # Error
+    ///
+    /// Returns an error if
+    /// - this field does not have the name of this extension type
+    ///   ([`ExtensionType::NAME`]) in the [`Field::metadata`] (mismatch or
+    ///   missing)
+    /// - the deserialization of the metadata
+    ///   ([`ExtensionType::deserialize_metadata`]) fails
+    /// - the construction of the extension type ([`ExtensionType::try_new`])
+    ///   fail (for example when the [`Field::data_type`] is not supported by
+    ///   the extension type ([`ExtensionType::supports_data_type`]))
+    pub fn try_extension_type<E: ExtensionType>(&self) -> Result<E, ArrowError> {
+        // Check the extension name in the metadata
+        match self.extension_type_name() {
+            // It should match the name of the given extension type
+            Some(name) if name == E::NAME => {
+                // Deserialize the metadata and try to construct the extension
+                // type
+                E::deserialize_metadata(self.extension_type_metadata())
+                    .and_then(|metadata| E::try_new(self.data_type(), metadata))
+            }
+            // Name mismatch
+            Some(name) => Err(ArrowError::InvalidArgumentError(format!(
+                "Field extension type name mismatch, expected {}, found {name}",
+                E::NAME
+            ))),
+            // Name missing
+            None => Err(ArrowError::InvalidArgumentError(
+                "Field extension type name missing".to_owned(),
+            )),
+        }
+    }
+
+    /// Returns an instance of the given [`ExtensionType`] of this [`Field`],
+    /// panics if this [`Field`] does not have this extension type.
+    ///
+    /// # Panic
+    ///
+    /// This calls [`Field::try_extension_type`] and panics when it returns an
+    /// error.
+    pub fn extension_type<E: ExtensionType>(&self) -> E {
+        self.try_extension_type::<E>()
+            .unwrap_or_else(|e| panic!("{e}"))
+    }
+
+    /// Updates the metadata of this [`Field`] with the [`ExtensionType::NAME`]
+    /// and [`ExtensionType::metadata`] of the given [`ExtensionType`], if the
+    /// given extension type supports the [`Field::data_type`] of this field
+    /// ([`ExtensionType::supports_data_type`]).
+    ///
+    /// If the given extension type defines no metadata, a previously set
+    /// value of [`EXTENSION_TYPE_METADATA_KEY`] is cleared.
+    ///
+    /// # Error
+    ///
+    /// This functions returns an error if the data type of this field does not
+    /// match any of the supported storage types of the given extension type.
+    pub fn try_with_extension_type<E: ExtensionType>(
+        &mut self,
+        extension_type: E,
+    ) -> Result<(), ArrowError> {
+        // Make sure the data type of this field is supported
+        extension_type.supports_data_type(&self.data_type)?;
+
+        self.metadata
+            .insert(EXTENSION_TYPE_NAME_KEY.to_owned(), E::NAME.to_owned());
+        match extension_type.serialize_metadata() {
+            Some(metadata) => self
+                .metadata
+                .insert(EXTENSION_TYPE_METADATA_KEY.to_owned(), metadata),
+            // If this extension type has no metadata, we make sure to
+            // clear previously set metadata.
+            None => self.metadata.remove(EXTENSION_TYPE_METADATA_KEY),
+        };
+
+        Ok(())
+    }
+
+    /// Updates the metadata of this [`Field`] with the [`ExtensionType::NAME`]
+    /// and [`ExtensionType::metadata`] of the given [`ExtensionType`].
+    ///
+    /// # Panics
+    ///
+    /// This calls [`Field::try_with_extension_type`] and panics when it
+    /// returns an error.
+    pub fn with_extension_type<E: ExtensionType>(mut self, extension_type: E) -> Self {

Review Comment:
   I suppose that makes sense!



-- 
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: github-unsubscribe@arrow.apache.org

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