bvolpato commented on code in PR #25831:
URL: https://github.com/apache/datafusion/pull/25831#discussion_r4240009207
##########
datafusion/proto/tests/cases/stack_safety.rs:
##########
@@ -47,12 +50,26 @@ fn deeply_aliased_plan(alias_depth: usize) -> LogicalPlan {
plan
}
-fn serialize_on_two_mib_stack(alias_depth: usize) {
+fn serde_on_two_mib_stack(alias_depth: usize) {
let plan = deeply_aliased_plan(alias_depth);
std::thread::Builder::new()
.name("two-megabyte-stack".into())
.stack_size(2 * 1024 * 1024)
- .spawn(move || logical_plan_to_bytes(&plan).unwrap())
+ .spawn(move || {
+ let bytes = logical_plan_to_bytes(&plan).unwrap();
+ match logical_plan_from_bytes(&bytes, &TaskContext::default()) {
+ Ok(_) => {}
+ Err(err)
+ if err.to_string().contains("recursion limit")
Review Comment:
nonblocking: the deep cases hit Prost's recursion limit before reaching
`try_into_logical_plan`, so they still pass with the deserialization fix
removed. I checked reconstruction directly on a 2 MiB stack: it succeeds with
this change and stack-overflows on the base. Just noting that these cases don't
exercise deep reconstruction.
--
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]
