From 61e99f21b23bf1a35f63c8c4bad9a3bb10d3ae29 Mon Sep 17 00:00:00 2001
From: Emil Ernerfeldt <emil.ernerfeldt@gmail.com>
Date: Sat, 11 Jan 2025 17:52:05 +0100
Subject: [PATCH] Remove unnecessary `TimeColumnDescriptor::datatype` field

---
 crates/store/re_chunk_store/src/dataframe.rs | 55 ++++++--------------
 crates/store/re_dataframe/src/query.rs       |  1 -
 2 files changed, 16 insertions(+), 40 deletions(-)

diff --git a/crates/store/re_chunk_store/src/dataframe.rs b/crates/store/re_chunk_store/src/dataframe.rs
index 0297f77225edc..6413b82e30ac1 100644
--- a/crates/store/re_chunk_store/src/dataframe.rs
+++ b/crates/store/re_chunk_store/src/dataframe.rs
@@ -4,6 +4,7 @@ use std::collections::{BTreeMap, BTreeSet};
 use std::ops::Deref;
 use std::ops::DerefMut;
 
+use arrow::datatypes::DataType as ArrowDatatype;
 use arrow2::{
     array::ListArray as ArrowListArray,
     datatypes::{DataType as Arrow2Datatype, Field as Arrow2Field},
@@ -45,7 +46,7 @@ impl ColumnDescriptor {
     #[inline]
     pub fn datatype(&self) -> Arrow2Datatype {
         match self {
-            Self::Time(descr) => descr.datatype.clone(),
+            Self::Time(descr) => descr.datatype().into(),
             Self::Component(descr) => descr.returned_datatype(),
         }
     }
@@ -76,39 +77,15 @@ impl ColumnDescriptor {
 }
 
 /// Describes a time column, such as `log_time`.
-#[derive(Debug, Clone, PartialEq, Eq, Hash)]
+#[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)]
 pub struct TimeColumnDescriptor {
     /// The timeline this column is associated with.
     pub timeline: Timeline,
-
-    /// The Arrow datatype of the column.
-    pub datatype: Arrow2Datatype,
-}
-
-impl PartialOrd for TimeColumnDescriptor {
-    #[inline]
-    fn partial_cmp(&self, other: &Self) -> Option<std::cmp::Ordering> {
-        Some(self.cmp(other))
-    }
-}
-
-impl Ord for TimeColumnDescriptor {
-    #[inline]
-    fn cmp(&self, other: &Self) -> std::cmp::Ordering {
-        let Self {
-            timeline,
-            datatype: _,
-        } = self;
-        timeline.cmp(&other.timeline)
-    }
 }
 
 impl TimeColumnDescriptor {
     fn metadata(&self) -> arrow2::datatypes::Metadata {
-        let Self {
-            timeline,
-            datatype: _,
-        } = self;
+        let Self { timeline } = self;
 
         std::iter::once(Some((
             "sorbet.index_name".to_owned(),
@@ -118,13 +95,18 @@ impl TimeColumnDescriptor {
         .collect()
     }
 
+    #[inline]
+    pub fn datatype(&self) -> ArrowDatatype {
+        self.timeline.datatype()
+    }
+
     #[inline]
     // Time column must be nullable since static data doesn't have a time.
     pub fn to_arrow_field(&self) -> Arrow2Field {
-        let Self { timeline, datatype } = self;
+        let Self { timeline } = self;
         Arrow2Field::new(
             timeline.name().to_string(),
-            datatype.clone(),
+            timeline.datatype().into(),
             true, /* nullable */
         )
         .with_metadata(self.metadata())
@@ -705,12 +687,10 @@ impl ChunkStore {
     pub fn schema(&self) -> Vec<ColumnDescriptor> {
         re_tracing::profile_function!();
 
-        let timelines = self.all_timelines_sorted().into_iter().map(|timeline| {
-            ColumnDescriptor::Time(TimeColumnDescriptor {
-                timeline,
-                datatype: timeline.datatype().into(),
-            })
-        });
+        let timelines = self
+            .all_timelines_sorted()
+            .into_iter()
+            .map(|timeline| ColumnDescriptor::Time(TimeColumnDescriptor { timeline }));
 
         let mut components = self
             .per_column_metadata
@@ -780,10 +760,7 @@ impl ChunkStore {
             .copied()
             .unwrap_or_else(|| Timeline::new_temporal(selector.timeline));
 
-        TimeColumnDescriptor {
-            timeline,
-            datatype: timeline.datatype().into(),
-        }
+        TimeColumnDescriptor { timeline }
     }
 
     /// Given a [`ComponentColumnSelector`], returns the corresponding [`ComponentColumnDescriptor`].
diff --git a/crates/store/re_dataframe/src/query.rs b/crates/store/re_dataframe/src/query.rs
index 92481b4c3958e..dd71855c72644 100644
--- a/crates/store/re_dataframe/src/query.rs
+++ b/crates/store/re_dataframe/src/query.rs
@@ -398,7 +398,6 @@ impl<E: StorageEngineLike> QueryHandle<E> {
                                             // TODO(cmc): I picked a sequence here because I have to pick something.
                                             // It doesn't matter, only the name will remain in the Arrow schema anyhow.
                                             timeline: Timeline::new_sequence(*selected_timeline),
-                                            datatype: arrow2::datatypes::DataType::Null,
                                         }),
                                     )
                                 },