chore: Del and NUMINCRBY use json::Path #2655
Conversation
romange
force-pushed
the
Pr1
branch
2 times, most recently
from
27f9921
to
e71c576
Compare
Also, fix various protocol bugs when we sent simple string instead of sending bulk strings. Fixed a typo in path.cc that lead to a data race bug. Finally, flip the flag in regression tests to start covering json::Path code. Signed-off-by: Roman Gershman <roman@dragonflydb.io>
| Dfs().Mutate(path, callback, json); | ||
| return 0; | ||
| Dfs dfs; | ||
| dfs.Mutate(path, callback, json); |
There was a problem hiding this comment.
maybe we can create static method Mutate that return Dfs object and we don't need to create the object before
There was a problem hiding this comment.
I do not see a problem with this code.
src/server/json_family.cc
Outdated
| enum ArithmeticOpType { OP_ADD, OP_MULTIPLY }; | ||
|
|
||
| // Returns true in case of overflow | ||
| bool BinOpApply(double num, bool num_is_double, ArithmeticOpType op, JsonType* val) { |
There was a problem hiding this comment.
I would prefer to see result JsonType and overflow flag like out parameter, current implementation looks awkward
src/server/json_family.cc
Outdated
| auto cb = [&](optional<string_view>, JsonType* val) { | ||
| if (val->is_number()) { | ||
| bool res = BinOpApply(num, has_fractional_part, op_type, val); | ||
| if (res) { |
There was a problem hiding this comment.
can we assign is_result_overflow in the if? and maybe remove res variable
There was a problem hiding this comment.
we can not since the callback may be called several times and we should not reset is_result_overflow if has been set at least once.
Signed-off-by: Roman Gershman <roman@dragonflydb.io>
|
@dranikpg I was in the middle of writing you that I am surprised myself how come |
|
@BorysTheDev Added the test for incrementing |
Also, fix various protocol bug when we sent simple string instead of sending bulk strings.