Add JSON.CLEAR command #306
Conversation
|
test:testClear - has tests for when multi-paths are handled - currently only the first is handled. |
|
test:testSetInvalidPathShouldFail - it turns out this test does not fails on master. Reviewing it. |
src/lib.rs
Outdated
| let mut paths: Vec<Path> = vec![]; | ||
| let path = backwards_compat_path(arg); | ||
| paths.push(Path::new(path)); | ||
| while let Ok(mut arg) = args.next_string() { |
There was a problem hiding this comment.
@oshadmi why do you need multi paths support? with the support of full JSONPath I don't see the need.
There was a problem hiding this comment.
@gkorland Do you mean that since multi paths could be expressed using a single JSONPath with wildcard, union iterator, recursive descent or slices?
It could still be useful to allow specifying several paths, either when they are plain/explicit, or when a complex path, with wildcards, etc., might yield unwanted paths, e.g., $..attr is too "permissive" when user only wants 2 specific paths out of many others, so $.sub1.attr and $.sub2.attr could be easily specified, while finding the exact single complex path could be harder.
There was a problem hiding this comment.
yes single "path" can be a union of multi paths.
And it's consistent with: JSON.DEL <key> [path]
There was a problem hiding this comment.
@gkorland
The current doc specifies one or more path args only for JSON.GET (using three dots)
JSON.GET <key>
[INDENT indentation-string]
[NEWLINE line-break-string]
[SPACE space-string]
[NOESCAPE]
[path ...]
But for JSON.DEL only a single path is specified:
JSON.DEL <key> [path]
For JSON.CLEAR we aimed to eventually support multi-paths, right?
So it would eventually be documented with three dots as:
JSON.CLEAR <key> [path ...]
With the return value being the number of paths cleared, similar to JSON.DEL.
Additional question (also related to single path which could use wildcards, union iterator, etc. to match several elements):
What should be done if one or more paths do not exist or do not resolve to an array or an object:
- Should the command be atomic, and avoid doing just part of the job? Should it then return the error messages reporting
the wrong paths, or just do nothing or return nil?
Or - Should the command report on erroneous path and resume with the rest of the paths, clearing only existing and matching arrays/objects and skipping the rest? (if the command is not atomic, the return value would be an array of numbers and/or error messages?)
* Update to Redis 6.2.2 (#332) * Fix json.arrappend performance (#324) * improve arr_pop, arr_insert & arr_trim performance * refactor do_json_* functions * normalize index for backward (#327) * normalize index for backward * adpat for full backward with 1.0.x * fix #318 (#333) * CircleCI: macOS and benchmark fixes (#337) * Update config.yml (#338) * Add JSON.CLEAR command (#306) * Restore optional path (default is $) for JSON.DEL * Add JSON.CLEAR * Handle JSON.CLEAR with $ path * Add JSON.CLEAR - Fix failure in test:testSetInvalidPathShouldFail * Idiomatic fixes per Gavrie review Co-authored-by: Guy Korland <gkorland@gmail.com> * Update RS_VERSIONS last promoted builds * Document flags for running tests * fix arr_pop backward to 1.0 (#345) * JSON.INSERTARR backward compatability for 1.0 (#343) * Fix JSON.INSERTARR backward compatibility with 1.0.x * Update test.py * Fix build error (use of undeclared crate or module `rawmod`) * Fix format Co-authored-by: Guy Korland <gkorland@gmail.com> Co-authored-by: Omer Shadmi <76992134+oshadmi@users.noreply.github.com> Co-authored-by: tomerhekredis <72793005+tomerhekredis@users.noreply.github.com> Co-authored-by: Gavrie Philipson <gavrie@philipson.co.il> Co-authored-by: oshadmi <omer.shadmi@redislabs.com>
Currently only handling the first
pathargument