kumarUjjawal opened a new pull request, #25073:
URL: https://github.com/apache/datafusion/pull/25073
## Which issue does this PR close?
- Part of #13815.
## Rationale for this change
`check_default_invariants` is generic over each `ExecutionPlan`
implementation. The compiler therefore generated many copies of its
dynamic-expression and input-distribution validation logic.
These checks depend only on erased values such as the plan name, children,
and dynamic expressions. Sharing the heavier validation paths reduces generated
code without changing invariant semantics or the public API.
Local release measurements against `upstream/main` at `92746a993`:
| Measurement | `upstream/main` | This PR | Change |
|---|---:|---:|---:|
| Targeted invariant-check LLVM IR lines | 68,856 | 47,082 | -31.6% |
| Total physical-plan LLVM IR lines | 2,560,976 | 2,513,101 | -1.87% |
| Release `rlib` size | 56,223,208 bytes | 55,689,856 bytes | -0.95% |
| Linked `partial_ordering` benchmark binary | 5,882,960 bytes | 5,882,960
bytes | No change |
The linked benchmark binary was unchanged because its linker removed unused
code. This PR does not claim a measured end-to-end WASM binary reduction.
The added Criterion benchmark showed no runtime regression:
| Case | `upstream/main` | This PR |
|---|---:|---:|
| Leaf plan | 13.016–13.030 ns | 12.939–12.951 ns |
| Four-child plan | 150.24–151.02 ns | 142.75–145.68 ns |
## What changes are included in this PR?
- Move dynamic-expression ID validation behind a non-generic helper.
- Make input-distribution invariant validation non-generic.
- Keep the inexpensive per-plan length checks inline.
- Skip dynamic-expression validation when a plan produces no dynamic
expressions.
- Preserve the public `check_default_invariants` signature and existing
error behavior.
- Add unit tests for malformed vectors, dynamic-expression IDs,
co-partitioning requirements, and trait-object callers.
- Add a Criterion benchmark for leaf and four-child plans.
## What is the testing strategy for this PR?
The new unit tests cover:
- incorrect invariant-vector lengths and their error messages;
- missing and duplicate dynamic-expression IDs;
- malformed and valid co-partitioning requirements;
- calls through concrete plans and `dyn ExecutionPlan`.
The following checks pass:
```shell
cargo fmt --all
cargo clippy --all-targets --all-features -- -D warnings
RUST_BACKTRACE=1 cargo test --profile ci \
--exclude datafusion-examples \
--exclude datafusion-benchmarks \
--exclude datafusion-cli \
--workspace --lib --tests --bins \
--features
avro,json,backtrace,extended_tests,recursive_protection,parquet_encryption
```
The code-size comparison and runtime measurements use the new
`invariant_check` Criterion benchmark and release builds of
`datafusion-physical-plan`.
## Are there any user-facing changes?
No.
--
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]
