eejbyfeldt commented on code in PR #13249:
URL: https://github.com/apache/datafusion/pull/13249#discussion_r1834438317


##########
datafusion/optimizer/src/eliminate_outer_join.rs:
##########
@@ -443,4 +361,214 @@ mod tests {
         \n    TableScan: t2";
         assert_optimized_plan_equal(plan, expected)
     }
+
+    #[test]
+    fn eliminate_full_with_hybrid_filter() -> Result<()> {
+        let t1 = test_table_scan_with_name("t1")?;
+        let t2 = test_table_scan_with_name("t2")?;
+
+        // eliminate to inner join
+        let plan = LogicalPlanBuilder::from(t1)
+            .join(
+                t2,
+                JoinType::Full,
+                (vec![Column::from_name("a")], vec![Column::from_name("a")]),
+                None,
+            )?
+            .filter(binary_expr(col("t1.b"), Gt, col("t2.b")))?
+            .build()?;
+        let expected = "\
+        Filter: t1.b > t2.b\
+        \n  Inner Join: t1.a = t2.a\
+        \n    TableScan: t1\
+        \n    TableScan: t2";
+        assert_optimized_plan_equal(plan, expected)
+    }
+
+    #[derive(Debug)]
+    struct DoNothingUdf {
+        signature: Signature,
+    }
+
+    impl DoNothingUdf {
+        pub fn new() -> Self {
+            Self {
+                signature: Signature::any(1, Volatility::Immutable),
+            }
+        }
+    }
+
+    impl ScalarUDFImpl for DoNothingUdf {
+        fn as_any(&self) -> &dyn std::any::Any {
+            self
+        }
+
+        fn name(&self) -> &str {
+            "do_nothing"
+        }
+
+        fn signature(&self) -> &Signature {
+            &self.signature
+        }
+
+        fn return_type(&self, arg_types: &[DataType]) -> Result<DataType> {
+            Ok(arg_types[0].clone())
+        }
+
+        fn invoke(&self, args: &[ColumnarValue]) -> Result<ColumnarValue> {
+            Ok(args[0].clone())
+        }
+    }
+
+    #[test]
+    fn eliminate_right_with_udf() -> Result<()> {
+        let t1 = test_table_scan_with_name("t1")?;
+        let t2 = test_table_scan_with_name("t2")?;
+        let fun = Arc::new(ScalarUDF::new_from_impl(DoNothingUdf::new()));
+
+        let plan = LogicalPlanBuilder::from(t1)
+            .join(
+                t2,
+                JoinType::Right,
+                (vec![Column::from_name("a")], vec![Column::from_name("a")]),
+                None,
+            )?
+            .filter(
+                Expr::ScalarFunction(ScalarFunction::new_udf(fun, 
vec![col("t1.b")]))
+                    .gt(lit(10u32)),
+            )?
+            .build()?;
+
+        let expected = "\
+        Filter: do_nothing(t1.b) > UInt32(10)\
+        \n  Inner Join: t1.a = t2.a\
+        \n    TableScan: t1\
+        \n    TableScan: t2";
+        assert_optimized_plan_equal(plan, expected)
+    }
+
+    #[derive(Debug)]
+    struct AlwaysNullUdf {
+        signature: Signature,
+    }
+
+    impl AlwaysNullUdf {
+        pub fn new() -> Self {
+            Self {
+                signature: Signature::any(1, Volatility::Immutable),
+            }
+        }
+    }
+
+    impl ScalarUDFImpl for AlwaysNullUdf {
+        fn as_any(&self) -> &dyn std::any::Any {
+            self
+        }
+
+        fn name(&self) -> &str {
+            "always_null"
+        }
+
+        fn signature(&self) -> &Signature {
+            &self.signature
+        }
+
+        fn return_type(&self, _arg_types: &[DataType]) -> Result<DataType> {
+            Ok(DataType::Null)
+        }
+
+        fn invoke(&self, args: &[ColumnarValue]) -> Result<ColumnarValue> {
+            Ok(match &args[0] {
+                ColumnarValue::Array(array) => {
+                    ColumnarValue::create_null_array(array.len())
+                }
+                ColumnarValue::Scalar(_) => 
ColumnarValue::Scalar(ScalarValue::Null),
+            })
+        }
+    }
+
+    #[test]
+    fn eliminate_right_with_null_udf() -> Result<()> {
+        let t1 = test_table_scan_with_name("t1")?;
+        let t2 = test_table_scan_with_name("t2")?;
+        let fun = Arc::new(ScalarUDF::new_from_impl(AlwaysNullUdf::new()));
+
+        let plan = LogicalPlanBuilder::from(t1)
+            .join(
+                t2,
+                JoinType::Right,
+                (vec![Column::from_name("a")], vec![Column::from_name("a")]),
+                None,
+            )?
+            .filter(
+                Expr::ScalarFunction(ScalarFunction::new_udf(fun, 
vec![col("t1.b")]))
+                    .is_null(),
+            )?
+            .build()?;
+
+        let expected = "\
+        Filter: always_null(t1.b) IS NULL\
+        \n  Right Join: t1.a = t2.a\
+        \n    TableScan: t1\
+        \n    TableScan: t2";
+        assert_optimized_plan_equal(plan, expected)
+    }
+
+    #[derive(Debug)]
+    struct VolatileUdf {
+        signature: Signature,
+    }
+
+    impl VolatileUdf {
+        pub fn new() -> Self {
+            Self {
+                signature: Signature::any(1, Volatility::Volatile),
+            }
+        }
+    }
+
+    impl ScalarUDFImpl for VolatileUdf {
+        fn as_any(&self) -> &dyn std::any::Any {
+            self
+        }
+
+        fn name(&self) -> &str {
+            "volatile_func"
+        }
+
+        fn signature(&self) -> &Signature {
+            &self.signature
+        }
+
+        fn return_type(&self, _arg_types: &[DataType]) -> Result<DataType> {
+            Ok(DataType::Boolean)
+        }

Review Comment:
   That or maybe some simple implementation. I would lead towards implementing 
it, just so so there is less chance of some other code change causing a panic 
here. 



-- 
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