ygf11 commented on code in PR #4193:
URL: https://github.com/apache/arrow-datafusion/pull/4193#discussion_r1023912724


##########
datafusion/sql/src/planner.rs:
##########
@@ -5509,6 +5599,99 @@ mod tests {
         assert!(logical_plan("SELECT \"1\"").is_err());
     }
 
+    #[test]
+    fn test_constant_expr_eq_join() {
+        let sql = "SELECT id, order_id \
+            FROM person \
+            INNER JOIN orders \
+            ON person.id = 10";
+
+        let expected = "Projection: person.id, orders.order_id\
+        \n  Filter: person.id = Int64(10)\
+        \n    CrossJoin:\
+        \n      TableScan: person\
+        \n      TableScan: orders";
+        quick_test(sql, expected);
+    }
+
+    #[test]
+    fn test_right_left_expr_eq_join() {
+        let sql = "SELECT id, order_id \
+            FROM person \
+            INNER JOIN orders \
+            ON orders.customer_id * 2 = person.id + 10";
+
+        let expected = "Projection: person.id, orders.order_id\
+        \n  Inner Join: person.id + Int64(10) = orders.customer_id * Int64(2)\
+        \n    Projection: person.id, person.first_name, person.last_name, 
person.age, person.state, person.salary, person.birth_date, person.😀, person.id 
+ Int64(10)\
+        \n      TableScan: person\
+        \n    Projection: orders.order_id, orders.customer_id, 
orders.o_item_id, orders.qty, orders.price, orders.delivered, 
orders.customer_id * Int64(2)\
+        \n      TableScan: orders";
+        quick_test(sql, expected);
+    }
+
+    #[test]
+    fn test_single_column_expr_eq_join() {
+        let sql = "SELECT id, order_id \
+            FROM person \
+            INNER JOIN orders \
+            ON person.id + 10 = orders.customer_id * 2";
+
+        let expected = "Projection: person.id, orders.order_id\
+        \n  Inner Join: person.id + Int64(10) = orders.customer_id * Int64(2)\
+        \n    Projection: person.id, person.first_name, person.last_name, 
person.age, person.state, person.salary, person.birth_date, person.😀, person.id 
+ Int64(10)\
+        \n      TableScan: person\
+        \n    Projection: orders.order_id, orders.customer_id, 
orders.o_item_id, orders.qty, orders.price, orders.delivered, 
orders.customer_id * Int64(2)\
+        \n      TableScan: orders";
+        quick_test(sql, expected);
+    }
+
+    #[test]
+    fn test_multiple_column_expr_eq_join() {
+        let sql = "SELECT id, order_id \
+            FROM person \
+            INNER JOIN orders \
+            ON person.id + person.age + 10 = orders.customer_id * 2 - 
orders.price";
+
+        let expected = "Projection: person.id, orders.order_id\
+        \n  Inner Join: person.id + person.age + Int64(10) = 
orders.customer_id * Int64(2) - orders.price\
+        \n    Projection: person.id, person.first_name, person.last_name, 
person.age, person.state, person.salary, person.birth_date, person.😀, person.id 
+ person.age + Int64(10)\
+        \n      TableScan: person\
+        \n    Projection: orders.order_id, orders.customer_id, 
orders.o_item_id, orders.qty, orders.price, orders.delivered, 
orders.customer_id * Int64(2) - orders.price\
+        \n      TableScan: orders";
+        quick_test(sql, expected);
+    }
+
+    #[test]
+    fn test_left_projection_expr_eq_join() {
+        let sql = "SELECT id, order_id \
+            FROM person \
+            INNER JOIN orders \
+            ON person.id + person.age + 10 = orders.customer_id";
+
+        let expected = "Projection: person.id, orders.order_id\
+        \n  Inner Join: person.id + person.age + Int64(10) = 
orders.customer_id\
+        \n    Projection: person.id, person.first_name, person.last_name, 
person.age, person.state, person.salary, person.birth_date, person.😀, person.id 
+ person.age + Int64(10)\
+        \n      TableScan: person\
+        \n    TableScan: orders";
+        quick_test(sql, expected);
+    }
+
+    #[test]
+    fn test_right_projection_expr_eq_join() {
+        let sql = "SELECT id, order_id \
+            FROM person \
+            INNER JOIN orders \
+            ON person.id = orders.customer_id * 2 - orders.price";
+
+        let expected = "Projection: person.id, orders.order_id\
+        \n  Inner Join: person.id = orders.customer_id * Int64(2) - 
orders.price\
+        \n    TableScan: person\
+        \n    Projection: orders.order_id, orders.customer_id, 
orders.o_item_id, orders.qty, orders.price, orders.delivered, 
orders.customer_id * Int64(2) - orders.price\
+        \n      TableScan: orders";
+        quick_test(sql, expected);
+    }
+

Review Comment:
   > I think it is OK to projection such kind expressions, the type of the new 
projected column type is bool, the bool value is evaluated again during the 
join.
   
   It seems your idea make things simple.
   
   Currently for join condition like `person.id = 10`, I treat `person.id` as 
left key, and `10` as `right key`, this need do type coercion.
   
   But as you suggestion, we can treat `person.id = 10` as left key, and right 
key as `true`, 
   then exist `type coercion` will handle this condition.
   
   I rewrite the expression to `(person.id = 10) = true`, then the test passes 
now. 
   The full join test case will like:
   ```rust
   #[test]
       fn test_one_side_constant_full_join() {
           let sql = "SELECT id, order_id \
               FROM person \
               FULL OUTER JOIN orders \
               ON person.id = 10";
   
           let expected = "Projection: person.id, orders.order_id\
           \n  Filter: person.id = Int64(10) = Boolean(true)\
           \n    CrossJoin:\
           \n      TableScan: person\
           \n      TableScan: orders";
           quick_test(sql, expected);
       }
   ```
    
    



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

Reply via email to