andygrove commented on code in PR #16946: URL: https://github.com/apache/datafusion/pull/16946#discussion_r2291750539
########## datafusion/sqllogictest/test_files/spark/conditional/if.slt: ########## @@ -21,7 +21,135 @@ # For more information, please see: # https://github.com/apache/datafusion/issues/15914 -## Original Query: SELECT if(1 < 2, 'a', 'b'); -## PySpark 3.5.5 Result: {'(IF((1 < 2), a, b))': 'a', 'typeof((IF((1 < 2), a, b)))': 'string', 'typeof((1 < 2))': 'boolean', 'typeof(a)': 'string', 'typeof(b)': 'string'} -#query -#SELECT if((1 < 2)::boolean, 'a'::string, 'b'::string); +## Basic IF function tests + +# Test basic true condition +query T +SELECT if(true, 'yes', 'no'); Review Comment: Could you add tests that use more complex expressions, such as referencing columns in an input file? Also, could you add tests that demonstrate that the 2nd expression is only evaluated when the 1st argument evaluates to true? There should be some tests in `case.slt` that can be repurposed. By the way, in Comet, we just translate an `if` condition to a `case` expression. ```rust impl IfExpr { /// Create a new IF expression pub fn new( if_expr: Arc<dyn PhysicalExpr>, true_expr: Arc<dyn PhysicalExpr>, false_expr: Arc<dyn PhysicalExpr>, ) -> Self { Self { if_expr: Arc::clone(&if_expr), true_expr: Arc::clone(&true_expr), false_expr: Arc::clone(&false_expr), case_expr: Arc::new( CaseExpr::try_new(None, vec![(if_expr, true_expr)], Some(false_expr)).unwrap(), ), } } } ``` -- 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
