fengyue-xve opened a new pull request, #2623: URL: https://github.com/apache/datafusion-sqlparser-rs/pull/2623
Printing a set-operation chain recurses once per branch through `SetExpr`'s `Display` (`left.fmt(f)`), so a long chain overflows the stack: a 25,000-branch `UNION ALL` chain on a 2 MB release stack, as reported in the review on #2621. `Expr`'s `Display` already carries `#[cfg_attr(feature = "recursive-protection", recursive::recursive)]`; `SetExpr`'s did not. This adds the same attribute. Follow-up to the review on #2621 (https://github.com/apache/datafusion-sqlparser-rs/pull/2621#pullrequestreview-5494013283): > A PR adding `recursive::recursive` to `SetExpr`'s `Display`, as `Expr` already has, would be welcome. Tests: `long_set_operation_chain_display_does_not_stack_overflow` parses a 25,000-branch `UNION ALL` chain and prints it on a thread with a 256 KiB stack, so the check does not depend on the platform's default test-thread stack. With the attribute removed, the test binary aborts with a stack overflow (`STATUS_STACK_OVERFLOW` on Windows); with it, the test passes. The parsed chain is torn down iteratively, because its recursive drop glue would overflow that small stack too — drop is the separate, pre-existing concern noted in the review. Dialects: none -- 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]
