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

Fix backlink check in migration. #4414

Merged
merged 23 commits into from
Feb 17, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

### Fixed
* <How to hit and notice issue? what was the impact?> ([#????](https://github.com/realm/realm-core/issues/????), since v?.?.?)
* None.
* Fixed a bug that prevented an ObjectSchema with incoming links from being marked as embedded during migrations. ([#4414](https://github.com/realm/realm-core/pull/4414))

### Breaking changes
* None.
Expand Down
22 changes: 7 additions & 15 deletions src/realm/object-store/object_store.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -563,16 +563,6 @@ static void apply_non_migration_changes(Group& group, std::vector<SchemaChange>
verify_no_errors<SchemaMismatchException>(applier, changes);
}

static void set_embedded(Table& table, ObjectSchema::IsEmbedded is_embedded)
{
if (!table.set_embedded(is_embedded) && is_embedded) {
auto msg =
util::format("Cannot convert object type '%1' to embedded because objects have multiple incoming links.",
ObjectStore::object_type_for_table_name(table.get_name()));
throw std::logic_error(std::move(msg));
}
}

static void set_primary_key(Table& table, const Property* property)
{
ColKey col;
Expand Down Expand Up @@ -612,7 +602,7 @@ static void create_initial_tables(Group& group, std::vector<SchemaChange> const&
// downside.
void operator()(ChangeTableType op)
{
set_embedded(table(op.object), op.object->is_embedded);
table(op.object).set_embedded(op.object->is_embedded);
}
void operator()(AddProperty op)
{
Expand Down Expand Up @@ -736,9 +726,8 @@ static void apply_pre_migration_changes(Group& group, std::vector<SchemaChange>
create_table(group, *op.object);
}
void operator()(RemoveTable) {}
void operator()(ChangeTableType op)
{
set_embedded(table(op.object), op.object->is_embedded);
void operator()(ChangeTableType)
{ /* delayed until after the migration */
}
void operator()(AddInitialProperties op)
{
Expand Down Expand Up @@ -840,7 +829,10 @@ static void apply_post_migration_changes(Group& group, std::vector<SchemaChange>
table(op.object).remove_search_index(op.property->column_key);
}

void operator()(ChangeTableType) {}
void operator()(ChangeTableType op)
{
table(op.object).set_embedded(op.object->is_embedded);
}
void operator()(RemoveTable) {}
void operator()(ChangePropertyType) {}
void operator()(MakePropertyNullable) {}
Expand Down
51 changes: 30 additions & 21 deletions src/realm/table.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1029,42 +1029,51 @@ void Table::do_erase_root_column(ColKey col_key)
bump_storage_version();
}

bool Table::set_embedded(bool embedded)
void Table::set_embedded(bool embedded)
{
if (embedded == m_is_embedded)
return true;
if (embedded == m_is_embedded) {
return;
}

if (Replication* repl = get_repl()) {
if (repl->get_history_type() == Replication::HistoryType::hist_SyncClient) {
throw std::logic_error("Cannot change embedded property in sync client");
throw std::logic_error("Cannot change table to embedded when using Sync.");
}
}

if (embedded == false) {
do_set_embedded(false);
return;
}

// Embedded objects cannot have a primary key.
if (get_primary_key_column()) {
return false;
throw std::logic_error("Cannot change table to embedded when using a primary key.");
}
if (size() > 0) {
// Check if the table has any backlink columns. If not, it is not required
// to check all objects for backlinks.
bool has_backlink_columns = false;
for_each_backlink_column([&has_backlink_columns](ColKey) {
has_backlink_columns = true;
return true; // Done
});

DominicFrei marked this conversation as resolved.
Show resolved Hide resolved
if (has_backlink_columns) {
for (auto o : *this) {
// each object should be owned by one and only one parent
if (o.get_backlink_count() != 1) {
return false;
}
// `has_backlink_columns` indicates if the table is embedded in any other table.
bool has_backlink_columns = false;
for_each_backlink_column([&has_backlink_columns](ColKey) {
has_backlink_columns = true;
return true;
});
if (!has_backlink_columns) {
throw std::logic_error("Cannot change table to embedded without backlink columns. Table must be embedded in "
"at least one other table.");
}
else if (size() > 0) {
for (auto object : *this) {
size_t backlink_count = object.get_backlink_count();
if (backlink_count == 0) {
throw std::logic_error("At least one object does not have a backlink (data would get lost).");
}
else if (backlink_count > 1) {
throw std::logic_error("At least one object does have multiple backlinks.");
}
}
}

do_set_embedded(embedded);

return true;
}

void Table::do_set_embedded(bool embedded)
Expand Down
5 changes: 2 additions & 3 deletions src/realm/table.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -161,9 +161,8 @@ class Table {
bool valid_column(ColKey col_key) const noexcept;
void check_column(ColKey col_key) const;
// Change the embedded property of a table. If switching to being embedded, the table must
// not have a primary key and all objects must have exactly 1 backlink. Return value
// indicates if the conversion was done
bool set_embedded(bool embedded);
// not have a primary key and all objects must have exactly 1 backlink.
void set_embedded(bool embedded);
//@}

/// True for `col_type_Link` and `col_type_LinkList`.
Expand Down
Loading