Add support for Weak/Strong links aka Cascading deletes by cmelchior · Pull Request #622 · realm/realm-object-store

cmelchior · 2018-01-12T11:46:03Z

This PR exposes cores concept of Weak/Strong links in the Object Store schema using the Relationship::Strong parameter when creating link properties. This can be used to implement cascading deletes.

// New constructor 
Property(std::string name, PropertyType type, std::string object_type, Relationship relationship)

Semantics:

Default (and current Linktype) is Weak.
Changing the link type is a breaking change
Modifying the link type does not clean up the Realm automatically, i.e. changing from weak -> Strong will not automatically remove all objects not connected. This cleanup only happens if a links is actually updated.
It is possible to create child objects with no parents, these will not be deleted automatically. This only happens if the parent is created, the link is created, and then removed again.
Cascading behaviour is immediate inside a transaction. Rolling back the transaction will undo all deletes.
Some corner cases exist for cyclic object graphs, they are documented in core

Java PR using this is here: realm/realm-java#5678

tgoyne · 2018-01-12T18:42:06Z


 #include "util/tagged_bool.hpp"

+#include <string>


This include was previously in the correct place.

I'm not entirely sure why, but If this is not here I get:

In file included from /Users/cm/Realm/realm-object-store/src/property.hpp:24: /Users/cm/Realm/realm-object-store/CMakeFiles/realm-core-5.0.1/include/realm/data_type.hpp:27:9: error: unknown type name 'int64_t' typedef int64_t Int;

data_types.hpp needs to include <cstdint>

tgoyne · 2018-01-12T18:43:16Z


+    // Return the underlying LinkType. Only useful for Object or Array properties.
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wreturn-type"


Giving Relationship a backing type of bool eliminates the need for this.

tgoyne · 2018-01-12T18:43:43Z

 {
 }

+inline Property::Property(std::string name, PropertyType type,


This constructor is redundant.

It is being used by tests and not having it results in:

Undefined symbols for architecture x86_64: "realm::Property::Property(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >, realm::PropertyType, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >)", referenced from: ____C_A_T_C_H____T_E_S_T____0() in list.cpp.o ____C_A_T_C_H____T_E_S_T____3() in migrations.cpp.o ____C_A_T_C_H____T_E_S_T____111() in migrations.cpp.o ____C_A_T_C_H____T_E_S_T____134() in migrations.cpp.o ____C_A_T_C_H____T_E_S_T____0() in object.cpp.o ____C_A_T_C_H____T_E_S_T____3() in object_store.cpp.o ____C_A_T_C_H____T_E_S_T____102() in results.cpp.o

It is functionally identical to the three-argument version with a default value for the third argument (i.e. what previously existed).

Those was changed as well, these constructors have no default values anymore as having two 3-argument constructors with different defaults is ambiguous. So there is now a constructor for creating backlinks (with a string default value) and one for creating normal links (with a relationship default value)

Having Property(std::string name, PropertyType type, std::string object_type, std::string link_origin_property_name=""); and Property(std::string name, PropertyType type, std::string object_type, Relationship relationship); is not ambiguous.

Ah, like that. No, your right.

2-arg constructor has been removed

tgoyne · 2018-01-12T18:44:03Z

+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wreturn-type"
+    LinkType link_type() const {
+        switch(relationship) {


space between keyword and (

tgoyne · 2018-01-12T18:44:12Z

        else if (current_prop.requires_index()) {
            changes.emplace_back(schema_change::RemoveIndex{&existing_schema, &current_prop});
        }
+        if(target_prop->relationship != current_prop.relationship) {


space between keyword and (

tgoyne · 2018-01-12T18:45:10Z

+// See Descriptor::set_link_type() in Core (descriptor.hpp) for an extended explanation of the exact semantics.
+
+namespace realm {
+class TestHelper {


There's no reason to put these functions in a class.

tgoyne · 2018-01-12T18:47:11Z

@@ -0,0 +1,303 @@
+////////////////////////////////////////////////////////////////////////////


This entire file appears to just be testing core functionality, and should be part of core's test suite instead.

tgoyne · 2018-01-12T18:48:02Z

    return schema;
 }

+Schema set_relationship(Schema schema, StringData object_name, StringData property_name, Relationship new_relationship_type) {


newline before {

cmelchior · 2018-02-16T11:18:02Z

All comments addressed

cmelchior · 2018-05-01T07:44:29Z

Currently, Sync will crash with Unsupported operation if you try to change the link type, so merging this PR is probably not a good idea until that has been fixed. Apparently, that will require a protocol upgrade.

Zhuinden · 2018-06-17T18:33:07Z

Destructive changes always caused "bad changeset received", how is this any different?

tgoyne · 2018-06-17T18:53:44Z

The problem is not that you cannot make existing links strong, but rather that sync does not support strong links at all.

nirinchev · 2020-10-14T09:19:36Z

Cascading deletes are now (somewhat) supported via embedded objects and it's unlikely sync will ever support strong links, so closing this.

cmelchior added 8 commits January 11, 2018 09:07

Add support for link types in properties and migrations

f3a10e3

Fix compiler issues

2c871e1

Fix unit test + cleanup

5012339

Add migration unit tests

f5d05c2

Add unit tests

6bdbfdb

Disallow setting LinkType on Backlinks.

96f89f1

Merge branch 'master' into cm/support-link-type

8a2a2fc

Support GCC

5616f81

realm-ci assigned cmelchior Jan 12, 2018

tgoyne reviewed Jan 12, 2018

View reviewed changes

PR feedback

c78dd01

cmelchior mentioned this pull request Jan 26, 2018

Better LinkType support realm/realm-core#2987

Merged

karwa mentioned this pull request Feb 15, 2018

Add support for cascading deletes realm/realm-swift#1186

Open

cmelchior added 2 commits February 16, 2018 11:52

Merge branch 'master' into cm/support-link-type

dabdcb7

Remove redundant constructor

8cdbb59

cmelchior added 3 commits April 30, 2018 10:52

Merge branch 'master' into cm/support-link-type

0aa34df

Use core helper function

8a7fb06

Fix test

ccf2d9b

Zhuinden mentioned this pull request Jun 16, 2018

Add support for Weak/Strong links aka. Cascading Deletes realm/realm-java#5678

Draft

4 tasks

bmunkholm added the Pipeline-On-Hold label Nov 26, 2019

nirinchev closed this Oct 14, 2020

nirinchev deleted the cm/support-link-type branch October 14, 2020 09:19

		@@ -0,0 +1,303 @@
		////////////////////////////////////////////////////////////////////////////

Conversation

cmelchior commented Jan 12, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tgoyne Jan 12, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cmelchior commented Feb 16, 2018

Uh oh!

cmelchior commented May 1, 2018

Uh oh!

Zhuinden commented Jun 17, 2018

Uh oh!

tgoyne commented Jun 17, 2018

Uh oh!

nirinchev commented Oct 14, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

cmelchior commented Jan 12, 2018 •

edited

Loading

tgoyne Jan 12, 2018 •

edited

Loading