From 95084fda96c410ff0be14f23c6e4c29c52e9a31f Mon Sep 17 00:00:00 2001
From: Bugen Zhao <i@bugenzhao.com>
Date: Tue, 17 Oct 2023 21:06:59 +0800
Subject: [PATCH] refine todo behavior

Signed-off-by: Bugen Zhao <i@bugenzhao.com>
---
 src/expr/core/src/expr/mod.rs                | 11 +++++++++--
 src/expr/core/src/expr/wrapper/mod.rs        |  2 +-
 src/expr/core/src/expr/wrapper/non_strict.rs | 10 ++++++++++
 3 files changed, 20 insertions(+), 3 deletions(-)

diff --git a/src/expr/core/src/expr/mod.rs b/src/expr/core/src/expr/mod.rs
index 7b65eb1fe8144..1b23bb85fd404 100644
--- a/src/expr/core/src/expr/mod.rs
+++ b/src/expr/core/src/expr/mod.rs
@@ -123,6 +123,9 @@ impl<E> NonStrictExpression<E>
 where
     E: Expression,
 {
+    /// Create a non-strict expression directly wrapping the given expression.
+    ///
+    /// Should only be used in tests as evaluation may panic.
     pub fn for_test(inner: E) -> NonStrictExpression
     where
         E: 'static,
@@ -130,8 +133,12 @@ where
         NonStrictExpression(inner.boxed())
     }
 
-    pub fn todo(inner: E) -> Self {
-        Self(inner)
+    /// Create a non-strict expression from the given expression, where only the evaluation of the
+    /// top-level expression is non-strict (which is subtly different from
+    /// [`crate::expr::build_non_strict_from_prost`]), and error will only be simply logged.
+    pub fn todo(inner: E) -> NonStrictExpression<impl Expression> {
+        let inner = wrapper::non_strict::NonStrict::new(inner, wrapper::LogReport);
+        NonStrictExpression(inner)
     }
 
     /// Get the return data type.
diff --git a/src/expr/core/src/expr/wrapper/mod.rs b/src/expr/core/src/expr/wrapper/mod.rs
index c93da021c882a..16988a050ad8d 100644
--- a/src/expr/core/src/expr/wrapper/mod.rs
+++ b/src/expr/core/src/expr/wrapper/mod.rs
@@ -15,4 +15,4 @@
 pub(crate) mod checked;
 pub(crate) mod non_strict;
 
-pub use non_strict::EvalErrorReport;
+pub use non_strict::{EvalErrorReport, LogReport};
diff --git a/src/expr/core/src/expr/wrapper/non_strict.rs b/src/expr/core/src/expr/wrapper/non_strict.rs
index 04213c194e622..0afbe5a0afc97 100644
--- a/src/expr/core/src/expr/wrapper/non_strict.rs
+++ b/src/expr/core/src/expr/wrapper/non_strict.rs
@@ -42,6 +42,16 @@ impl EvalErrorReport for ! {
     }
 }
 
+/// Log the error to report an error during evaluation.
+#[derive(Clone)]
+pub struct LogReport;
+
+impl EvalErrorReport for LogReport {
+    fn report(&self, error: ExprError) {
+        tracing::error!(%error, "failed to evaluate expression");
+    }
+}
+
 /// A wrapper of [`Expression`] that evaluates in a non-strict way. Basically...
 /// - When an error occurs during chunk-level evaluation, recompute in row-based execution and pad
 ///   with NULL for each failed row.