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

Add mutate method to jsonpath #2606

Merged
merged 2 commits into from
Feb 18, 2024
Merged

Add mutate method to jsonpath #2606

merged 2 commits into from
Feb 18, 2024

Conversation

romange
Copy link
Collaborator

@romange romange commented Feb 17, 2024

Plug it in into OpToggle op.

BorysTheDev
BorysTheDev previously approved these changes Feb 18, 2024
Comment on lines 352 to 353
bool erase = Mutate(cb, it->key(), &it->value());
if (erase) {
Copy link
Contributor

Choose a reason for hiding this comment

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

nit:
if (Mutate(cb, it->key(), &it->value())) {

src/core/json/path.cc Outdated Show resolved Hide resolved
src/core/json/path.cc Outdated Show resolved Hide resolved
Comment on lines 386 to 390
if (res) {
it = node->erase(it);
} else {
++it;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

it = res ? node->erase(it) : it + 1;

Comment on lines +591 to +617
auto cb = [&vec](optional<string_view>, JsonType* val) {
if (val->is_bool()) {
bool next_val = val->as_bool() ^ true;
*val = next_val;
vec.emplace_back(next_val);
} else {
vec.emplace_back(nullopt);
}
return false;
};
json::MutatePath(expr, std::move(cb), pv.GetJson());
it_res->post_updater.Run();
op_args.shard->search_indices()->AddDoc(key, op_args.db_cntx, pv);
} else {
auto cb = [&vec](const auto&, JsonType& val) {
if (val.is_bool()) {
bool current_val = val.as_bool() ^ true;
val = current_val;
vec.emplace_back(current_val);
} else {
vec.emplace_back(nullopt);
}
};
Copy link
Contributor

Choose a reason for hiding this comment

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

cb are the same but pointer type, I think you can implement one callback via another or do a pointer dereference with constexpr if

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

it's not worth it since it's a transitional code and once we are confident it's robust we will delete the old one

Copy link
Contributor

Choose a reason for hiding this comment

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

any case I have approved it

BorysTheDev
BorysTheDev previously approved these changes Feb 18, 2024
Plug it in into OpToggle op.

Signed-off-by: Roman Gershman <roman@dragonflydb.io>
Signed-off-by: Roman Gershman <roman@dragonflydb.io>
@romange romange merged commit 750f039 into main Feb 18, 2024
10 checks passed
@romange romange deleted the Pr1 branch February 18, 2024 14:06
lsvmello pushed a commit to lsvmello/dragonfly that referenced this pull request Feb 19, 2024
* Add mutate method to jsonpath

Plug it in into OpToggle op.

---------

Signed-off-by: Roman Gershman <roman@dragonflydb.io>
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.

None yet

2 participants