Skip to content

Commit

Permalink
Use Columns<Link> when property is Dictionary of links (#6705)
Browse files Browse the repository at this point in the history
If a Dictionary property has links as value type, we can use Columns<Link> to handle
the links instead of the basic Columns<Dictionary>. This has the effect that when we
compare with a single value, we will optimize to use LinksToNode. So we need to make
LinksToNode handle the Dictionary case.

When we compare with a list of links, we must ensure that the list is converted to
a list obj ObjKeys - which is the type that Column<Link> evaluates to.

 Use LinksToNode for lists in QueryParser
  • Loading branch information
jedelbo authored Jun 14, 2023
1 parent d2c4e10 commit d861035
Show file tree
Hide file tree
Showing 8 changed files with 219 additions and 89 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
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)
* Fix SessionWrapper use-after-free crash when tearing down sessions when using session multiplexing ([#6656](https://github.com/realm/realm-core/issues/6656), since v13.9.3)

### Breaking changes
Expand Down
133 changes: 81 additions & 52 deletions src/realm/parser/driver.cpp
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;
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
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
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
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()) {
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

0 comments on commit d861035

Please sign in to comment.