xavlee opened a new pull request, #24698: URL: https://github.com/apache/datafusion/pull/24698
## Which issue does this PR relate to? - Part of #24438. - Depends on #24697. ## Rationale for this change The main design risk in #24438 is propagation safety: a stale assertion could make an aggregate emit a group too early and return incorrect results. This PR isolates that concern from aggregate execution so the assertion's lifecycle can be reviewed on its own. ## Contract `ExecutionPlan::group_contiguous_exprs` describes one complete composite tuple. Within each output partition, every distinct tuple may occupy at most one contiguous range of rows. Tuple values need not be sorted, and the assertion implies neither ordering nor distribution. The lifecycle is intentionally fail-closed: 1. `DataSource` may declare the tuple, and `DataSourceExec` delegates to it. 2. `ProjectionExec` maps the complete tuple only when every component can be projected; otherwise it drops the assertion. 3. `CooperativeExec` delegates explicitly because it is a row-transparent wrapper inserted by the default optimizer. 4. Every other execution operator gets the default empty assertion. The assertion is deliberately stored outside `PlanProperties`. An operator that clones cached plan properties therefore cannot preserve it accidentally. `ProjectionExec` also recomputes the assertion on the same-properties child-replacement fast path. This is a correctness contract: a source that declares an invalid tuple can cause incorrect results once a consumer uses it. ## Scope - This PR adds propagation only; the next stack commit adds the first and only consumer. - It does not infer contiguity through monotonic functions. A source may certify a derived expression such as `date_bin(time)`, and projection can map that exact expression. - It does not extend DataFusion FFI. An FFI boundary therefore drops the assertion via the default implementation, which is safe. The motivating production scan is compiled and serialized through its local physical extension codec rather than DataFusion FFI. ## Stack GitHub cannot use a branch in the contributor fork as an Apache PR base, so this draft includes #24697 as its first commit. The propagation change is the second commit (`49f15a4c9`). After #24697 merges, this branch can be rebased to show only the propagation diff. ## Are these changes tested? Yes. Tests cover: - `DataSource` to `DataSourceExec` delegation and explain display; - complete and partial projection mapping; - same-`PlanProperties` child replacement; - explicit `CooperativeExec` preservation; and - default loss through operators that do not opt in. Validation: - `cargo test -p datafusion-physical-plan --lib` - `cargo test -p datafusion-datasource --lib` - strict clippy and rustdoc for both crates - `cargo-semver-checks` for both crates ## Are there any user-facing changes? Yes. `ExecutionPlan` and `DataSource` implementors may opt into the new assertion. Both trait methods have safe empty defaults. -- 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]
