Tortoise-orm: Unable to use .save() method on a nullable ForeignKeyField

[nsidnev]

About the bug
Returns wrong value when trying to save FK field, which may be null

To Reproduce

import asyncio
import logging

from tortoise import Tortoise, fields, models

logging.basicConfig(level=logging.DEBUG)

POSTGRES_URL = "postgres://postgres:postgres@localhost/postgres"


class ModelA(models.Model):
    string = fields.TextField(default="")


class ModelB(models.Model):
    a_model = fields.ForeignKeyField(
        "models.ModelA", related_name="b_models", null=True
    )


async def main():
    await Tortoise.init(db_url=POSTGRES_URL, modules={"models": ["__main__"]})
    await Tortoise.generate_schemas()

    a, _ = await ModelA.get_or_create()
    b, _ = await ModelB.get_or_create()

    b.a_model = a
    await b.save()

    await Tortoise.close_connections()


asyncio.run(main())

This code uses None as a_model_id on update query. But if there is .create or .get_or_create method with passed a_model arg, then the code uses the right values.

Expected behavior
.save method should use the right values when updating nullable foreign key.

Additional context
Python version: CPython 3.7.4
Installed packages:
aiosqlite==0.10.0
asyncpg==0.18.3
atlastk==0.10.7
ciso8601==2.1.1
PyPika==0.35.4
tortoise-orm==0.13.3

[grigi]

I'll have a look at this tomorrow.
By the way, your bug reports are really good :+1: I'm a big fan!

[nsidnev]

By the way, your bug reports are really good I'm a big fan!

Thanks! 😄
I'm glad that this project exists and I am happy to help it this way, at least until I haven't enough knowledge to open the PR with fix.

[grigi]

Oh wow, I don't think this ever worked right...
How did we miss something this basic?
…
How do I dynamically create a property? :thinking:
We can do it sync, as all the data is available already.

[nsidnev]

Well. Perhaps I don’t quite understand something, because I haven’t yet fully figured out the code base of this project, but why do we need to store both the ForeignKeyField and the value that it points to in DB in the model? Can't this value be retrieved/generated using only ForeignKeyField? Then, as it seems to me, getting the value from the model will be successful.

[grigi]

The in-DB representation of a FK could be many different types, whereas in Python it is a reference to another object (always the same)
Hence a FK is actually two different things composed together from the ORM's point of view.

The ForeignKeyField is the ORM object, which may not yet exist in memory.
It's also for performance, don't create an object unless you need it, especially as it can be quite expensive to create (A whole model instance is heavy)

I have found a solution: place a property: https://docs.python.org/3.5/library/functions.html#property on the dynamically created class, and have a private variable for the actual instance.
essentially:

def set_fk(self, value):
    self._fk = value
    self.fk_id = value.pk
model.fk = property(lambda self: self._fk, set_fk)

Which is pretty easy to dynamically create. Initial version will probably be partial'ed methods instead of code-gen (I want to only add code-gen if there is a real benefit, but to determine that we need a more comprehensive benchmark suite)

[nsidnev]

Oh. Now I got it, thanks for the explanation. I will wait for the patch! :)

[grigi]

@nsidnev Please test #185

e.g. do a pip install https://github.com/tortoise/tortoise-orm/archive/fix_null_fk.zip

FYI, the partial/property change is only in the models file here: https://github.com/tortoise/tortoise-orm/pull/185/files#diff-2b3e042db0b773b93d8a98880434ee35

I can see how to extend this to give more capabilities, e.g. make the FK directly awaitable? And do the same with RPK & M2M for performance improvements to make it even more lazy (e.g. only create the objects when used instead of in __init__)

[nsidnev]

FYI, the partial/property change is only in the models file here: tortoise/tortoise-orm/pull/185/files#diff-2b3e042db0b773b93d8a98880434ee35

Thank you, I'll watch it.

Just checked on my project. It worked as it should, thanks for the quick fix!