Model.create can't take id

[knoguchi]

I am trying to create an object with id. e.g. User.create(id=123, name='kenji'). However it raises exception. User.create(name='kenji') works, but then the id is auto generated uuid that is not desirable for my app. Why create can't take id?

[linkyndy]

id was not intended to be modifiable. I thought of remodel being as simple as possible, therefore I went with the default primary key field - 'id' - and the default values for the primary key - UUIDs.

If this feature is considered important, I will think of a way to implement it. But bare in mind, primary key values must be unique within a table; how did you plan to solve it? How do you guarantee that the ID you're assigning is unique?

[knoguchi]

In my app the id is generated by other system, and they guarantee the uniqueness. Rethinkdb .insert() takes conflict parameter which determines the handling of documents with conflicting primary key -- error, replace or update. Do you think you can add it to .save() method?

[knoguchi]

Oh wait a sec. I just read the code of the save method. There's update-or-insert logic already! So .create() should work with id.

After debugging a bit I noticed the line 76 does NOT raise exception when the id does not exist in the db. It still sets result['errors'] though. Therefore the except clause is skipped, and finally raises OperationError.

73        try:
74            # Attempt update
75            id_ = fields_dict['id']
76            result = (r.table(self._table).get(id_).replace(r.row
77                        .without(r.row.keys().difference(list(fields_dict.keys())))
78                        .merge(fields_dict), return_changes='always').run())
79
80        except KeyError:
81            # Resort to insert
82            result = (r.table(self._table).insert(fields_dict, return_changes=True)
83                      .run())
84
85        if result['errors'] > 0:
86            raise OperationError(result['first_error'])

Maybe you can replace the whole logic above with .insert(conflict="update") ?

[knoguchi]

This patch seems to work for me. But I'm not confident if this works as same as your update logic.

diff --git a/remodel/models.py b/remodel/models.py
index a06b51a..17214ad 100644
--- a/remodel/models.py
+++ b/remodel/models.py
@@ -70,17 +70,9 @@ class Model(object):
         self._run_callbacks('before_save')

         fields_dict = self.fields.as_dict()
-        try:
-            # Attempt update
-            id_ = fields_dict['id']
-            result = (r.table(self._table).get(id_).replace(r.row
-                        .without(r.row.keys().difference(list(fields_dict.keys())))
-                        .merge(fields_dict), return_changes='always').run())
-
-        except KeyError:
-            # Resort to insert
-            result = (r.table(self._table).insert(fields_dict, return_changes=True)
-                      .run())
+        # update or insert
+        result = (r.table(self._table).insert(fields_dict, return_changes=True, conflict="update")
+                  .run())

         if result['errors'] > 0:
             raise OperationError(result['first_error'])

[linkyndy]

Sorry for getting back so late on this. While your changes might work for you, I am reluctant of including this into remodel. Here are my thoughts:

• the current logic happening at update is a lot more involved: it checks whether there are fields that appear upstream but not in the local object, this resulting in the upstream fields being removed as well;
• conflict='update' won't work in this case since, as stated earlier, there might be fields that need to be removed from upstream; update just adds missing fields or updates existing ones;
• return_changes=True might not work either, since it may not return anything if an update is performed with nothing changed (see #23 for the previous issue we had);
• not least, I feel that opening too many parts for customization in the library will derive from remodel's initial goal of being very simple and easy to use.

These being said, if this doesn't get merged to develop (and then to master), you're always free to implement whatever you think is useful on a separate fork of remodel. Be sure to have it thoroughly tested and maybe if there are many requests on that it will be included one day in the library.

[snootywords]

Hi, I'm encountering this same issue with a use case for id I believe is reasonable as well. I have a model set up as follows:

class User(Model):
  has_many = ('Post', 'Vote')

class Post(Model):
  belongs_to = ('User',)
  has_many = ('Vote',)

# Keeps track of unique votes by a user on a post (a user can't vote more than once on a post)
class Vote(Model):
  belongs_to = ('Post', 'User')

I would then like to execute the following (assuming I created a sample user and post earlier): Vote.create(id=(sample_user['id']+'.'+sample_post['id']), user=sample_user, post=sample_post)

This ensures votes have a unique pair of user/post, and uses their IDs in its ID derivation. Searching if the vote pair exists first and then updating it I believe is not guaranteed to be atomic like this solution would be. Further, I think it's very simple/intuitive to the end user (at least me) to be able to specify the id at create time. Let me know if you have any questions or want more information - thanks!

[linkyndy]

On top of what I've mentioned above, allowing changes to an object's id might potentially corrupt other objects already loaded, since they will point to an existing id. Added to the fact that relationships also depend on this field, I'm afraid the changeset to make this happen will be too big for the actual gains. I would rather look into a way of declaring indexes or the like, to enforce constraints as in @snootywords's example.

[knoguchi]

wow update after almost 4 years. I'm no longer using Rethinkdb. Closing this issue.

[linkyndy]

@knoguchi, were you expecting an update on this? I've stated my opinions at the time.