test: cover LikeExpr proto hooks; add shared proto test stubs

Add direct unit tests for `LikeExpr::try_to_proto` / `try_from_proto`
covering the happy path and every error branch (wrong node, missing
`expr`/`pattern`, and child encode/decode failures), reaching 100% line
and region coverage of the new proto hook code.

Extract the generic encode/decode test stubs into a cfg-guarded
`proto_test_util` module so the other `PhysicalExpr` migrations in this
crate (issue #22418) can reuse them; LikeExpr keeps only its
expression-specific node builder and fixture.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Author: adriangb

datafusion/physical-expr/src/expressions/like.rs

@@ -346,3 +346,192 @@ mod test {
         Ok(())
     }
 }
+
+/// Tests for the `try_to_proto` / `try_from_proto` hooks (issue #22418).
+///
+/// These drive the encode/decode hooks directly with small stub
+/// encoder/decoders so every branch can be covered without depending on
+/// `datafusion-proto`: the happy path, the wrong-node and missing-child
+/// rejections, and child encode/decode error propagation.
+#[cfg(all(test, feature = "proto"))]
+mod proto_tests {
+    use super::*;
+    use crate::expressions::{Column, col};
+    use crate::proto_test_util::{
+        StubDecoder, StubEncoder, UnreachableDecoder, column_node,
+    };
+    use arrow::datatypes::Field;
+    use datafusion_common::DataFusionError;
+    use datafusion_physical_expr_common::physical_expr::proto_decode::PhysicalExprDecodeCtx;
+    use datafusion_physical_expr_common::physical_expr::proto_encode::PhysicalExprEncodeCtx;
+    use datafusion_proto_models::protobuf::{
+        PhysicalExprNode, PhysicalLikeExprNode, physical_expr_node,
+    };
+
+    /// Build a `LikeExpr` proto node with the given children.
+    fn like_node(
+        negated: bool,
+        case_insensitive: bool,
+        expr: Option<Box<PhysicalExprNode>>,
+        pattern: Option<Box<PhysicalExprNode>>,
+    ) -> PhysicalExprNode {
+        PhysicalExprNode {
+            expr_id: None,
+            expr_type: Some(physical_expr_node::ExprType::LikeExpr(Box::new(
+                PhysicalLikeExprNode {
+                    negated,
+                    case_insensitive,
+                    expr,
+                    pattern,
+                },
+            ))),
+        }
+    }
+
+    /// A `LikeExpr` over two `Utf8` columns with both flags set, so the
+    /// `negated` / `case_insensitive` wiring is actually exercised.
+    fn like_fixture() -> LikeExpr {
+        let schema = Schema::new(vec![
+            Field::new("a", DataType::Utf8, false),
+            Field::new("b", DataType::Utf8, false),
+        ]);
+        LikeExpr::new(
+            true,
+            true,
+            col("a", &schema).unwrap(),
+            col("b", &schema).unwrap(),
+        )
+    }
+
+    #[test]
+    fn try_to_proto_encodes_like_expr() {
+        let like = like_fixture();
+        let encoder = StubEncoder::ok();
+        let ctx = PhysicalExprEncodeCtx::new(&encoder);
+
+        let node = like
+            .try_to_proto(&ctx)
+            .unwrap()
+            .expect("LikeExpr should encode to Some(node)");
+
+        // Built-in exprs never set expr_id; only dynamic filters do.
+        assert!(node.expr_id.is_none());
+        let like_node = match node.expr_type {
+            Some(physical_expr_node::ExprType::LikeExpr(boxed)) => *boxed,
+            other => panic!("expected a LikeExpr node, got {other:?}"),
+        };
+        assert!(like_node.negated);
+        assert!(like_node.case_insensitive);
+        assert!(like_node.expr.is_some());
+        assert!(like_node.pattern.is_some());
+    }
+
+    #[test]
+    fn try_to_proto_propagates_expr_encode_error() {
+        let like = like_fixture();
+        let encoder = StubEncoder::failing_on(1);
+        let ctx = PhysicalExprEncodeCtx::new(&encoder);
+        let err = like.try_to_proto(&ctx).unwrap_err();
+        assert!(matches!(err, DataFusionError::Internal(msg) if msg.contains("call 1")));
+    }
+
+    #[test]
+    fn try_to_proto_propagates_pattern_encode_error() {
+        let like = like_fixture();
+        let encoder = StubEncoder::failing_on(2);
+        let ctx = PhysicalExprEncodeCtx::new(&encoder);
+        let err = like.try_to_proto(&ctx).unwrap_err();
+        assert!(matches!(err, DataFusionError::Internal(msg) if msg.contains("call 2")));
+    }
+
+    #[test]
+    fn try_from_proto_decodes_like_expr() {
+        let node = like_node(
+            true,
+            true,
+            Some(Box::new(column_node("a"))),
+            Some(Box::new(column_node("b"))),
+        );
+        let schema = Schema::empty();
+        let decoder = StubDecoder::ok();
+        let ctx = PhysicalExprDecodeCtx::new(&schema, &decoder);
+
+        let decoded = LikeExpr::try_from_proto(&node, &ctx).unwrap();
+        let like = decoded
+            .downcast_ref::<LikeExpr>()
+            .expect("decoded expr should be a LikeExpr");
+        assert!(like.negated());
+        assert!(like.case_insensitive());
+        assert!(like.expr().downcast_ref::<Column>().is_some());
+        assert!(like.pattern().downcast_ref::<Column>().is_some());
+    }
+
+    #[test]
+    fn try_from_proto_rejects_non_like_node() {
+        let node = column_node("a");
+        let schema = Schema::empty();
+        let decoder = UnreachableDecoder;
+        let ctx = PhysicalExprDecodeCtx::new(&schema, &decoder);
+        let err = LikeExpr::try_from_proto(&node, &ctx).unwrap_err();
+        assert!(matches!(
+            err,
+            DataFusionError::Internal(msg) if msg == "PhysicalExprNode is not a LikeExpr"
+        ));
+    }
+
+    #[test]
+    fn try_from_proto_rejects_missing_expr() {
+        let node = like_node(false, false, None, Some(Box::new(column_node("b"))));
+        let schema = Schema::empty();
+        let decoder = UnreachableDecoder;
+        let ctx = PhysicalExprDecodeCtx::new(&schema, &decoder);
+        let err = LikeExpr::try_from_proto(&node, &ctx).unwrap_err();
+        assert!(matches!(
+            err,
+            DataFusionError::Internal(msg) if msg == "LikeExpr is missing required field 'expr'"
+        ));
+    }
+
+    #[test]
+    fn try_from_proto_rejects_missing_pattern() {
+        let node = like_node(false, false, Some(Box::new(column_node("a"))), None);
+        let schema = Schema::empty();
+        let decoder = UnreachableDecoder;
+        let ctx = PhysicalExprDecodeCtx::new(&schema, &decoder);
+        let err = LikeExpr::try_from_proto(&node, &ctx).unwrap_err();
+        assert!(matches!(
+            err,
+            DataFusionError::Internal(msg) if msg == "LikeExpr is missing required field 'pattern'"
+        ));
+    }
+
+    #[test]
+    fn try_from_proto_propagates_expr_decode_error() {
+        let node = like_node(
+            false,
+            false,
+            Some(Box::new(column_node("a"))),
+            Some(Box::new(column_node("b"))),
+        );
+        let schema = Schema::empty();
+        let decoder = StubDecoder::failing_on(1);
+        let ctx = PhysicalExprDecodeCtx::new(&schema, &decoder);
+        let err = LikeExpr::try_from_proto(&node, &ctx).unwrap_err();
+        assert!(matches!(err, DataFusionError::Internal(msg) if msg.contains("call 1")));
+    }
+
+    #[test]
+    fn try_from_proto_propagates_pattern_decode_error() {
+        let node = like_node(
+            false,
+            false,
+            Some(Box::new(column_node("a"))),
+            Some(Box::new(column_node("b"))),
+        );
+        let schema = Schema::empty();
+        let decoder = StubDecoder::failing_on(2);
+        let ctx = PhysicalExprDecodeCtx::new(&schema, &decoder);
+        let err = LikeExpr::try_from_proto(&node, &ctx).unwrap_err();
+        assert!(matches!(err, DataFusionError::Internal(msg) if msg.contains("call 2")));
+    }
+}

