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

Implement json_set #878

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

mkanilsson
Copy link

@mkanilsson mkanilsson commented Feb 3, 2025

This PR adds support for json_set.

There are three helper functions added:

  1. json_path_from_owned_value, this function turns an OwnedValue into a JsonPath.
  2. find_or_create_target, this function is similar to find_target with the added bonus of creating the target if it doesn't exist. There is a caveat with this function and that is that it will create objects/arrays as it goes, meaning if you send {} into it and try getting the path $.some.nested.array[123].field, it will return {"some":{"nested":array:[]}} since creation of some, nested and array will succeed, but accessing element 123 will fail.
  3. create_and_mutate_json_by_path, this function is very similar to mutate_json_by_path but calls find_or_create_target instead of find_target

Related to #127


Related questions: (This is my first PR in this project :) )
How do I test for errors in testing/*.test files? I added {{Runtime errror: ...}} and it worked fine when running make test but it failed in CI?
How do I run all the CI on my machine? Just so I don't have to force-push as much, I did cargo clippy --fix but it gave changes in unrelated files.

Also, sorry for the messy Github log, I opened this PR as a draft and wrote some questions, before making it into a real PR. And while waiting for CI to finish I discovered a couple of edge cases that didn't work like sqlite did, all of those have been fixed now and test cases have been added. Some day I'll get the hang of the workflow and I'll be more thorough in the future!

@mkanilsson mkanilsson force-pushed the feature/json_set branch 3 times, most recently from 1223232 to 456ae94 Compare February 4, 2025 17:05
@mkanilsson mkanilsson marked this pull request as ready for review February 4, 2025 17:14
@mkanilsson mkanilsson marked this pull request as draft February 4, 2025 17:31
@mkanilsson mkanilsson force-pushed the feature/json_set branch 2 times, most recently from 6fcf8ef to a8e3ae7 Compare February 4, 2025 17:42
Test cases are included.
Related to tursodatabase#127
@mkanilsson mkanilsson marked this pull request as ready for review February 4, 2025 18:21
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.

1 participant