kosiew commented on code in PR #22038:
URL: https://github.com/apache/datafusion/pull/22038#discussion_r3924241103


##########
datafusion/common/src/config.rs:
##########
@@ -1031,6 +1031,25 @@ config_namespace! {
         /// Default: 128 MB
         pub max_spill_file_size_bytes: ConfigNonZeroUsize, default = 
non_zero_usize_default(128 * 1024 * 1024)
 
+        /// Enables the memory-limited fallback for `NestedLoopJoinExec` join
+        /// types that emit unmatched left rows in the final output (LEFT, LEFT
+        /// SEMI, LEFT ANTI, LEFT MARK, FULL) when the right side has multiple
+        /// partitions.
+        ///
+        /// This fallback coordinates per-chunk left state (visited bitmap and
+        /// probe-thread counter) across all right-side partitions, which
+        /// assumes every partition runs in the same process. Distributed
+        /// engines that execute each output partition as an independent task
+        /// (e.g. Ballista, datafusion-distributed) build a separate 
coordinator
+        /// per task and poll only one partition, so the cross-partition
+        /// counter never reaches zero and the fallback would stall. Such
+        /// engines should set this to `false`: the coordinated fallback is 
then
+        /// disabled for left-emitting multi-partition joins, which instead 
fail
+        /// with a resource-exhaustion error under memory pressure rather than
+        /// deadlocking. Single-partition and non-left-emitting joins are
+        /// unaffected and always keep the fallback.
+        pub enable_nlj_coordinated_fallback: bool, default = true

Review Comment:
   I think the remaining issue here is the API-health follow-up rather than 
whether this field shape can be accepted.
   
   This is still a valid `constructible_struct_adds_field` SemVer break because 
downstream exhaustive `ExecutionOptions { ... }` literals will no longer 
compile. I agree that the existing project precedent and advisory CI support 
accepting this kind of config-field change.
   
   Could we document the source migration in the version-specific Upgrade 
Guide? It would be helpful for the guide to show downstream users either how to 
add the new field explicitly or how to switch to `..Default::default()`.



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

Reply via email to