Skip to content

Commit

Permalink
Remove unnecessary TimeColumnDescriptor::datatype field
Browse files Browse the repository at this point in the history
  • Loading branch information
emilk committed Jan 11, 2025
1 parent 910269c commit 61e99f2
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 40 deletions.
55 changes: 16 additions & 39 deletions crates/store/re_chunk_store/src/dataframe.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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},
Expand Down Expand Up @@ -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(),
}
}
Expand Down Expand Up @@ -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(),
Expand All @@ -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())
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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`].
Expand Down
1 change: 0 additions & 1 deletion crates/store/re_dataframe/src/query.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}),
)
},
Expand Down

0 comments on commit 61e99f2

Please sign in to comment.