jayzhan211 commented on code in PR #26090:
URL: https://github.com/apache/datafusion/pull/26090#discussion_r4370158964
##########
datafusion/physical-plan/src/joins/sort_merge_join/bitwise_stream.rs:
##########
@@ -330,6 +376,22 @@ impl BitwiseSortMergeJoinStream {
let peak_mem_used =
MetricBuilder::new(metrics).peak_memory_usage("peak_mem_used",
partition);
+ if let Some(mode) = null_aware {
Review Comment:
nit: `execute()` already builds this mode with `null_aware_mode()` from the
same join type, keys, filter, null equality and sort options, and it's the only
non-test caller, so this check can't fail. I'd drop it and keep validation in
`try_new` and `execute()` only.
##########
datafusion/physical-plan/src/joins/sort_merge_join/exec.rs:
##########
@@ -64,6 +64,61 @@ use
datafusion_physical_expr_common::physical_expr::{PhysicalExprRef, fmt_sql};
use datafusion_physical_expr_common::sort_expr::{LexOrdering,
OrderingRequirements};
use futures::StreamExt;
+/// Null-aware (`NOT IN`) semantics of a soft merge join, derived from
Review Comment:
nit: a few typos: "soft merge join" here, `subqury_state` in
`bitwise_stream.rs` (line 208), "semetics" (line 1224), and the doc on
`null_aware()` (line 400) only says "Null aware". Something like "Whether this
join uses null-aware (`NOT IN`) semantics" would help.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
