goldmedal commented on code in PR #14728:
URL: https://github.com/apache/datafusion/pull/14728#discussion_r1958504929


##########
datafusion/functions/src/regex/regexpcount.rs:
##########
@@ -655,11 +657,12 @@ mod tests {
             let v_sv = ScalarValue::Utf8(Some(v.to_string()));
             let regex_sv = ScalarValue::Utf8(Some(regex.to_string()));
             let expected = expected.get(pos).cloned();
-            #[allow(deprecated)] // TODO: migrate to invoke_with_args
-            let re = RegexpCountFunc::new().invoke_batch(
-                &[ColumnarValue::Scalar(v_sv), 
ColumnarValue::Scalar(regex_sv)],
-                1,
-            );
+            #[allow(deprecated)]
+            let re = 
RegexpCountFunc::new().invoke_with_args(ScalarFunctionArgs {
+                args: vec![ColumnarValue::Scalar(v_sv), 
ColumnarValue::Scalar(regex_sv)],
+                number_rows: 2,

Review Comment:
   I just noticed that it's different from the previous number 🤔. However, I 
think `regexp_count` never uses the number of rows. It's fine.



##########
datafusion/functions/src/regex/regexpcount.rs:
##########
@@ -655,11 +657,12 @@ mod tests {
             let v_sv = ScalarValue::Utf8(Some(v.to_string()));
             let regex_sv = ScalarValue::Utf8(Some(regex.to_string()));
             let expected = expected.get(pos).cloned();
-            #[allow(deprecated)] // TODO: migrate to invoke_with_args
-            let re = RegexpCountFunc::new().invoke_batch(
-                &[ColumnarValue::Scalar(v_sv), 
ColumnarValue::Scalar(regex_sv)],
-                1,
-            );
+            #[allow(deprecated)]

Review Comment:
   There are other similar parts. We can remove all of them.



##########
datafusion/functions/src/regex/regexpcount.rs:
##########
@@ -655,11 +657,12 @@ mod tests {
             let v_sv = ScalarValue::Utf8(Some(v.to_string()));
             let regex_sv = ScalarValue::Utf8(Some(regex.to_string()));
             let expected = expected.get(pos).cloned();
-            #[allow(deprecated)] // TODO: migrate to invoke_with_args
-            let re = RegexpCountFunc::new().invoke_batch(
-                &[ColumnarValue::Scalar(v_sv), 
ColumnarValue::Scalar(regex_sv)],
-                1,
-            );
+            #[allow(deprecated)]

Review Comment:
   I think it is out of date. It's for `inovke_batch` but we have migrated to 
`inovek_with_args`. We can remove it.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org
For additional commands, e-mail: github-h...@datafusion.apache.org

Reply via email to