[GitHub] [arrow-datafusion] jackwener commented on a diff in pull request #4725: Support non-tuple expression for in-subquery to join

[GitBox <gi...@apache.org>]

jackwener commented on code in PR #4725:
URL: https://github.com/apache/arrow-datafusion/pull/4725#discussion_r1056956206


##########
datafusion/core/tests/sql/joins.rs:
##########
@@ -2837,3 +2837,96 @@ async fn type_coercion_join_with_filter_and_equi_expr() -> Result<()> {
 
     Ok(())
 }
+
+#[tokio::test]
+async fn reduce_where_in_to_expr_equijoin() -> Result<()> {
+    let ctx = create_join_context("t1_id", "t2_id", false)?;

Review Comment:
   👍



##########
datafusion/optimizer/src/subquery_filter_to_join.rs:
##########
@@ -108,13 +107,17 @@ impl OptimizerRule for SubqueryFilterToJoin {
                             };
 
                             let right_key = right_schema.field(0).qualified_column();
-                            let left_key = match *expr.clone() {
-                                Expr::Column(col) => col,
-                                _ => return Err(DataFusionError::NotImplemented(
-                                    "Filtering by expression not implemented for InSubquery"
-                                        .to_string(),
-                                )),
-                            };
+                            let left_key = *expr.clone();
+                            let (on, filter) =
+                                // When left is a constant expression, like 1, 
+                                // the join predicate will be `1 = right_key`, it is better to add it to filter.
+                                if left_key.to_columns()?.is_empty() {
+                                    let equi_expr =
+                                        Expr::eq(*expr.clone(), Expr::Column(right_key));
+                                    (vec![], Some(equi_expr))
+                                } else {

Review Comment:
   IMHO, I think it's a good improvement but maybe isn't suitable to put in this rule.🤔
   It could be better to handle separate/handle `on/joinfilter` in a new rule(I also contain more improvement).
   
   Just a thought, I think we also can add a TODO, and handle them as a future ticket.



-- 
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-unsubscribe@arrow.apache.org

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