Question about nested objects and .save_related()

[franzwollang]

I'm converting over from using SQLAlchemy ORM and I'm having some trouble with getting nested objects to save correctly.

I have models and relationships set up like so:
Vendor <-- Restaurant <-- Menu <-- MenuSection <-- MenuItem <-- Offer
Vendor <--> Employee
Order --> Vendor

In order to seed the database for testings purposes, I'm adding some new employees, a vendor, a restaurant, and some orders. I create the vendor object w/ nested objects in it, and the order object. Then I run the following code:

await vendor.save()

        for employee in vendor.employees: # adding employees into DB manually because M2M relationship 
                await employee.save()

await vendor.save_related(follow=True, save_all=True)
await order.save()

After executing this code, the employees, vendor, and order get added correctly (including relationships) but the restaurants, menus, etc... don't get added. If I manually go in and add a loop to call .save() on each of the restaurant objects first, then they will also get added correctly (including relationships) but then menus, menuSections, etc... don't get added. If I do the same manually for menus, then it works but menuSections, menuItems, etc... don't get added.

I feel like I'm missing something important about how .save_related() is supposed to work. Any ideas? I'm getting pretty frustrated trying to figure this out. Help would be much appreciated.

[collerek]

You don't provide a full code sample but I would assume you provide a primary keys for your deeply nested relations.

Save related is a shortcut function that executes upsert on each model. The insert/update operation is based on the presence of the primary keys. If the primary key is present ormar assumes it's an update (otherwise you would get an integrity error with duplicated primary key if in fact the given models already exists and you try to insert them again).

So if you want to create the full tree at once (so all models are not existing) the child models cannot have primary keys set (so should be autoincrement or columns with default). I can add something like force_insert flag but then it's on you to handle the integrity errors.

I want to avoid running a separate query to check if the model already exists for each instance as that would slow down the whole process substantiality.

[franzwollang]

I see. Thank you ten times over for your help, so quickly, and even on a Sunday evening! You are a programming saint. I briefly thought about the primary keys being set and if it would be a problem, but I didn't think about the concerns you noted. I will fix my problem by leaving the primary keys nulled, as you suggested.

I think a flag to force insert would be nice; I'm a fan of keeping the default behavior clean and safe, but giving people the option to shoot themselves in the foot if they want to. The extra flexibility can often make it easier to do specific things, as long as you know what you're doing.

While we're on the topic of nested models, there is a note in the documentation stating the inserts are done in many different SQL calls. What are the obstacles to making it possible to insert nested models in fewer (or even a single) SQL call? I would be willing to help work on this over the next few months if there is a possible path towards implementing this kind of optimization.

[franzwollang]

@collerek I removed the primary keys like you suggested, but it is still failing. I'm using non-integer primary keys with a default; upon inspection, it looks like the 'default' is applied to the empty primary key fields when the entities are instantiated, so when I then call .save_related() it ends up being the same as if I had just manually inserted the keys.

Is this an Ormar bug or am I still doing something wrong?

[collerek]

Yes, to keep it empty you would have to use server_default, pydantic is populating the default values on init.

That would require this force_insert flag if you know all of them are new. (you can open an issue for this).

BTW. Thank you for your sponsoring 😄

[franzwollang]

Ok, thank you for explaining.

My pleasure. Thank you for stepping up to create this project; there is no doubt that there is a lot of value in not having to maintain two models for every object when an API starts to get large, plus this further promotes higher cohesion by incentivizing one to write import/export processing code also in the single Ormar model using Pydantic pre-validators and decoders rather than as a separate translation layer between Pydantic and SQLAlchemy ORM models. I also like the way you are approaching the querying features in contrast to the SQLAlchemy ORM, which feels more rigid and less transparent.

[collerek]

Thanks for the nice words😁

I assume you mean multiple related models of the same type/class are inserted separately, why not issue one query per model? The answer is mainly the complexity and primary keys.

Since depending on the side of the relation and which one is which (fk or m2m) you need to save sometimes the left side first, other times the right side first and obtain the primary keys to populate the relations. So it's never a linear save a-b-c-d (well it can be, so never say never but that's just one of many cases). Very well you might have to save a than d than c and b at the end.

So in order to save all models, keep the corresponding primary keys after save (bulk save return none in SQL so you need additional query there to fetch inserted primary keys and server defaults), save proper models in order including through models for m2m relations it becomes much more complicated than you would think in a fists place.

All in all you need to create something similar to prefetch query. (Which on master is a little nightmare, it's completely rewritten in composite primary keys branch 😅).

There might be also problems with primary key fetching after bulk save, since you can have simultaneous connections, so you cannot just return last inserted n rows as someone might have added something in the meantime, so you can break relations if you are not carefull.

[franzwollang]

So in conclusion, what does this mean for the possibility of 1) trying to speed up writes of data spanning many relationships by take advantage of generating IDs instead of letting the SQL engine assign them and 2) maybe supporting a SLUG type similar to the existing UUID type?

Are you in favor of exploring these changes or against them?

[collerek]

As for the data insert optimization -> sure - it can be done along with force_insert flag, I want this change to be nonbreaking.

As for the slug you can already save bytes field type as base64 encoded string, wouldn't that be enough? I assume the only difference is with the generation of the shorter encoded UUID into basae64, but since fields do not provide any default values - the user has to do this - what would be the logic of this field? Or if you prefer strings can't you provide a default generator that produces those base64 encoded strings for you and simply use string field?

[franzwollang]

Ok. Let me know if/how I can help. I'm not particularly well-versed with the SQLAlchemy core library, but I'm open helping if I can.

Yes, I can do that. Everything's good then.

[franzwollang]

On second thought, I realize that foreign key columns have an integrity constraint requiring the key they point to to already exist. So at minimum, you'd always have to set any row inserts first and only then could you update foreign key columns on models or in through tables. So there's always a minimum requirement of 2 batches of statements.

But if that's the case, then it doesn't matter whether you provide the key manually or let a default or auto-increment handle it. You have to wait for the row inserts anyway, so you could also just get the inserted keys as well before doing the foreign key column updates.

So I guess the best that could be done is ensure that for nested objects across related tables, inserts always happen in exactly two batches of statements. So the latency would be around two round trips worth. Don't know if that's how it already works; in any case, sorry for not thinking it through carefully before discussing it with you.

[collerek]

If all keys on both sides will have keys populated you can insert them one after another in one transaction.
Only then you can issue queries in batch - if you have autoincrement keys there is no easy way to populate and retrieve the values in batch mode to populate related values on different models.

Yeah, that's what I meant when I wrote that one time you need to save left side first and some times right time first (depending on fk -> if it's reversed or normal relation (a->b or b->a with a-b-c-d relation)).

Since right now there is upsert going it does not work in batch. The force_insert flag will work only when the primary keys will be already populated (so set by hand or default function provided), even with autoincrement but values not set it will raise an exception as this will not work in batch mode.