Skip to content

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

Merged
oshadmi merged 6 commits intogeneric_json_pathfrom
omer-multipath-api
Oct 28, 2021
Merged

Changes needed for ReJSON multi values and multi paths #477#18
oshadmi merged 6 commits intogeneric_json_pathfrom
omer-multipath-api

Conversation

@oshadmi
Copy link
Copy Markdown

@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

Comment thread src/select/mod.rs Outdated
}

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

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
Copy Markdown
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

Copy link
Copy Markdown

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
Copy Markdown

@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
Copy Markdown
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