Skip to content

Commit 1a84bc3

Browse files
committed
docs: explain limit pushdown state transitions
Signed-off-by: discord9 <discord9@163.com>
1 parent 4b30ece commit 1a84bc3

1 file changed

Lines changed: 29 additions & 14 deletions

File tree

datafusion/physical-optimizer/src/limit_pushdown.rs

Lines changed: 29 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -83,12 +83,12 @@ use datafusion_physical_plan::{ExecutionPlan, ExecutionPlanProperties};
8383
#[derive(Default, Debug)]
8484
pub struct LimitPushdown {}
8585

86-
/// This is a "data class" we use within the [`LimitPushdown`] rule to push
87-
/// down limits in the plan. GlobalRequirements are hold as a rule-wide state
88-
/// and holds the fetch and skip information. The struct also has a field named
89-
/// satisfied which means if the "current" plan is valid in terms of limits or not.
86+
/// State carried through [`LimitPushdown`] while it pushes limits down the plan.
9087
///
91-
/// For example: If the plan is satisfied with current fetch info, we decide to not add a LocalLimit
88+
/// While `status` is pending, `skip` and `fetch` are semantic requirements
89+
/// needing enforcement. Once no enforcement remains pending, a retained `fetch`
90+
/// is only an early-stop budget for descendant operators; it must not create
91+
/// another semantic limit.
9292
///
9393
/// [`LimitPushdown`]: crate::limit_pushdown::LimitPushdown
9494
#[derive(Clone, Debug)]
@@ -99,12 +99,19 @@ pub struct GlobalRequirements {
9999
status: LimitStatus,
100100
}
101101

102+
/// Tracks a requirement's scope and enforcement state, which cannot be inferred
103+
/// from a numeric operator `fetch` or the rewritten plan shape.
102104
#[derive(Clone, Copy, Debug, Default, PartialEq, Eq)]
103105
enum LimitStatus {
106+
/// No inherited semantic requirement or fetch budget.
104107
#[default]
105108
None,
109+
/// One pending cap is required per output partition of the current branch.
106110
PendingLocal,
111+
/// One pending cap is required across all output partitions of the current subtree.
107112
PendingGlobal,
113+
/// The semantic obligation has been enforced, absorbed, or proven redundant
114+
/// at or above this point; retained `fetch` is an early-stop hint only.
108115
Enforced,
109116
}
110117

@@ -153,13 +160,18 @@ pub fn pushdown_limit_helper(
153160
if global_state.status == LimitStatus::PendingLocal
154161
&& pushdown_plan.output_partitioning().partition_count() == 1
155162
{
163+
// Local and global scope are equivalent with one output partition, but
164+
// retain the global scope in case recursion later exposes multi-partition
165+
// children, including through extension combiners.
156166
global_state.status = LimitStatus::PendingGlobal;
157167
}
158168

159169
if let Some(global_limit) = pushdown_plan.downcast_ref::<GlobalLimitExec>()
160170
&& global_limit.skip() == 0
161171
&& global_limit.fetch().is_none()
162172
{
173+
// Remove this no-op wrapper without clearing inherited state, which may
174+
// have been promoted from local to global scope at a one-output boundary.
163175
return Ok((
164176
Transformed {
165177
data: Arc::clone(global_limit.input()),
@@ -173,6 +185,10 @@ pub fn pushdown_limit_helper(
173185
if global_state.status == LimitStatus::PendingGlobal
174186
&& pushdown_plan.output_partitioning().partition_count() > 1
175187
{
188+
// This must precede generic `plan.fetch()` handling: a fetch on multiple
189+
// outputs cannot by itself prove a global cap, because the `ExecutionPlan`
190+
// trait does not formally encode scope. Retain it only as a per-partition
191+
// hint; an existing smaller hint does not imply a smaller global cap.
176192
let hint = global_state.fetch.map(|fetch| fetch + global_state.skip);
177193
if let Some(hint) = hint {
178194
let hint = pushdown_plan.fetch().map_or(hint, |fetch| fetch.min(hint));
@@ -248,8 +264,7 @@ pub fn pushdown_limit_helper(
248264
));
249265
}
250266

251-
// If we have a non-limit operator with fetch capability, update global
252-
// state as necessary:
267+
// Merge a fetch already present on a non-limit operator into global state.
253268
if pushdown_plan.fetch().is_some() {
254269
if global_state.skip == 0 {
255270
global_state.status = LimitStatus::Enforced;
@@ -419,10 +434,9 @@ pub(crate) fn pushdown_limits(
419434
(new_node, global_state) = pushdown_limit_helper(new_node.data, global_state)?;
420435
}
421436

422-
// Once a limit has been materialized above the current node, child
423-
// subtrees should not inherit its `skip`. Keep `fetch`, but clear
424-
// `skip` before recursing so child-local limits are not merged with
425-
// an `OFFSET` that has already been applied.
437+
// No semantic enforcement remains pending for a child subtree. Descendants
438+
// may inherit the `fetch` budget for early stopping, but `OFFSET` must not
439+
// cross this point or combine with nested limits.
426440
if global_state.status == LimitStatus::Enforced {
427441
global_state.skip = 0;
428442
}
@@ -469,9 +483,10 @@ fn add_limit(
469483
}
470484
}
471485

472-
/// Materializes a global requirement at a single-partition boundary. A fetch
473-
/// on a multi-partition plan is only a per-partition hint, so it must be
474-
/// followed by a partition combiner before the requirement is satisfied.
486+
/// Materializes a global requirement at a single-partition boundary. A fetch on
487+
/// a multi-partition plan cannot by itself prove a global cap, because the
488+
/// `ExecutionPlan` trait does not formally encode scope. It is retained only as
489+
/// a per-partition hint, so a partition combiner must satisfy the requirement.
475490
fn materialize_global_requirement(
476491
pushdown_plan: Arc<dyn ExecutionPlan>,
477492
skip: usize,

0 commit comments

Comments
 (0)