Skip to content

Conversation

niebayes
Copy link
Contributor

@niebayes niebayes commented Oct 15, 2025

In our scenario, we set Break a custom Error type, with its size being around 100 bytes. This caused our stack space to be rapidly consumed during visitor recursion, and for more complex SQL queries, it even led to stack overflow.
We solved this problem by boxing the Error.

/// node and `post_visit_` methods are invoked after visiting all
/// children of the node.
///
/// Important note: The `Break` type should be kept as small as possible to prevent
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could we place the note on the documentation of Break instead? here for example

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, moved the note.

/// node and `post_visit_` methods are invoked after visiting all
/// children of the node.
///
/// Important note: The `Break` type should be kept as small as possible to prevent
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could we place the note on the documentation of Break instead? here for example

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

moved

@iffyio iffyio force-pushed the chore/visitor_caution branch from 03a2dc3 to d92528c Compare October 16, 2025 08:28
Copy link
Contributor

@iffyio iffyio left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @niebayes!

@iffyio iffyio added this pull request to the merge queue Oct 16, 2025
Merged via the queue into apache:main with commit 218f43c Oct 16, 2025
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants