Refactor table filtering using a FilterTrait trait (#11389)

Author: abey79

crates/viewer/re_dataframe_ui/src/datafusion_adapter.rs

@@ -11,9 +11,8 @@ use re_log_types::Timestamp;
 use re_sorbet::{BatchType, SorbetBatch};
 use re_viewer_context::AsyncRuntimeHandle;
 
-use crate::RequestedObject;
-use crate::filters::Filter;
 use crate::table_blueprint::{EntryLinksSpec, PartitionLinksSpec, SortBy, TableBlueprint};
+use crate::{ColumnFilter, RequestedObject};
 
 /// Make sure we escape column names correctly for datafusion.
 ///
@@ -38,7 +37,7 @@ struct DataFusionQueryData {
     pub partition_links: Option<PartitionLinksSpec>,
     pub entry_links: Option<EntryLinksSpec>,
     pub prefilter: Option<datafusion::prelude::Expr>,
-    pub filters: Vec<Filter>,
+    pub column_filters: Vec<ColumnFilter>,
 }
 
 impl From<&TableBlueprint> for DataFusionQueryData {
@@ -48,15 +47,15 @@ impl From<&TableBlueprint> for DataFusionQueryData {
             partition_links,
             entry_links,
             prefilter,
-            filters,
+            column_filters,
         } = value;
 
         Self {
             sort_by: sort_by.clone(),
             partition_links: partition_links.clone(),
             entry_links: entry_links.clone(),
             prefilter: prefilter.clone(),
-            filters: filters.clone(),
+            column_filters: column_filters.clone(),
         }
     }
 }
@@ -108,7 +107,7 @@ impl DataFusionQuery {
             partition_links,
             entry_links,
             prefilter,
-            filters,
+            column_filters,
         } = &self.query_data;
 
         //
