Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Treewalker: Yield for each array element itself, not just children #303

Merged
merged 1 commit into from
Dec 23, 2023

Conversation

lfittl
Copy link
Member

@lfittl lfittl commented Dec 15, 2023

Due to an oversight in how the treewalker was implemented, we would never pass the direct array elements to the walk! block, only any children the elements have.

In practice this is usually not an issue, since the array elements are typically the generic PgQuery::Node object, and we would call the walk! function successfully on its children. However, working with those node objects directly is necessary when wanting to replace the node.

This may be a backwards incompatible change if one relies on the parent_field to always be a symbol (it can now be an integer as well).

Due to an oversight in how the treewalker was implemented, we would
never pass the direct array elements to the walk! block, only any
children the elements have.

In practice this is usually not an issue, since the array elements
are typically the generic PgQuery::Node object, and we would call the
walk! function successfully on its children. However, working with those
node objects directly is necessary when wanting to replace the node.

This may be a backwards incompatible change if one relies on the
parent_field to always be a symbol (it can now be an integer as well).
@lfittl lfittl requested a review from a team December 15, 2023 09:09
@lfittl lfittl merged commit 18682bd into main Dec 23, 2023
5 checks passed
@lfittl lfittl deleted the treewalker-walk-into-array-nodes branch January 1, 2024 20:00
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.

3 participants