fix: coerce nested regexp_match to boolean in filters (#5019)

When combining `regexp_match(col, ...)` with boolean predicates (e.g.,
`AND natural_caption IS NOT NULL AND natural_caption <> '' ...`),
planning failed with:
"Cannot infer common argument type for logical boolean operation
List(Utf8) AND Boolean"
Root cause:
`regexp_match` returns a list, not a boolean.
Our coercion only handled top-level `regexp_match`, not occurrences
nested inside boolean expressions.

Signed-off-by: BubbleCal <bubble-cal@outlook.com>

Author: BubbleCal

rust/lance-datafusion/src/logical_expr.rs

@@ -198,17 +198,24 @@ pub fn coerce_expr(expr: &Expr, dtype: &DataType) -> Result<Expr> {
 ///
 /// - *expr*: a datafusion logical expression
 pub fn coerce_filter_type_to_boolean(expr: Expr) -> Expr {
-    match &expr {
-        // TODO: consider making this dispatch more generic, i.e. fun.output_type -> coerce
-        // instead of hardcoding coerce method for each function
-        Expr::ScalarFunction(ScalarFunction { func, .. }) => {
-            if func.name() == "regexp_match" {
-                Expr::IsNotNull(Box::new(expr))
-            } else {
-                expr
-            }
+    match expr {
+        // Coerce regexp_match to boolean by checking for non-null
+        Expr::ScalarFunction(sf) if sf.func.name() == "regexp_match" => {
+            Expr::IsNotNull(Box::new(Expr::ScalarFunction(sf)))
         }
-        _ => expr,
+
+        // Recurse into boolean contexts so nested regexp_match terms are also coerced
+        Expr::BinaryExpr(BinaryExpr { left, op, right }) => Expr::BinaryExpr(BinaryExpr {
+            left: Box::new(coerce_filter_type_to_boolean(*left)),
+            op,
+            right: Box::new(coerce_filter_type_to_boolean(*right)),
+        }),
+        Expr::Not(inner) => Expr::Not(Box::new(coerce_filter_type_to_boolean(*inner))),
+        Expr::IsNull(inner) => Expr::IsNull(Box::new(coerce_filter_type_to_boolean(*inner))),
+        Expr::IsNotNull(inner) => Expr::IsNotNull(Box::new(coerce_filter_type_to_boolean(*inner))),
+
+        // Pass-through for all other nodes
+        other => other,
     }
 }
 

rust/lance-datafusion/src/planner.rs

@@ -1740,4 +1740,88 @@ mod tests {
         let ctx_provider = LanceContextProvider::default();
         assert!(!ctx_provider.get_expr_planners().is_empty());
     }
+
+    #[test]
+    fn test_regexp_match_and_non_empty_captions() {
+        // Repro for a bug where regexp_match inside an AND chain wasn't coerced to boolean,
+        // causing planning/evaluation failures. This should evaluate successfully.
+        let schema = Arc::new(Schema::new(vec![
+            Field::new("keywords", DataType::Utf8, true),
+            Field::new("natural_caption", DataType::Utf8, true),
+            Field::new("poetic_caption", DataType::Utf8, true),
+        ]));
+
+        let planner = Planner::new(schema.clone());
+
+        let expr = planner
+            .parse_filter(
+                "regexp_match(keywords, 'Liberty|revolution') AND \
+                 (natural_caption IS NOT NULL AND natural_caption <> '' AND \
+                  poetic_caption IS NOT NULL AND poetic_caption <> '')",
+            )
+            .unwrap();
+
+        let physical_expr = planner.create_physical_expr(&expr).unwrap();
+
+        let batch = RecordBatch::try_new(
+            schema,
+            vec![
+                Arc::new(StringArray::from(vec![
+                    Some("Liberty for all"),
+                    Some("peace"),
+                    Some("revolution now"),
+                    Some("Liberty"),
+                    Some("revolutionary"),
+                    Some("none"),
+                ])) as ArrayRef,
+                Arc::new(StringArray::from(vec![
+                    Some("a"),
+                    Some("b"),
+                    None,
+                    Some(""),
+                    Some("c"),
+                    Some("d"),
+                ])) as ArrayRef,
+                Arc::new(StringArray::from(vec![
+                    Some("x"),
+                    Some(""),
+                    Some("y"),
+                    Some("z"),
+                    None,
+                    Some("w"),
+                ])) as ArrayRef,
+            ],
+        )
+        .unwrap();
+
+        let result = physical_expr.evaluate(&batch).unwrap();
+        assert_eq!(
+            result.into_array(0).unwrap().as_ref(),
+            &BooleanArray::from(vec![true, false, false, false, false, false])
+        );
+    }
+
+    #[test]
+    fn test_regexp_match_infer_error_without_boolean_coercion() {
+        // With the fix applied, using parse_filter should coerce regexp_match to boolean
+        // even when nested in a larger AND expression, so this should plan successfully.
+        let schema = Arc::new(Schema::new(vec![
+            Field::new("keywords", DataType::Utf8, true),
+            Field::new("natural_caption", DataType::Utf8, true),
+            Field::new("poetic_caption", DataType::Utf8, true),
+        ]));
+
+        let planner = Planner::new(schema);
+
+        let expr = planner
+            .parse_filter(
+                "regexp_match(keywords, 'Liberty|revolution') AND \
+                 (natural_caption IS NOT NULL AND natural_caption <> '' AND \
+                  poetic_caption IS NOT NULL AND poetic_caption <> '')",
+            )
+            .unwrap();
+
+        // Should not panic
+        let _physical = planner.create_physical_expr(&expr).unwrap();
+    }
 }

rust/lance/src/dataset/scanner.rs

@@ -4127,6 +4127,82 @@ mod test {
         Ok(())
     }
 
+    #[tokio::test]
+    async fn test_scan_regexp_match_and_non_empty_captions() {
+        // Build a small dataset with three Utf8 columns and verify the full
+        // scan().filter(...) path handles regexp_match combined with non-null/non-empty checks.
+        let schema = Arc::new(ArrowSchema::new(vec![
+            ArrowField::new("keywords", DataType::Utf8, true),
+            ArrowField::new("natural_caption", DataType::Utf8, true),
+            ArrowField::new("poetic_caption", DataType::Utf8, true),
+        ]));
+
+        let batch = RecordBatch::try_new(
+            schema.clone(),
+            vec![
+                Arc::new(StringArray::from(vec![
+                    Some("Liberty for all"),
+                    Some("peace"),
+                    Some("revolution now"),
+                    Some("Liberty"),
+                    Some("revolutionary"),
+                    Some("none"),
+                ])) as ArrayRef,
+                Arc::new(StringArray::from(vec![
+                    Some("a"),
+                    Some("b"),
+                    None,
+                    Some(""),
+                    Some("c"),
+                    Some("d"),
+                ])) as ArrayRef,
+                Arc::new(StringArray::from(vec![
+                    Some("x"),
+                    Some(""),
+                    Some("y"),
+                    Some("z"),
+                    None,
+                    Some("w"),
+                ])) as ArrayRef,
+            ],
+        )
+        .unwrap();
+
+        let reader = RecordBatchIterator::new(vec![Ok(batch.clone())], schema.clone());
+        let dataset = Dataset::write(reader, "memory://", None).await.unwrap();
+
+        let mut scan = dataset.scan();
+        scan.filter(
+            "regexp_match(keywords, 'Liberty|revolution') AND \
+             (natural_caption IS NOT NULL AND natural_caption <> '' AND \
+              poetic_caption IS NOT NULL AND poetic_caption <> '')",
+        )
+        .unwrap();
+
+        let out = scan.try_into_batch().await.unwrap();
+        assert_eq!(out.num_rows(), 1);
+
+        let out_keywords = out
+            .column_by_name("keywords")
+            .unwrap()
+            .as_string::<i32>()
+            .value(0);
+        let out_nat = out
+            .column_by_name("natural_caption")
+            .unwrap()
+            .as_string::<i32>()
+            .value(0);
+        let out_poetic = out
+            .column_by_name("poetic_caption")
+            .unwrap()
+            .as_string::<i32>()
+            .value(0);
+
+        assert_eq!(out_keywords, "Liberty for all");
+        assert_eq!(out_nat, "a");
+        assert_eq!(out_poetic, "x");
+    }
+
     #[tokio::test]
     async fn test_nested_projection() {
         let point_fields: Fields = vec![