@@ -162,13 +161,11 @@ impl DataFusionQuery {
         // Filters
         //
 
-        let schema = dataframe.schema();
-
-        let filter_exprs = filters
+        let filter_exprs = column_filters
             .iter()
             .filter_map(|filter| {
                 filter
-                    .as_filter_expression(schema)
+                    .as_filter_expression()
                     .inspect_err(|err| {
                         // TODO(ab): error handling will need to be improved once we introduce non-
                         // UI means of setting up filters.

crates/viewer/re_dataframe_ui/src/datafusion_table_widget.rs

@@ -23,7 +23,7 @@ use re_viewer_context::{
 
 use crate::datafusion_adapter::{DataFusionAdapter, DataFusionQueryResult};
 use crate::display_record_batch::DisplayColumn;
-use crate::filters::{Filter, FilterKind, FilterState};
+use crate::filters::{ColumnFilter, FilterState};
 use crate::header_tooltip::column_header_tooltip_ui;
 use crate::table_blueprint::{
     ColumnBlueprint, EntryLinksSpec, PartitionLinksSpec, SortBy, SortDirection, TableBlueprint,
@@ -715,8 +715,10 @@ impl egui_table::TableDelegate for DataFusionTableDelegate<'_> {
                                     // In the future, we'll probably need to be more fine-grained.
                                     #[expect(clippy::collapsible_if)]
                                     if column.blueprint.variant_ui.is_none()
-                                        && let Some(filter_op) =
-                                            FilterKind::default_for_column(column_field)
+                                        && let Some(column_filter) =
+                                            ColumnFilter::default_for_column(Arc::clone(
+                                                column_field,
+                                            ))
                                     {
                                         if ui
                                             .icon_and_text_menu_item(
@@ -725,10 +727,7 @@ impl egui_table::TableDelegate for DataFusionTableDelegate<'_> {
                                             )
                                             .clicked()
                                         {
-                                            self.filter_state.push_new_filter(Filter::new(
-                                                column_physical_name.clone(),
-                                                filter_op,
-                                            ));
+                                            self.filter_state.push_new_filter(column_filter);
                                         }
                                     }
                                 });

crates/viewer/re_dataframe_ui/src/filters/boolean.rs

@@ -6,10 +6,10 @@ use datafusion::logical_expr::{Expr, col, lit, not};
 use datafusion::prelude::{array_element, array_has, array_sort};
 use strum::VariantArray as _;
 
-use re_ui::UiExt as _;
 use re_ui::syntax_highlighting::SyntaxHighlightedBuilder;
+use re_ui::{SyntaxHighlighting, UiExt as _};
 
-use super::{FilterError, FilterUiAction};
+use super::{Filter, FilterError, FilterUiAction};
 
 /// Filter for non-nullable boolean columns.
 ///
@@ -28,20 +28,18 @@ impl NonNullableBooleanFilter {
             Self::IsFalse => false,
         }
     }
+}
 
-    pub fn as_filter_expression(
-        &self,
-        column: &Column,
-        field: &Field,
-    ) -> Result<Expr, FilterError> {
+impl Filter for NonNullableBooleanFilter {
+    fn as_filter_expression(&self, field: &Field) -> Result<Expr, FilterError> {
         match field.data_type() {
-            DataType::Boolean => Ok(col(column.clone()).eq(lit(self.as_bool()))),
+            DataType::Boolean => Ok(col(field.name().clone()).eq(lit(self.as_bool()))),
 
             DataType::List(field) | DataType::ListView(field)
                 if field.data_type() == &DataType::Boolean =>
             {
                 // `ANY` semantics
-                Ok(array_has(col(column.clone()), lit(self.as_bool())))
+                Ok(array_has(col(field.name().clone()), lit(self.as_bool())))
             }
 
             _ => Err(FilterError::InvalidNonNullableBooleanFilter(
@@ -51,11 +49,13 @@ impl NonNullableBooleanFilter {
         }
     }
 
-    pub fn operand_text(&self) -> String {
-        self.as_bool().to_string()
-    }
-
-    pub fn popup_ui(&mut self, ui: &mut egui::Ui, column_name: &str) -> FilterUiAction {
+    fn popup_ui(
+        &mut self,
+        ui: &mut egui::Ui,
+        _timestamp_format: re_log_types::TimestampFormat,
+        column_name: &str,
+        _popup_just_opened: bool,
+    ) -> FilterUiAction {
         ui.label(
             SyntaxHighlightedBuilder::body_default(column_name)
                 .with_keyword(" is")
@@ -80,6 +80,13 @@ impl NonNullableBooleanFilter {
     }
 }
 
+impl SyntaxHighlighting for NonNullableBooleanFilter {
+    fn syntax_highlight_into(&self, builder: &mut SyntaxHighlightedBuilder) {
+        builder.append_keyword("is ");
+        builder.append_primitive(&self.as_bool().to_string());
+    }
+}
+
 #[derive(Debug, Clone, Copy, Default, PartialEq, Eq, strum::VariantArray)]
 #[expect(clippy::enum_variant_names)]
 pub enum NullableBooleanValue {
@@ -163,17 +170,25 @@ impl NullableBooleanFilter {
         self
     }
 
-    pub fn as_filter_expression(
-        &self,
-        column: &Column,
-        field: &Field,
-    ) -> Result<Expr, FilterError> {
+    pub fn operand_text(&self) -> String {
+        if let Some(value) = self.value.as_bool() {
+            value.to_string()
+        } else {
+            "null".to_owned()
+        }
+    }
+}
+
+impl Filter for NullableBooleanFilter {
+    fn as_filter_expression(&self, field: &Field) -> Result<Expr, FilterError> {
+        let column = Column::from(field.name().clone());
+
         let expr = match field.data_type() {
             DataType::Boolean => {
                 if let Some(value) = self.value.as_bool() {
-                    col(column.clone()).eq(lit(value))
+                    col(column).eq(lit(value))
                 } else {
-                    col(column.clone()).is_null()
+                    col(column).is_null()
                 }
             }
 
@@ -182,10 +197,10 @@ impl NullableBooleanFilter {
             {
                 // `ANY` semantics
                 if let Some(value) = self.value.as_bool() {
-                    array_has(col(column.clone()), lit(value))
+                    array_has(col(column), lit(value))
                 } else {
                     col(column.clone()).is_null().or(array_element(
-                        array_sort(col(column.clone()), lit("ASC"), lit("NULLS FIRST")),
+                        array_sort(col(column), lit("ASC"), lit("NULLS FIRST")),
                         lit(1),
                     )
                     .is_null())
@@ -206,19 +221,13 @@ impl NullableBooleanFilter {
         }
     }
 
-    pub fn operand_text(&self) -> String {
-        if let Some(value) = self.value.as_bool() {
-            value.to_string()
-        } else {
-            "null".to_owned()
-        }
-    }
-
-    pub fn operator(&self) -> NullableBooleanOperator {
-        self.operator
-    }
-
-    pub fn popup_ui(&mut self, ui: &mut egui::Ui, column_name: &str) -> FilterUiAction {
+    fn popup_ui(
+        &mut self,
+        ui: &mut egui::Ui,
+        _timestamp_format: re_log_types::TimestampFormat,
+        column_name: &str,
+        _popup_just_opened: bool,
+    ) -> FilterUiAction {
         ui.horizontal(|ui| {
             ui.label(
                 SyntaxHighlightedBuilder::body_default(column_name).into_widget_text(ui.style()),
@@ -276,6 +285,14 @@ impl NullableBooleanFilter {
     }
 }
 
+impl SyntaxHighlighting for NullableBooleanFilter {
+    fn syntax_highlight_into(&self, builder: &mut SyntaxHighlightedBuilder) {
+        builder.append_keyword(&self.operator.to_string());
+        builder.append_keyword(" ");
+        builder.append_primitive(&self.operand_text());
+    }
+}
+
 fn primitive_widget_text(ui: &egui::Ui, s: &str) -> egui::WidgetText {
     SyntaxHighlightedBuilder::primitive(s).into_widget_text(ui.style())
 }

crates/viewer/re_dataframe_ui/src/filters/column_filter.rs

@@ -0,0 +1,100 @@
+use std::fmt::Formatter;
+
+use arrow::datatypes::{DataType, Field, FieldRef};
+use datafusion::prelude::Expr;
+
+use super::{Filter as _, FilterError, TypedFilter};
+
+/// The nullability of a nested arrow datatype.
+#[derive(Clone, Copy, PartialEq, Eq)]
+pub struct Nullability {
+    /// The inner datatype is nullable (e.g. for a list array, one row's array may contain nulls).
+    pub inner: bool,
+
+    /// The outer datatype is nullable (e.g, the a list array, one row may have a null instead of an
+    /// array).
+    pub outer: bool,
+}
+
+// for test snapshot naming
+impl std::fmt::Debug for Nullability {
+    fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
+        match (self.inner, self.outer) {
+            (false, false) => write!(f, "no_null"),
+            (false, true) => write!(f, "outer_null"),
+            (true, false) => write!(f, "inner_null"),
+            (true, true) => write!(f, "both_null"),
+        }
+    }
+}
+
+impl Nullability {
+    pub const NONE: Self = Self {
+        inner: false,
+        outer: false,
+    };
+
+    pub const BOTH: Self = Self {
+        inner: true,
+        outer: true,
+    };
+
+    pub const INNER: Self = Self {
+        inner: true,
+        outer: false,
+    };
+
+    pub const OUTER: Self = Self {
+        inner: false,
+        outer: true,
+    };
+
+    pub const ALL: &'static [Self] = &[Self::NONE, Self::INNER, Self::OUTER, Self::BOTH];
+
+    pub fn from_field(field: &Field) -> Self {
+        match field.data_type() {
+            DataType::List(inner_field) | DataType::ListView(inner_field) => Self {
+                inner: inner_field.is_nullable(),
+                outer: field.is_nullable(),
+            },
+
+            //TODO(ab): support other containers
+            _ => Self {
+                inner: field.is_nullable(),
+                outer: false,
+            },
+        }
+    }
+
+    pub fn is_either(&self) -> bool {
+        self.inner || self.outer
+    }
+}
+
+/// A filter applied to a table's column.
+#[derive(Debug, Clone, PartialEq)]
+pub struct ColumnFilter {
+    pub field: FieldRef,
+    pub filter: TypedFilter,
+}
+
+impl ColumnFilter {
+    pub fn new(field: FieldRef, filter: impl Into<TypedFilter>) -> Self {
+        Self {
+            field,
+            filter: filter.into(),
+        }
+    }
+
+    pub fn default_for_column(field: FieldRef) -> Option<Self> {
+        let filter = TypedFilter::default_for_column(&field)?;
+        Some(Self::new(field, filter))
+    }
+
+    /// Convert to an [`Expr`].
+    ///
+    /// The expression is used for filtering and should thus evaluate to a boolean.
+    pub fn as_filter_expression(&self) -> Result<Expr, FilterError> {
+        self.filter.as_filter_expression(&self.field)
+    }
+}