(arrow-rs) branch main updated: Deprecate `parquet::basic::ColumnOrder::sort_order_for_type` (#10104)

[etseidl]

This is an automated email from the ASF dual-hosted git repository.

etseidl pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/arrow-rs.git


The following commit(s) were added to refs/heads/main by this push:
     new c9c3c8467f Deprecate 
`parquet::basic::ColumnOrder::sort_order_for_type` (#10104)
c9c3c8467f is described below

commit c9c3c8467f6f68f910327b33d358f6bfe589d437
Author: Ed Seidl <etse...@users.noreply.github.com>
AuthorDate: Tue Aug 4 10:55:29 2026 -0700

    Deprecate `parquet::basic::ColumnOrder::sort_order_for_type` (#10104)
    
    # Which issue does this PR close?
    
    <!--
    We generally require a GitHub issue to be filed for all bug fixes and
    enhancements and this helps us generate change logs for our releases.
    You can link an issue to this PR using the GitHub syntax.
    -->
    
    - Closes #10103.
    - Depends on #9619
    
    # Rationale for this change
    With the addition of new `ColumnOrder` variants, the public function
    `sort_order_for_type` has become unwieldy.
    
    # What changes are included in this PR?
    Add a deprecation notice to `sort_order_for_type`, and add a new
    `pub(crate)` function `get_sort_order_for_type` to replace it. The
    preferred path to obtain a `SortOrder` is now via
    `ColumnOrder::sort_order`.
    
    # Are these changes tested?
    Covered by existing tests
    
    # Are there any user-facing changes?
    Yes, this deprecates the public `sort_order_for_type`.
    
    ---------
    
    Co-authored-by: Andrew Lamb <and...@nerdnetworks.org>
---
 parquet/src/basic.rs                    | 21 ++++++++++++++++++++-
 parquet/src/file/metadata/thrift/mod.rs |  6 ++++--
 parquet/src/schema/types.rs             | 14 +-------------
 3 files changed, 25 insertions(+), 16 deletions(-)

diff --git a/parquet/src/basic.rs b/parquet/src/basic.rs
index 19b89738a0..4cbf6deb63 100644
--- a/parquet/src/basic.rs
+++ b/parquet/src/basic.rs
@@ -1045,7 +1045,7 @@ impl ColumnOrder {
             ColumnOrder::IEEE_754_TOTAL_ORDER
         } else {
             let sort_order =
-                Self::sort_order_for_type(logical_type, converted_type, 
physical_type, true);
+                Self::get_sort_order_for_type(logical_type, converted_type, 
physical_type, true);
             ColumnOrder::TYPE_DEFINED_ORDER(sort_order)
         }
     }
@@ -1054,11 +1054,30 @@ impl ColumnOrder {
     ///
     /// `is_type_defined` indicates whether the column order for this type is
     /// [`ColumnOrder::TYPE_DEFINED_ORDER`].
+    ///
+    /// It is now preferred to obtain this via [`Self::sort_order`].
+    #[deprecated(since = "60.0.0", note = "use `ColumnOrder::sort_order` 
instead")]
     pub fn sort_order_for_type(
         logical_type: Option<&LogicalType>,
         converted_type: ConvertedType,
         physical_type: Type,
         is_type_defined: bool,
+    ) -> SortOrder {
+        ColumnOrder::get_sort_order_for_type(
+            logical_type,
+            converted_type,
+            physical_type,
+            is_type_defined,
+        )
+    }
+
+    // this is pub(crate) so it can be used in the thrift parser to correctly 
instantiate
+    // the column_orders vec
+    pub(crate) fn get_sort_order_for_type(
+        logical_type: Option<&LogicalType>,
+        converted_type: ConvertedType,
+        physical_type: Type,
+        is_type_defined: bool,
     ) -> SortOrder {
         match logical_type {
             Some(logical) => match logical {
diff --git a/parquet/src/file/metadata/thrift/mod.rs 
b/parquet/src/file/metadata/thrift/mod.rs
index ce4c9719f6..bd60de4177 100644
--- a/parquet/src/file/metadata/thrift/mod.rs
+++ b/parquet/src/file/metadata/thrift/mod.rs
@@ -892,11 +892,13 @@ pub(crate) fn parquet_metadata_from_bytes(
         return Err(general_err!("Column order length mismatch"));
     }
     // replace default type defined column orders with ones having the correct 
sort order
-    // TODO(ets): this could instead be done above when decoding
     let column_orders = column_orders.map(|mut cos| {
         for (i, column) in schema_descr.columns().iter().enumerate() {
             if let ColumnOrder::TYPE_DEFINED_ORDER(_) = cos[i] {
-                let sort_order = ColumnOrder::sort_order_for_type(
+                // use `get_sort_order_for_type` so we don't replace a type 
defined sort order
+                // with a more recent ordering. we need to preserve what was 
actually in the
+                // footer.
+                let sort_order = ColumnOrder::get_sort_order_for_type(
                     column.logical_type_ref(),
                     column.converted_type(),
                     column.physical_type(),
diff --git a/parquet/src/schema/types.rs b/parquet/src/schema/types.rs
index 784c61d002..f81ff64ddc 100644
--- a/parquet/src/schema/types.rs
+++ b/parquet/src/schema/types.rs
@@ -1018,19 +1018,7 @@ impl ColumnDescriptor {
     ///
     /// Returns `SortOrder::UNDEFINED` for non-primitive types.
     pub fn sort_order(&self) -> SortOrder {
-        match self.primitive_type.as_ref() {
-            Type::PrimitiveType {
-                basic_info,
-                physical_type,
-                ..
-            } => ColumnOrder::column_order_for_type(
-                basic_info.logical_type_ref(),
-                basic_info.converted_type(),
-                *physical_type,
-            )
-            .sort_order(),
-            _ => SortOrder::UNDEFINED,
-        }
+        self.primitive_type.get_basic_info().sort_order()
     }
 }