ninglinLiu opened a new pull request, #26741: URL: https://github.com/apache/datafusion/pull/26741
## Which issue does this PR close? - Closes #26738. ## Rationale for this change Converting a physical plan or physical expression to and from protobuf recurses once per nesting level, but unlike logical plan serialization (#23823 / #24124) none of these functions had a `recursive` stack-growth checkpoint. A deeply nested physical plan or expression could therefore abort the process with a native stack overflow, even when `recursive_protection` is enabled. ## What changes are included in this PR? Adds `#[cfg_attr(feature = "recursive_protection", recursive::recursive)]` to the four recursive entry points of physical serde: - `PhysicalPlanNodeExt::try_from_physical_plan_with_converter` (plan serialization) - `PhysicalPlanNodeExt::try_into_physical_plan_with_context` (plan deserialization) - `serialize_physical_expr_with_converter` (expression serialization) - `parse_physical_expr_with_converter` (expression deserialization) Every child plan / child expression goes through these functions, including the default `PhysicalProtoConverterExtension` hooks, so this covers the whole recursion. The `recursive` dependency and `recursive_protection` feature already exist in `datafusion-proto`, so there are no Cargo changes. ## What is the testing strategy for this PR? Added two tests to `datafusion/proto/tests/cases/stack_safety.rs` (gated on `recursive_protection`, like the existing growable-stack test) that round-trip, on a 2 MiB thread stack, a plan of 2,000 nested `GlobalLimitExec`s and an expression of 2,000 nested `NotExpr`s. They reuse the existing child-process harness (generalised into `run_in_child_with`) so a regression shows up as a test failure instead of aborting the test binary. Without the source change both new tests fail with `thread 'two-megabyte-stack' has overflowed its stack`; with it they pass: ``` cargo test -p datafusion-proto --features recursive_protection --test proto_integration stack_safety ``` `cargo clippy -p datafusion-proto --all-targets --features recursive_protection -- -D warnings` and `cargo fmt` are clean. ## Are there any user-facing changes? No API changes. With `recursive_protection` enabled, deeply nested physical plans and expressions now serialize/deserialize instead of overflowing the stack. --- This PR was prepared with AI assistance and reviewed/tested by the author. -- 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]