datafusion/physical-expr/src/lib.rs

@@ -40,6 +40,10 @@ mod partitioning;
 mod physical_expr;
 pub mod planner;
 pub mod projection;
+/// Shared test helpers for the `try_to_proto` / `try_from_proto` migration
+/// (issue #22418). Compiled only under `cfg(test)` with the `proto` feature.
+#[cfg(all(test, feature = "proto"))]
+pub(crate) mod proto_test_util;
 mod scalar_function;
 pub mod scalar_subquery;
 pub mod simplifier;

datafusion/physical-expr/src/proto_test_util.rs

@@ -0,0 +1,148 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+//! Shared test helpers for exercising the `try_to_proto` / `try_from_proto`
+//! hooks (issue #22418) without depending on `datafusion-proto`.
+//!
+//! As built-in expressions are ported off the central proto downcast chain,
+//! each one's unit tests need to drive the new encode/decode hooks in
+//! isolation. The generic scaffolding — stub encoder/decoders and a
+//! placeholder child-node builder — lives here so it is written once and
+//! reused across expressions. Expression-specific node builders and fixtures
+//! stay in each expression's own test module.
+
+use std::cell::Cell;
+use std::sync::Arc;
+
+use arrow::datatypes::Schema;
+use datafusion_common::{DataFusionError, Result};
+use datafusion_physical_expr_common::physical_expr::PhysicalExpr;
+use datafusion_physical_expr_common::physical_expr::proto_decode::PhysicalExprDecode;
+use datafusion_physical_expr_common::physical_expr::proto_encode::PhysicalExprEncode;
+use datafusion_proto_models::protobuf::{self, PhysicalExprNode, physical_expr_node};
+
+use crate::expressions::Column;
+
+/// A proto node for a `Column`, useful as a stand-in child node when building
+/// an expression's proto representation in tests.
+pub(crate) fn column_node(name: &str) -> PhysicalExprNode {
+    PhysicalExprNode {
+        expr_id: None,
+        expr_type: Some(physical_expr_node::ExprType::Column(
+            protobuf::PhysicalColumn {
+                name: name.to_string(),
+                index: 0,
+            },
+        )),
+    }
+}
+
+/// Decoder stub for driving `try_from_proto`: returns a fixed `Column` for each
+/// child node, optionally failing on the Nth `decode` call so the
+/// `ctx.decode(..)?` error arms can be exercised.
+pub(crate) struct StubDecoder {
+    fail_on_call: Option<usize>,
+    calls: Cell<usize>,
+}
+
+impl StubDecoder {
+    /// Always succeeds, returning a placeholder `Column` per child.
+    pub(crate) fn ok() -> Self {
+        Self {
+            fail_on_call: None,
+            calls: Cell::new(0),
+        }
+    }
+
+    /// Fails on the `call`-th invocation (1-based), succeeding otherwise.
+    pub(crate) fn failing_on(call: usize) -> Self {
+        Self {
+            fail_on_call: Some(call),
+            calls: Cell::new(0),
+        }
+    }
+}
+
+impl PhysicalExprDecode for StubDecoder {
+    fn decode(
+        &self,
+        _node: &PhysicalExprNode,
+        _schema: &Schema,
+    ) -> Result<Arc<dyn PhysicalExpr>> {
+        let call = self.calls.get() + 1;
+        self.calls.set(call);
+        if Some(call) == self.fail_on_call {
+            return Err(DataFusionError::Internal(format!(
+                "stub decode failure on call {call}"
+            )));
+        }
+        Ok(Arc::new(Column::new("decoded", 0)))
+    }
+}
+
+/// Decoder that must never run: used to assert that the reject paths of a
+/// `try_from_proto` (wrong node, missing child) bail out before decoding.
+pub(crate) struct UnreachableDecoder;
+
+impl PhysicalExprDecode for UnreachableDecoder {
+    fn decode(
+        &self,
+        _node: &PhysicalExprNode,
+        _schema: &Schema,
+    ) -> Result<Arc<dyn PhysicalExpr>> {
+        unreachable!("decode must not be reached when the node is rejected")
+    }
+}
+
+/// Encoder stub for driving `try_to_proto`: emits a placeholder `Column` node
+/// for each child, optionally failing on the Nth `encode` call so the
+/// `ctx.encode_child(..)?` error arms can be exercised.
+pub(crate) struct StubEncoder {
+    fail_on_call: Option<usize>,
+    calls: Cell<usize>,
+}
+
+impl StubEncoder {
+    /// Always succeeds, emitting a placeholder `Column` node per child.
+    pub(crate) fn ok() -> Self {
+        Self {
+            fail_on_call: None,
+            calls: Cell::new(0),
+        }
+    }
+
+    /// Fails on the `call`-th invocation (1-based), succeeding otherwise.
+    pub(crate) fn failing_on(call: usize) -> Self {
+        Self {
+            fail_on_call: Some(call),
+            calls: Cell::new(0),
+        }
+    }
+}
+
+impl PhysicalExprEncode for StubEncoder {
+    fn encode(&self, _expr: &Arc<dyn PhysicalExpr>) -> Result<PhysicalExprNode> {
+        let call = self.calls.get() + 1;
+        self.calls.set(call);
+        if Some(call) == self.fail_on_call {
+            return Err(DataFusionError::Internal(format!(
+                "stub encode failure on call {call}"
+            )));
+        }
+        Ok(column_node("child"))
+    }
+}