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.

Sign up for GitHub

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

Use Columns<Link> when property is Dictionary of links #6705

Merged
merged 7 commits into from
Jun 14, 2023
Merged

Use Columns<Link> when property is Dictionary of links #6705

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
adfd6d8
Use Columns<Link> when property is Dictionary if links
jedelbo Jun 8, 2023
7a33c49
Review update
jedelbo Jun 9, 2023
3a1cad3
Use LinksToNode for lists in QueryParser
jedelbo Jun 9, 2023
54eff5e
Review update 2
jedelbo Jun 12, 2023
f9307b1
Merge branch 'master' into je/column-links
jedelbo Jun 12, 2023
5de3e5b
Review update 3
jedelbo Jun 13, 2023
330189d
Merge branch 'master' into je/column-links
jedelbo Jun 14, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

### Fixed
* <How do the end-user experience this issue? what was the impact?> ([#????](https://github.com/realm/realm-core/issues/????), since v?.?.?)
* Querying with object list arguments does not work. ([#6688](https://github.com/realm/realm-core/issues/6688), since v10.3.3)
* None.

### Breaking changes
Expand Down
133 changes: 81 additions & 52 deletions src/realm/parser/driver.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -435,37 +435,59 @@ Query EqualityNode::visit(ParserDriver* drv)
auto left_type = left->get_type();
auto right_type = right->get_type();

auto handle_typed_link = [drv](std::unique_ptr<Subexpr>& list, std::unique_ptr<Subexpr>& value, DataType& type) {
auto handle_typed_links = [drv](std::unique_ptr<Subexpr>& list, std::unique_ptr<Subexpr>& expr, DataType& type) {
if (auto link_column = dynamic_cast<const Columns<Link>*>(list.get())) {
if (value->get_mixed().is_null()) {
type = ColumnTypeTraits<realm::null>::id;
value = std::make_unique<Value<realm::null>>();
}
else {
auto left_dest_table_key = link_column->link_map().get_target_table()->get_key();
auto right_table_key = value->get_mixed().get_link().get_table_key();
auto right_obj_key = value->get_mixed().get_link().get_obj_key();
if (left_dest_table_key == right_table_key) {
value = std::make_unique<Value<ObjKey>>(right_obj_key);
type = type_Link;
}
else {
const Group* g = drv->m_base_table->get_parent_group();
throw InvalidQueryArgError(util::format(
"The relationship '%1' which links to type '%2' cannot be compared to an argument of type %3",
link_column->link_map().description(drv->m_serializer_state),
link_column->link_map().get_target_table()->get_class_name(),
print_pretty_objlink(value->get_mixed().get_link(), g)));
// Change all TypedLink values to ObjKey values
auto value = dynamic_cast<ValueBase*>(expr.get());
auto left_dest_table_key = link_column->link_map().get_target_table()->get_key();
auto sz = value->size();
auto obj_keys = std::make_unique<Value<ObjKey>>();
obj_keys->init(expr->has_multiple_values(), sz);
obj_keys->set_comparison_type(expr->get_comparison_type());
for (size_t i = 0; i < sz; i++) {
auto val = value->get(i);
// i'th entry is already NULL
if (!val.is_null()) {
TableKey right_table_key;
ObjKey right_obj_key;
if (val.is_type(type_Link)) {
right_table_key = left_dest_table_key;
right_obj_key = val.get<ObjKey>();
}
else if (val.is_type(type_TypedLink)) {
right_table_key = val.get_link().get_table_key();
right_obj_key = val.get_link().get_obj_key();
}
else {
const char* target_type = get_data_type_name(val.get_type());
throw InvalidQueryError(
util::format("Unsupported comparison between '%1' and type '%2'",
link_column->link_map().description(drv->m_serializer_state), target_type));
}
if (left_dest_table_key == right_table_key) {
obj_keys->set(i, right_obj_key);
}
else {
const Group* g = drv->m_base_table->get_parent_group();
throw InvalidQueryArgError(
util::format("The relationship '%1' which links to type '%2' cannot be compared to "
"an argument of type %3",
link_column->link_map().description(drv->m_serializer_state),
link_column->link_map().get_target_table()->get_class_name(),
print_pretty_objlink(ObjLink(right_table_key, right_obj_key), g)));
}
}
}
expr = std::move(obj_keys);
type = type_Link;
}
};

if (left_type == type_Link && right_type == type_TypedLink && right->has_single_value()) {
handle_typed_link(left, right, right_type);
if (left_type == type_Link && right->has_constant_evaluation()) {
handle_typed_links(left, right, right_type);
}
if (right_type == type_Link && left_type == type_TypedLink && left->has_single_value()) {
handle_typed_link(right, left, left_type);
if (right_type == type_Link && left->has_constant_evaluation()) {
handle_typed_links(right, left, left_type);
}

if (left_type.is_valid() && right_type.is_valid() && !Mixed::data_types_are_comparable(left_type, right_type)) {
Expand All @@ -488,9 +510,33 @@ Query EqualityNode::visit(ParserDriver* drv)
}
}

const ObjPropertyBase* prop = dynamic_cast<const ObjPropertyBase*>(left.get());
if (right->has_single_value() && (left_type == right_type || left_type == type_Mixed)) {
if (left_type == type_Link && left_type == right_type && right->has_constant_evaluation()) {
if (auto link_column = dynamic_cast<const Columns<Link>*>(left.get())) {
if (link_column->link_map().get_nb_hops() == 1 &&
link_column->get_comparison_type().value_or(ExpressionComparisonType::Any) ==
ExpressionComparisonType::Any) {
REALM_ASSERT(dynamic_cast<const Value<ObjKey>*>(right.get()));
auto link_values = static_cast<const Value<ObjKey>*>(right.get());
// We can use a LinksToNode based query
std::vector<ObjKey> values;
Copy link
Member

Choose a reason for hiding this comment

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

I don't know if it makes sense to do it here, but we should know the size of the values to insert in the list of obj keys, so we could do something like: values.reserve(link_values.size());

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure

values.reserve(link_values->size());
for (auto val : *link_values) {
values.emplace_back(val.is_null() ? ObjKey() : val.get<ObjKey>());
}
if (op == CompareNode::EQUAL) {
return drv->m_base_table->where().links_to(link_column->link_map().get_first_column_key(),
values);
}
else if (op == CompareNode::NOT_EQUAL) {
return drv->m_base_table->where().not_links_to(link_column->link_map().get_first_column_key(),
values);
}
}
}
}
else if (right->has_single_value() && (left_type == right_type || left_type == type_Mixed)) {
Mixed val = right->get_mixed();
const ObjPropertyBase* prop = dynamic_cast<const ObjPropertyBase*>(left.get());
if (prop && !prop->links_exist()) {
auto col_key = prop->column_key();
if (val.is_null()) {
Expand Down Expand Up @@ -529,20 +575,6 @@ Query EqualityNode::visit(ParserDriver* drv)
break;
}
}
else if (left_type == type_Link) {
auto link_column = dynamic_cast<const Columns<Link>*>(left.get());
if (link_column && link_column->link_map().get_nb_hops() == 1 &&
link_column->get_comparison_type().value_or(ExpressionComparisonType::Any) ==
ExpressionComparisonType::Any) {
// We can use equal/not_equal and get a LinksToNode based query
if (op == CompareNode::EQUAL) {
return drv->m_base_table->where().equal(link_column->link_map().get_first_column_key(), val);
}
else if (op == CompareNode::NOT_EQUAL) {
return drv->m_base_table->where().not_equal(link_column->link_map().get_first_column_key(), val);
}
}
}
}
if (case_sensitive) {
switch (op) {
Expand Down Expand Up @@ -1783,11 +1815,17 @@ std::unique_ptr<Subexpr> LinkChain::column(const std::string& col)
}
}

auto col_type{col_key.get_type()};
if (Table::is_link_type(col_type)) {
add(col_key);
return create_subexpr<Link>(col_key);
}

if (col_key.is_dictionary()) {
return create_subexpr<Dictionary>(col_key);
}
else if (col_key.is_set()) {
switch (col_key.get_type()) {
switch (col_type) {
case col_type_Int:
return create_subexpr<Set<Int>>(col_key);
case col_type_Bool:
Expand All @@ -1810,15 +1848,12 @@ std::unique_ptr<Subexpr> LinkChain::column(const std::string& col)
return create_subexpr<Set<ObjectId>>(col_key);
case col_type_Mixed:
return create_subexpr<Set<Mixed>>(col_key);
case col_type_Link:
add(col_key);
return create_subexpr<Link>(col_key);
default:
break;
}
}
else if (col_key.is_list()) {
switch (col_key.get_type()) {
switch (col_type) {
case col_type_Int:
return create_subexpr<Lst<Int>>(col_key);
case col_type_Bool:
Expand All @@ -1841,9 +1876,6 @@ std::unique_ptr<Subexpr> LinkChain::column(const std::string& col)
return create_subexpr<Lst<ObjectId>>(col_key);
case col_type_Mixed:
return create_subexpr<Lst<Mixed>>(col_key);
case col_type_LinkList:
add(col_key);
return create_subexpr<Link>(col_key);
default:
break;
}
Expand All @@ -1854,7 +1886,7 @@ std::unique_ptr<Subexpr> LinkChain::column(const std::string& col)
expression_cmp_type_to_str(m_comparison_type)));
}

switch (col_key.get_type()) {
switch (col_type) {
case col_type_Int:
return create_subexpr<Int>(col_key);
case col_type_Bool:
Expand All @@ -1877,9 +1909,6 @@ std::unique_ptr<Subexpr> LinkChain::column(const std::string& col)
return create_subexpr<ObjectId>(col_key);
case col_type_Mixed:
return create_subexpr<Mixed>(col_key);
case col_type_Link:
add(col_key);
return create_subexpr<Link>(col_key);
default:
break;
}
Expand Down
6 changes: 6 additions & 0 deletions src/realm/query.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -546,6 +546,12 @@ Query& Query::links_to(ColKey origin_column, const std::vector<ObjKey>& target_k
return *this;
}

Query& Query::not_links_to(ColKey origin_column_key, const std::vector<ObjKey>& target_keys)
{
add_node(std::unique_ptr<ParentNode>(new LinksToNode<NotEqual>(origin_column_key, target_keys)));
return *this;
}

// int64 constant vs column
Query& Query::equal(ColKey column_key, int64_t value)
{
Expand Down
3 changes: 3 additions & 0 deletions src/realm/query.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,9 @@ class Query final {
// Find links that point to specific target objects
Query& links_to(ColKey column_key, const std::vector<ObjKey>& target_obj);

// Find links that does not point to specific target objects
Query& not_links_to(ColKey column_key, const std::vector<ObjKey>& target_obj);

// Conditions: null
Query& equal(ColKey column_key, null);
Query& not_equal(ColKey column_key, null);
Expand Down
86 changes: 66 additions & 20 deletions src/realm/query_engine.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -819,23 +819,43 @@ ExpressionNode::ExpressionNode(const ExpressionNode& from)
template <>
size_t LinksToNode<Equal>::find_first_local(size_t start, size_t end)
{
if (m_column_type == col_type_LinkList || m_condition_column_key.is_set()) {
// LinkLists never contain null
if (!m_target_keys[0] && m_target_keys.size() == 1 && start != end)
return not_found;

BPlusTree<ObjKey> links(m_table.unchecked_ptr()->get_alloc());
for (size_t i = start; i < end; i++) {
if (ref_type ref = get_ref(i)) {
links.init_from_ref(ref);
for (auto& key : m_target_keys) {
if (key) {
if (links.find_first(key) != not_found)
if (m_condition_column_key.is_collection()) {
Allocator& alloc = m_table.unchecked_ptr()->get_alloc();
if (m_condition_column_key.is_dictionary()) {
auto target_table_key = m_table->get_opposite_table(m_condition_column_key)->get_key();
Array top(alloc);
for (size_t i = start; i < end; i++) {
if (ref_type ref = get_ref(i)) {
top.init_from_ref(ref);
BPlusTree<Mixed> values(alloc);
values.set_parent(&top, 1);
values.init_from_parent();
for (auto& key : m_target_keys) {
ObjLink link(target_table_key, key);
if (values.find_first(link) != not_found)
return i;
}
}
}
}
else {
// LinkLists never contain null
if (!m_target_keys[0] && m_target_keys.size() == 1 && start != end)
return not_found;

BPlusTree<ObjKey> links(alloc);
for (size_t i = start; i < end; i++) {
if (ref_type ref = get_ref(i)) {
links.init_from_ref(ref);
for (auto& key : m_target_keys) {
if (key) {
if (links.find_first(key) != not_found)
return i;
}
}
}
}
}
}
else if (m_list) {
for (auto& key : m_target_keys) {
Expand All @@ -856,15 +876,41 @@ size_t LinksToNode<NotEqual>::find_first_local(size_t start, size_t end)
REALM_ASSERT(m_target_keys.size() == 1);
ObjKey key = m_target_keys[0];

if (m_column_type == col_type_LinkList || m_condition_column_key.is_set()) {
BPlusTree<ObjKey> links(m_table.unchecked_ptr()->get_alloc());
for (size_t i = start; i < end; i++) {
if (ref_type ref = get_ref(i)) {
links.init_from_ref(ref);
auto sz = links.size();
for (size_t j = 0; j < sz; j++) {
if (links.get(j) != key) {
if (m_condition_column_key.is_collection()) {
Copy link
Member

Choose a reason for hiding this comment

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

This code seems the same of the one that starts at line 822, maybe this can be encapsulated into some utility method.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It probably could, but I am not sure the code would be much nicer to read.

Allocator& alloc = m_table.unchecked_ptr()->get_alloc();
if (m_condition_column_key.is_dictionary()) {
auto target_table_key = m_table->get_opposite_table(m_condition_column_key)->get_key();
Array top(alloc);
for (size_t i = start; i < end; i++) {
if (ref_type ref = get_ref(i)) {
top.init_from_ref(ref);
BPlusTree<Mixed> values(alloc);
values.set_parent(&top, 1);
values.init_from_parent();

ObjLink link(target_table_key, key);
bool found = false;
values.for_all([&](const Mixed& val) {
if (val != link) {
found = true;
}
return !found;
});
if (found)
return i;
}
}
}
else {
BPlusTree<ObjKey> links(alloc);
for (size_t i = start; i < end; i++) {
if (ref_type ref = get_ref(i)) {
links.init_from_ref(ref);
auto sz = links.size();
for (size_t j = 0; j < sz; j++) {
if (links.get(j) != key) {
return i;
}
}
}
}
Expand Down
Loading