tortoise / tortoise/tortoise-orm
Inherited mixin with ForeignKeyField does not respect source_field argument value.
Nobody has claimed this yet.
- Dominant language
- Python
- Stars
- 5.6k
- Forks
- 516
- Avg merge
- 2d 21h
- Merged PRs (30d)
- 9
Description
Describe the bug
When defining two or more models by using the same mixin class to reuse common fields, fields that are defined as fields.ForeignKeyField do not respect function argument's source_field value. Only the last class definitions works as expected. I think code explains this better, than putting in words.
To Reproduce
To reproduce the problem, execute:
from tortoise import Tortoise, fields, run_async, utils, connections
from tortoise.models import Model
class Owner(Model):
id = fields.IntField(pk = True)
name = fields.TextField(null = True)
class FkMixin:
id_owner = fields.ForeignKeyField('models.Owner',
db_constraint = False,
source_field = 'id_owner'
)
class TeaPot(FkMixin, Model):
id: fields.IntField(pk = True)
class Fork(FkMixin, Model):
id: fields.IntField(pk = True)
async def run():
await Tortoise.init(db_url="sqlite://:memory:", modules={"models": ["__main__"]})
await Tortoise.generate_schemas()
con = connections.get("default")
sql = utils.get_schema_sql(con, safe = True)
print(sql)
if __name__ == "__main__":
run_async(run())
Output is:
CREATE TABLE IF NOT EXISTS "owner" (
"id" INTEGER PRIMARY KEY AUTOINCREMENT NOT NULL,
"name" TEXT
);
CREATE TABLE IF NOT EXISTS "fork" (
"id" INTEGER PRIMARY KEY AUTOINCREMENT NOT NULL,
"id_owner" INT NOT NULL
);
CREATE TABLE IF NOT EXISTS "teapot" (
"id" INTEGER PRIMARY KEY AUTOINCREMENT NOT NULL,
"id_owner_id" INT NOT NULL
);
See the unexpected column name id_owner_id in teapot table definition.
Expected behavior
I would expect that generated schema is as shown below, where column name id_owner is for both tables.
CREATE TABLE IF NOT EXISTS "owner" (
"id" INTEGER PRIMARY KEY AUTOINCREMENT NOT NULL,
"name" TEXT
);
CREATE TABLE IF NOT EXISTS "fork" (
"id" INTEGER PRIMARY KEY AUTOINCREMENT NOT NULL,
"id_owner" INT NOT NULL
);
CREATE TABLE IF NOT EXISTS "teapot" (
"id" INTEGER PRIMARY KEY AUTOINCREMENT NOT NULL,
"id_owner" INT NOT NULL
);
Additional context
__version__ = "0.19.2"
The problem starts with the fact, that there is no way to explicitly define column name used for fields.ForeignKeyField, at the moment suffix "_id" is automatically appended to given property name in class which forces to use specific style of column naming convention "suffix *_id", but there exists other conventions as well.
For example, fields.ManyToManyField allows to use forward_key, backward_key to define column names used by intermediate table. Intuitively i expected that fields.ForeignKeyField had a similar property, but it does not.
Relevant code definitions:
def ManyToManyField(
model_name: str,
through: Optional[str] = None,
forward_key: Optional[str] = None,
backward_key: str = "",
...
VS
def ForeignKeyField(
model_name: str,
related_name: Union[Optional[str], Literal[False]] = None,
on_delete: str = CASCADE,
db_constraint: bool = True,
**kwargs: Any,
So i looked into code, and it seemed that class ForeignKeyFieldInstance allows to use source_field argument, due to inheritance from "base" class, and the thing is that it even works as i would expect, but only once and it bites you back later on in unexpected ways (i'll try to add comment and extra example later).
This issue is not so much about problem that arises do to using mixin, but more about the fact that fields.ForeignKeyFieldInstance does not allow to provide a custom column name.
My thoughts on intuition... well it really differs what is intuitive for whom and context of object use, but for me, i dislike the fact that fields with ForeignKeyFieldInstance are automatically/implicitly suffixed with "_id". For me personally, a more intuitive way would be that the field name is exactly as the property name, unless i stated differently. I'm a new user to tortoise-orm, and at first i did not expect that there is such a behavior and when i found this, it felt as if rug was pulled under my feet. I kind of understand why this is so, but still for me it is desirable so that i can specify column name explicitly, because when you are reusing an existing database, there might be columns that are named camelCase or prefix "id_" or whatever style. Because "_id" suffix is added automatically, it would be nice at least to have that mentioned in documentation.
So for me the question is - how to explicitly specify column name used for foreign key field? Is usage of argument "source_field" is not allowed for this use-case?
BTW a lot of things are very intuitive and convenient.
I tried to search for related issues where i think the root cause might be the same:
in #632 - issue describes problem with mixin case where model is being overridden, but i somewhat do not understand the explanation, but is seems related though;
in #1189 - issue described involves source_field argument and speaks about _id suffix and problem;
in #1118 most probably it is due to source_field usage, since i have somewhat similar problems down the road.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start by running the supplied SQLite reproduction and inspect ForeignKeyField, ForeignKeyFieldInstance, and the inherited source_field handling that produces id_owner_id for TeaPot. Compare the generated schema for TeaPot and Fork with the expected SQL, then determine how custom foreign-key column names should behave when fields come from a mixin and cover the result with a regression test.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- python, sqlite
- Domain
- backend, databases
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 38/100