Re: [PR] Fix: Sort Merge Join LeftSemi issues when JoinFilter is set [datafusion]

alamb commented on PR #10304: URL: https://github.com/apache/datafusion/pull/10304#issuecomment-2121484917 🚀 -- 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

Re: [PR] Fix: Sort Merge Join LeftSemi issues when JoinFilter is set [datafusion]

comphead merged PR #10304: URL: https://github.com/apache/datafusion/pull/10304 -- 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...@dataf

Re: [PR] Fix: Sort Merge Join LeftSemi issues when JoinFilter is set [datafusion]

viirya commented on PR #10304: URL: https://github.com/apache/datafusion/pull/10304#issuecomment-2120729543 > I've seen some issues in this patch. It doesn't look like a correct fix. Took another look. Looks okay to me. -- This is an automated message from the Apache Git Ser

Re: [PR] Fix: Sort Merge Join LeftSemi issues when JoinFilter is set [datafusion]

viirya commented on code in PR #10304: URL: https://github.com/apache/datafusion/pull/10304#discussion_r1606948252 ## datafusion/physical-plan/src/joins/sort_merge_join.rs: ## @@ -989,8 +996,21 @@ impl SMJStream { } } Ordering::Equal =>

Re: [PR] Fix: Sort Merge Join LeftSemi issues when JoinFilter is set [datafusion]

viirya commented on code in PR #10304: URL: https://github.com/apache/datafusion/pull/10304#discussion_r1606944266 ## datafusion/physical-plan/src/joins/sort_merge_join.rs: ## @@ -989,8 +996,21 @@ impl SMJStream { } } Ordering::Equal =>

Re: [PR] Fix: Sort Merge Join LeftSemi issues when JoinFilter is set [datafusion]

viirya commented on code in PR #10304: URL: https://github.com/apache/datafusion/pull/10304#discussion_r1606942756 ## datafusion/physical-plan/src/joins/sort_merge_join.rs: ## @@ -989,8 +996,21 @@ impl SMJStream { } } Ordering::Equal =>

Re: [PR] Fix: Sort Merge Join LeftSemi issues when JoinFilter is set [datafusion]

viirya commented on code in PR #10304: URL: https://github.com/apache/datafusion/pull/10304#discussion_r1606924344 ## datafusion/physical-plan/src/joins/sort_merge_join.rs: ## @@ -1365,6 +1402,69 @@ fn get_filter_column( filter_columns } +/// Get `buffered_indices` rows

Re: [PR] Fix: Sort Merge Join LeftSemi issues when JoinFilter is set [datafusion]

viirya commented on code in PR #10304: URL: https://github.com/apache/datafusion/pull/10304#discussion_r1606910619 ## datafusion/physical-plan/src/joins/sort_merge_join.rs: ## @@ -1365,6 +1402,69 @@ fn get_filter_column( filter_columns } +/// Get `buffered_indices` rows

Re: [PR] Fix: Sort Merge Join LeftSemi issues when JoinFilter is set [datafusion]

viirya commented on code in PR #10304: URL: https://github.com/apache/datafusion/pull/10304#discussion_r1606910619 ## datafusion/physical-plan/src/joins/sort_merge_join.rs: ## @@ -1365,6 +1402,69 @@ fn get_filter_column( filter_columns } +/// Get `buffered_indices` rows

Re: [PR] Fix: Sort Merge Join LeftSemi issues when JoinFilter is set [datafusion]

comphead commented on PR #10304: URL: https://github.com/apache/datafusion/pull/10304#issuecomment-2120655736 > I've seen some issues in this patch. It doesn't look like a correct fix. The tests currently in sync with what hash join returns, is there a test showing the opposite? --

Re: [PR] Fix: Sort Merge Join LeftSemi issues when JoinFilter is set [datafusion]

comphead commented on code in PR #10304: URL: https://github.com/apache/datafusion/pull/10304#discussion_r1606905348 ## datafusion/physical-plan/src/joins/sort_merge_join.rs: ## @@ -1365,6 +1402,69 @@ fn get_filter_column( filter_columns } +/// Get `buffered_indices` row

Re: [PR] Fix: Sort Merge Join LeftSemi issues when JoinFilter is set [datafusion]

viirya commented on code in PR #10304: URL: https://github.com/apache/datafusion/pull/10304#discussion_r1606310433 ## datafusion/physical-plan/src/joins/sort_merge_join.rs: ## @@ -1365,6 +1402,69 @@ fn get_filter_column( filter_columns } +/// Get `buffered_indices` rows

