Skip to content

Update StatementParser::run_walk to not recurse twice#1105

Merged
sgrif merged 1 commit into
mainfrom
sg-update-recursion
Jun 26, 2026
Merged

Update StatementParser::run_walk to not recurse twice#1105
sgrif merged 1 commit into
mainfrom
sg-update-recursion

Conversation

@sgrif

@sgrif sgrif commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

I changed the behavior of the parser to not pass the root node to the callback. I found recursing based on pointer equality to be too painful as I work on porting the routing functions.

As a side effect, this should be more efficient. We did a full AST walk of each subselect looking for advisory locks, and then walked it again looking for tables. With this change they're now a single walk, and the code should be easier to follow

I changed the behavior of the parser to not pass the root node to the
callback. I found recursing based on pointer equality to be too painful
as I work on porting the routing functions.

As a side effect, this should be more efficient. We did a full AST walk
of each subselect looking for advisory locks, and then walked it again
looking for tables. With this change they're now a single walk, and the
code should be easier to follow
@sgrif sgrif requested a review from levkk June 25, 2026 22:16
@sgrif sgrif marked this pull request as ready for review June 25, 2026 22:16

@levkk levkk left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

fancy!

@codecov

codecov Bot commented Jun 25, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@sgrif sgrif merged commit b7fa71b into main Jun 26, 2026
24 checks passed
@sgrif sgrif deleted the sg-update-recursion branch June 26, 2026 00:10
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