Skip to content

Changes needed for ReJSON multi values and multi paths #477 #18

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

Merged
merged 6 commits into from
Oct 28, 2021

Conversation

oshadmi
Copy link

@oshadmi oshadmi commented Oct 20, 2021

Needed for ReJSON multi values and multi paths #477

This change is needed to also get the values, in addition to the paths, from compute_paths

&mut tokens,
&mut visited,
&mut visited_order,
);
}

//No path was computed for missing targets
result.clear();

Choose a reason for hiding this comment

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

Why the clear is needed?

Copy link
Author

Choose a reason for hiding this comment

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

If some value, initially in the result vector, is not visited by _walk and not removed by retain, and does not have a corresponding path in the return value, we also don't want it in the final result (and we also want both returned vectors to have the same length)
This could happen if such an initial value is not a descendant of any node of self.
Actually compute_paths is not a public, so we can assume it should not happen,
So I can remove the clear

Choose a reason for hiding this comment

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

If it can not happen we can assert it no?

Copy link

@MeirShpilraien MeirShpilraien left a comment

Choose a reason for hiding this comment

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

Small question, by the way, I think it's OK to clone the result vector instead of doing this trick, after all, you are basically doing what clone would have done, no?

@oshadmi
Copy link
Author

oshadmi commented Oct 25, 2021

Small question, by the way, I think it's OK to clone the result vector instead of doing this trick, after all, you are basically doing what clone would have done, no?

Actually, this is required for the sake of getting the returned paths aligned with the returned values, so they have corresponding positions in each vector. Otherwise, the initial order of the result vector might not match the final order of the returned paths.

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