Re: [PR] Fix: Sort Merge Join LeftSemi issues when JoinFilter is set [datafusion]

viirya commented on code in PR #10304: URL: https://github.com/apache/datafusion/pull/10304#discussion_r1606287947 ## datafusion/physical-plan/src/joins/sort_merge_join.rs: ## @@ -989,8 +996,21 @@ impl SMJStream { } } Ordering::Equal =>

Re: [PR] Fix: Sort Merge Join LeftSemi issues when JoinFilter is set [datafusion]

viirya commented on code in PR #10304: URL: https://github.com/apache/datafusion/pull/10304#discussion_r1606289302 ## datafusion/physical-plan/src/joins/sort_merge_join.rs: ## @@ -989,8 +996,21 @@ impl SMJStream { } } Ordering::Equal =>

Re: [PR] Fix: Sort Merge Join LeftSemi issues when JoinFilter is set [datafusion]

viirya commented on code in PR #10304: URL: https://github.com/apache/datafusion/pull/10304#discussion_r1606287947 ## datafusion/physical-plan/src/joins/sort_merge_join.rs: ## @@ -989,8 +996,21 @@ impl SMJStream { } } Ordering::Equal =>

Re: [PR] Fix: Sort Merge Join LeftSemi issues when JoinFilter is set [datafusion]

comphead commented on PR #10304: URL: https://github.com/apache/datafusion/pull/10304#issuecomment-2119569734 @viirya I'm planning to merge this PR soon as it fixes the crash, and addresses your concern (please see the slt test covering this specific case). All other improvements can be in

Re: [PR] Fix: Sort Merge Join LeftSemi issues when JoinFilter is set [datafusion]

comphead commented on code in PR #10304: URL: https://github.com/apache/datafusion/pull/10304#discussion_r1604220440 ## datafusion/physical-plan/src/joins/sort_merge_join.rs: ## @@ -54,7 +45,17 @@ use datafusion_execution::TaskContext; use datafusion_physical_expr::equivalence:

Re: [PR] Fix: Sort Merge Join LeftSemi issues when JoinFilter is set [datafusion]

viirya commented on code in PR #10304: URL: https://github.com/apache/datafusion/pull/10304#discussion_r1601943239 ## datafusion/physical-plan/src/joins/sort_merge_join.rs: ## @@ -991,6 +992,9 @@ impl SMJStream { Ordering::Equal => { if matches!(sel

Re: [PR] Fix: Sort Merge Join LeftSemi issues when JoinFilter is set [datafusion]

viirya commented on code in PR #10304: URL: https://github.com/apache/datafusion/pull/10304#discussion_r1601943239 ## datafusion/physical-plan/src/joins/sort_merge_join.rs: ## @@ -991,6 +992,9 @@ impl SMJStream { Ordering::Equal => { if matches!(sel

Re: [PR] Fix: Sort Merge Join LeftSemi issues when JoinFilter is set [datafusion]

comphead commented on code in PR #10304: URL: https://github.com/apache/datafusion/pull/10304#discussion_r1601861040 ## datafusion/physical-plan/src/joins/sort_merge_join.rs: ## @@ -991,6 +992,9 @@ impl SMJStream { Ordering::Equal => { if matches!(s

Re: [PR] Fix: Sort Merge Join LeftSemi issues when JoinFilter is set [datafusion]

comphead commented on code in PR #10304: URL: https://github.com/apache/datafusion/pull/10304#discussion_r1599285424 ## datafusion/physical-plan/src/joins/sort_merge_join.rs: ## @@ -991,6 +992,9 @@ impl SMJStream { Ordering::Equal => { if matches!(s

Re: [PR] Fix: Sort Merge Join LeftSemi issues when JoinFilter is set [datafusion]

viirya commented on code in PR #10304: URL: https://github.com/apache/datafusion/pull/10304#discussion_r1599279884 ## datafusion/physical-plan/src/joins/sort_merge_join.rs: ## @@ -991,6 +992,9 @@ impl SMJStream { Ordering::Equal => { if matches!(sel

Re: [PR] Fix: Sort Merge Join LeftSemi issues when JoinFilter is set [datafusion]

comphead commented on code in PR #10304: URL: https://github.com/apache/datafusion/pull/10304#discussion_r1599255387 ## datafusion/physical-plan/src/joins/sort_merge_join.rs: ## @@ -991,6 +992,9 @@ impl SMJStream { Ordering::Equal => { if matches!(s

Re: [PR] Fix: Sort Merge Join LeftSemi issues when JoinFilter is set [datafusion]

