tortoise / tortoise-orm

Familiar asyncio ORM for python, built with relations in mind
https://tortoise.github.io
Apache License 2.0
4.69k stars 390 forks source link

Do not validate when loading from the db #1750

Closed henadzit closed 3 weeks ago

henadzit commented 4 weeks ago

Description

Motivation and Context

None of the ORMs that I checked validate records when loading them from the database:

I do not see a reason why validating the record on loading should be a default behavior. I can see that you might want to do it in some cases but you can always call .validate explicitly. But I can imagine the issue where you add a new validator, run a data migration to fix the data but while the migration is being run, the reads are failing.

I also ran a benchmark where I was loading 100 testmodels.ValidatorModel records from the database and on average not running validation is ~5% faster for a model with many validators. I do not think at all that this is a decisive argument but something to keep in mind.

How Has This Been Tested?

from tortoise import Tortoise, fields, run_async from tortoise.functions import Sum from tortoise.models import Model from tortoise.expressions import F, Q from tortoise.validators import MinValueValidator

class Deal(Model): test = fields.IntField() broker_payed_money = fields.FloatField(default=0, validators=[MinValueValidator(0)])

broker_commission = fields.FloatField(null=False)

developer_payed_money = fields.FloatField(
    null=False, default=0, validators=[MinValueValidator(0)]
)

async def main():

Initializing Tortoise and creating the schema

await Tortoise.init(  # type: ignore
    db_url="sqlite://:memory:",
    modules={"models": ["__main__"]},
)
await Tortoise.generate_schemas()

await Deal.create(
    test=1, broker_payed_money=10, broker_commission=0.1, developer_payed_money=20
)

result = await Deal.annotate(
    balance=Sum(F("developer_payed_money") * F("broker_commission") - F("broker_payed_money")),
).values_list("balance")
print("res", result)

run_async(main())



## Checklist:
<!--- Go over all the following points, and put an `x` in all the boxes that apply. -->
<!--- If you're unsure about any of these, don't hesitate to ask. We're here to help! -->
- [x] My code follows the code style of this project.
- [ ] My change requires a change to the documentation.
- [ ] I have updated the documentation accordingly.
- [ ] I have added the changelog accordingly.
- [x] I have read the **CONTRIBUTING** document.
- [x] I have added tests to cover my changes.
- [x] All new and existing tests passed.
coveralls commented 4 weeks ago

Pull Request Test Coverage Report for Build 11540154248

Details


Changes Missing Coverage Covered Lines Changed/Added Lines %
tortoise/backends/sqlite/executor.py 3 4 75.0%
<!-- Total: 5 6 83.33% -->
Files with Coverage Reduction New Missed Lines %
tortoise/backends/sqlite/executor.py 1 77.14%
<!-- Total: 1 -->
Totals Coverage Status
Change from base Build 11529966516: -0.02%
Covered Lines: 5974
Relevant Lines: 6598

💛 - Coveralls
henadzit commented 3 weeks ago

@abondar, would love to hear your feedback on this one!