kumarUjjawal commented on code in PR #23071:
URL: https://github.com/apache/datafusion/pull/23071#discussion_r3457736149


##########
datafusion/sqllogictest/test_files/spark/string/repeat.slt:
##########


Review Comment:
   The code change teaches the Spark array-repeat function to accept a null 
count. But the test calls the two-argument repeat form, and in this test 
context that name resolves to the core string-repeat function, which already 
returned null for a null count long before this PR.
   
   Add the null-count case to the file that actually tests the array-repeat 
function. 
   
   



##########
datafusion/core/tests/dataframe/mod.rs:
##########
@@ -7242,3 +7243,22 @@ async fn test_grouping_with_alias() -> Result<()> {
 
     Ok(())
 }
+
+#[tokio::test]

Review Comment:
   The test builds the column expression and asserts that construction doesn't 
error. That does cover the issue, which is good. But the issue's real goal is a 
correct end-to-end result, and the test never resolves the lambda variables, 
never builds a physical plan, and never executes, so it can't catch a 
regression where planning succeeds but the resolved type or the executed output 
is wrong.



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to