comphead commented on code in PR #10304: URL: https://github.com/apache/datafusion/pull/10304#discussion_r1599216844 ## datafusion/physical-plan/src/joins/sort_merge_join.rs: ## @@ -1363,6 +1380,57 @@ fn get_filter_column( filter_columns } +// Get buffered data sliece by

Re: [PR] Fix: Sort Merge Join LeftSemi issues when JoinFilter is set [datafusion]

comphead commented on code in PR #10304: URL: https://github.com/apache/datafusion/pull/10304#discussion_r1599204082 ## datafusion/physical-plan/src/joins/sort_merge_join.rs: ## @@ -1363,6 +1380,57 @@ fn get_filter_column( filter_columns } +// Get buffered data sliece by

Re: [PR] Fix: Sort Merge Join LeftSemi issues when JoinFilter is set [datafusion]

comphead commented on code in PR #10304: URL: https://github.com/apache/datafusion/pull/10304#discussion_r1599103669 ## datafusion/physical-plan/src/joins/sort_merge_join.rs: ## @@ -991,6 +992,9 @@ impl SMJStream { Ordering::Equal => { if matches!(s

Re: [PR] Fix: Sort Merge Join LeftSemi issues when JoinFilter is set [datafusion]

viirya commented on code in PR #10304: URL: https://github.com/apache/datafusion/pull/10304#discussion_r1599088624 ## datafusion/physical-plan/src/joins/sort_merge_join.rs: ## @@ -991,6 +992,9 @@ impl SMJStream { Ordering::Equal => { if matches!(sel

Re: [PR] Fix: Sort Merge Join LeftSemi issues when JoinFilter is set [datafusion]

comphead commented on PR #10304: URL: https://github.com/apache/datafusion/pull/10304#issuecomment-2108764729 Thanks @alamb I'll add more docs and also added a task to check RightSemi join to https://github.com/apache/datafusion/issues/9846 I'll let it be opened a little longer to giv

Re: [PR] Fix: Sort Merge Join LeftSemi issues when JoinFilter is set [datafusion]

comphead commented on code in PR #10304: URL: https://github.com/apache/datafusion/pull/10304#discussion_r1599052085 ## datafusion/physical-plan/src/joins/sort_merge_join.rs: ## @@ -1363,6 +1380,57 @@ fn get_filter_column( filter_columns } +// Get buffered data sliece by

Re: [PR] Fix: Sort Merge Join LeftSemi issues when JoinFilter is set [datafusion]

comphead commented on code in PR #10304: URL: https://github.com/apache/datafusion/pull/10304#discussion_r1599051364 ## datafusion/physical-plan/src/joins/sort_merge_join.rs: ## @@ -1363,6 +1380,57 @@ fn get_filter_column( filter_columns } +// Get buffered data sliece by

Re: [PR] Fix: Sort Merge Join LeftSemi issues when JoinFilter is set [datafusion]

comphead commented on code in PR #10304: URL: https://github.com/apache/datafusion/pull/10304#discussion_r1599049286 ## datafusion/physical-plan/src/joins/sort_merge_join.rs: ## @@ -1161,6 +1162,15 @@ impl SMJStream { let filter_columns = if chunk.buffered_batch_idx

Re: [PR] Fix: Sort Merge Join LeftSemi issues when JoinFilter is set [datafusion]

alamb commented on code in PR #10304: URL: https://github.com/apache/datafusion/pull/10304#discussion_r1598870577 ## datafusion/sqllogictest/test_files/sort_merge_join.slt: ## @@ -263,5 +263,139 @@ DROP TABLE t1; statement ok DROP TABLE t2; + Review Comment: To verify th

Re: [PR] Fix: Sort Merge Join LeftSemi issues when JoinFilter is set [datafusion]

viirya commented on PR #10304: URL: https://github.com/apache/datafusion/pull/10304#issuecomment-2108010944 I'll take another look today. -- 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 speci

Re: [PR] Fix: Sort Merge Join LeftSemi issues when JoinFilter is set [datafusion]

alamb commented on PR #10304: URL: https://github.com/apache/datafusion/pull/10304#issuecomment-2107980838 I will review this today -- 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 co

Re: [PR] Fix: Sort Merge Join LeftSemi issues when JoinFilter is set [datafusion]

comphead commented on PR #10304: URL: https://github.com/apache/datafusion/pull/10304#issuecomment-2107801927 @viirya @alamb can I get a review on this PR please? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the UR

