-
Notifications
You must be signed in to change notification settings - Fork 168
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
Expose Obj::get_parent_object
in the C API
#5851
Changes from 4 commits
47ff44b
588d5be
cabfa9c
7e8d9ce
930a9d4
0977db3
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -244,11 +244,15 @@ RLM_API realm_query_t* realm_query_parse_for_list(const realm_list_t* list, cons | |
const realm_query_arg_t* args) | ||
{ | ||
return wrap_err([&]() { | ||
auto existing_query = list->get_query(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I guess this is part of another PR There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. oh yes sorry, I have multiple small PRs open... my bad. let me revert this.. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Code has been reverted |
||
auto realm = list->get_realm(); | ||
auto table = list->get_table(); | ||
auto query = parse_and_apply_query(realm, table, query_string, num_args, args); | ||
auto ordering = query.get_ordering(); | ||
return new realm_query_t{std::move(query), std::move(ordering), realm}; | ||
auto combined = existing_query.and_query(query); | ||
auto ordering_copy = util::make_bind<DescriptorOrdering>(); | ||
if (auto ordering = query.get_ordering()) | ||
ordering_copy->append(*ordering); | ||
return new realm_query_t{std::move(query), std::move(ordering_copy), realm}; | ||
}); | ||
} | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've seen the C API littered with these types of checks and I'm no sure I understand their purpose. Not supplying a pointer to store the parent in sounds like a fundamental error in using the API. I'd rather have this be an assert than the method silently not doing anything.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree, but it is important we keep the same pattern, I can clean all this up in a separate PR.