Fix bug when joining cleared optional components

crates/re_query/src/archetype_view.rs

@@ -343,7 +343,13 @@ impl<A: Archetype> ArchetypeView<A> {
 
         let component = self.components.get(&C::name());
 
-        if let Some(component) = component {
+        // If the component is found and not empty, run the joining iterator on it.
+        // Otherwise just output nulls of the length of the primary.
+        // Note that this guard is specifically a precondition of the inner optimization
+        // for matched instance keys which will debug_assert if a zero-length component is
+        // referenced there.
+        let is_empty = component.map_or(true, |c| c.is_empty());
+        if let (Some(component), false) = (component, is_empty) {
             // NOTE(1): Autogenerated instance keys are interned behind datacells.
             // If two or more rows in the store share the same keys, then they will share
             // also the same cells.
@@ -356,7 +362,20 @@ impl<A: Archetype> ArchetypeView<A> {
             //   faster iterator.
             // - If the data isn't the same, the cost of the comparison will be dwarfed by the cost
             //   of the join anyway.
+
             if self.required_comp().instance_keys == component.instance_keys {
+                // This fast iterator is assumed to match the length of the
+                // primary component We shouldn't hit this since the store
+                // should enforce matched lengths for non-empty components, and
+                // the outer if-guard should keep us from reaching this in the
+                // case of an empty component.
+                // TODO(#1893):  This assert and the implementation both need to
+                // be addressed when we allow single rows containing splats.
+                debug_assert!(
+                    self.required_comp().instance_keys.num_instances()
+                        == component.values.num_instances()
+                );
+
                 // NOTE: A component instance cannot be optional in itself, and if we're on this
                 // path then we know for a fact that both batches can be intersected 1-to-1.
                 // Therefore there cannot be any null values, therefore we can go through the fast

crates/re_query/tests/archetype_visit_tests.rs

@@ -285,3 +285,86 @@ fn joint_visit() {
     assert_eq!(positions, positions_out);
     assert_eq!(expected_colors, colors_out);
 }
+
+#[test]
+fn joint_visit_with_empty() {
+    let positions = vec![
+        Position2D::new(1.0, 2.0), //
+        Position2D::new(3.0, 4.0),
+        Position2D::new(5.0, 6.0),
+        Position2D::new(7.0, 8.0),
+        Position2D::new(9.0, 10.0),
+    ];
+
+    let shared_ids = InstanceKey::from_iter(0..5);
+
+    let colors: Vec<Color> = vec![];
+
+    let positions_comp =
+        ComponentWithInstances::from_native(shared_ids.clone(), positions.clone()).unwrap();
+    let colors_comp = ComponentWithInstances::from_native(shared_ids, colors).unwrap();
+
+    let arch_view =
+        ArchetypeView::<Points2D>::from_components(RowId::ZERO, [positions_comp, colors_comp]);
+
+    let positions_out = arch_view
+        .iter_required_component::<Position2D>()
+        .unwrap()
+        .collect_vec();
+    let colors_out = arch_view
+        .iter_optional_component::<Color>()
+        .unwrap()
+        .collect_vec();
+    assert_eq!(positions_out.len(), colors_out.len());
+
+    let expected_colors = vec![None, None, None, None, None];
+
+    assert_eq!(positions, positions_out);
+    assert_eq!(expected_colors, colors_out);
+}
+
+#[test]
+fn joint_visit_with_splat() {
+    let positions = vec![
+        Position2D::new(1.0, 2.0), //
+        Position2D::new(3.0, 4.0),
+        Position2D::new(5.0, 6.0),
+        Position2D::new(7.0, 8.0),
+        Position2D::new(9.0, 10.0),
+    ];
+
+    let shared_ids = InstanceKey::from_iter(0..5);
+
+    let color = Color::from(0xff000000);
+    let colors: Vec<Color> = vec![color];
+
+    let positions_comp =
+        ComponentWithInstances::from_native(shared_ids, positions.clone()).unwrap();
+    // TODO(#1893): Replace the instance_keys with with shared_ids.
+    let colors_comp =
+        ComponentWithInstances::from_native(vec![InstanceKey::SPLAT], colors).unwrap();
+
+    let arch_view =
+        ArchetypeView::<Points2D>::from_components(RowId::ZERO, [positions_comp, colors_comp]);
+
+    let positions_out = arch_view
+        .iter_required_component::<Position2D>()
+        .unwrap()
+        .collect_vec();
+    let colors_out = arch_view
+        .iter_optional_component::<Color>()
+        .unwrap()
+        .collect_vec();
+    assert_eq!(positions_out.len(), colors_out.len());
+
+    let expected_colors = vec![
+        Some(color),
+        Some(color),
+        Some(color),
+        Some(color),
+        Some(color),
+    ];
+
+    assert_eq!(positions, positions_out);
+    assert_eq!(expected_colors, colors_out);
+}