Re: [PR] Fix: Sort Merge Join LeftSemi issues when JoinFilter is set [datafusion]

comphead commented on code in PR #10304: URL: https://github.com/apache/datafusion/pull/10304#discussion_r1594814388 ## datafusion/physical-plan/src/joins/sort_merge_join.rs: ## @@ -1195,7 +1205,40 @@ impl SMJStream { .into_array(filter_batch.num_rows())

Re: [PR] Fix: Sort Merge Join LeftSemi issues when JoinFilter is set [datafusion]

comphead commented on code in PR #10304: URL: https://github.com/apache/datafusion/pull/10304#discussion_r1593140372 ## datafusion/physical-plan/src/joins/sort_merge_join.rs: ## @@ -991,6 +992,9 @@ impl SMJStream { Ordering::Equal => { if matches!(s

Re: [PR] Fix: Sort Merge Join LeftSemi issues when JoinFilter is set [datafusion]

comphead commented on code in PR #10304: URL: https://github.com/apache/datafusion/pull/10304#discussion_r1593140372 ## datafusion/physical-plan/src/joins/sort_merge_join.rs: ## @@ -991,6 +992,9 @@ impl SMJStream { Ordering::Equal => { if matches!(s

Re: [PR] Fix: Sort Merge Join LeftSemi issues when JoinFilter is set [datafusion]

viirya commented on code in PR #10304: URL: https://github.com/apache/datafusion/pull/10304#discussion_r1593122230 ## datafusion/physical-plan/src/joins/sort_merge_join.rs: ## @@ -991,6 +992,9 @@ impl SMJStream { Ordering::Equal => { if matches!(sel

Re: [PR] Fix: Sort Merge Join LeftSemi issues when JoinFilter is set [datafusion]

viirya commented on code in PR #10304: URL: https://github.com/apache/datafusion/pull/10304#discussion_r1592978951 ## datafusion/physical-plan/src/joins/sort_merge_join.rs: ## @@ -1195,11 +1207,45 @@ impl SMJStream { .into_array(filter_batch.num_rows())?

Re: [PR] Fix: Sort Merge Join LeftSemi issues when JoinFilter is set [datafusion]

viirya commented on code in PR #10304: URL: https://github.com/apache/datafusion/pull/10304#discussion_r1592977161 ## datafusion/physical-plan/src/joins/sort_merge_join.rs: ## @@ -1161,6 +1164,15 @@ impl SMJStream { let filter_columns = if chunk.buffered_batch_idx.i

Re: [PR] Fix: Sort Merge Join LeftSemi issues when JoinFilter is set [datafusion]

viirya commented on code in PR #10304: URL: https://github.com/apache/datafusion/pull/10304#discussion_r1592971894 ## datafusion/physical-plan/src/joins/sort_merge_join.rs: ## @@ -991,6 +992,9 @@ impl SMJStream { Ordering::Equal => { if matches!(sel

Re: [PR] Fix: Sort Merge Join LeftSemi issues when JoinFilter is set [datafusion]

viirya commented on code in PR #10304: URL: https://github.com/apache/datafusion/pull/10304#discussion_r1592969741 ## datafusion/physical-plan/src/joins/sort_merge_join.rs: ## @@ -991,6 +992,9 @@ impl SMJStream { Ordering::Equal => { if matches!(sel

Re: [PR] Fix: Sort Merge Join LeftSemi issues when JoinFilter is set [datafusion]

viirya commented on code in PR #10304: URL: https://github.com/apache/datafusion/pull/10304#discussion_r1592966756 ## datafusion/physical-plan/src/joins/sort_merge_join.rs: ## @@ -1134,7 +1138,6 @@ impl SMJStream { .collect::, ArrowError>>()?; let

Re: [PR] Fix: Sort Merge Join LeftSemi issues when JoinFilter is set [datafusion]

viirya commented on code in PR #10304: URL: https://github.com/apache/datafusion/pull/10304#discussion_r1592956011 ## datafusion/physical-plan/src/joins/sort_merge_join.rs: ## @@ -991,6 +992,9 @@ impl SMJStream { Ordering::Equal => { if matches!(sel

Re: [PR] Fix: Sort Merge Join LeftSemi issues when JoinFilter is set [datafusion]

viirya commented on code in PR #10304: URL: https://github.com/apache/datafusion/pull/10304#discussion_r1592943870 ## datafusion/physical-plan/src/joins/sort_merge_join.rs: ## @@ -824,6 +824,7 @@ impl SMJStream { self.join_metrics.input_rows.add(batc