Tortoise-orm: Provide an API for only fetching certain database fields

Created on 4 Apr 2019  路  16Comments  路  Source: tortoise/tortoise-orm

Consider the following:

class Employee(Model):
    id = fields.IntField(pk=True)
    name = fields.CharField(max_length=1024)
    ... 100+ other fields

async def get_employee_names():
    employees = await Employee.all()
    return [employee.name for employee in employees]

In this case, get_employee_names only needs a single field from the employee table, which is name. If Employee has a lot of fields in the table (as often occurs with mature databases), we serialize all those fields into memory even when we only need a single field. That's wasteful.

An improved API might be

async def get_employee_names():
    employees = await Employee.only('name').all()
    return [employee.name for employee in employees]

Django implements this API as documented here: https://docs.djangoproject.com/en/2.2/ref/models/querysets/#only

enhancement

Most helpful comment

Ok, let's just do this. I could use it for the pydantic serialisation to fetch less data as well.

All 16 comments

Hi

Take a look at values and values_list
https://tortoise-orm.readthedocs.io/en/latest/query.html#tortoise.queryset.QuerySet.values

Your case could be written like this

employee_names = await Employee.all().values_list('name', flat=True)

While my example could be handled by values_list, it breaks down when you want 3 of the 100+ fields. values is a fine workaround, but if you end up with a dict instead of a model instance, you're kind of circumventing the declarative model slightly.

The problem with .only() is that it still creates a full Python object, and then adds lazy accessors for all the remaining fields. Django can get away with this because there isn't strict boundaries in place for when it can go fetch data. This is also part of the reason Django is percieved to be slow. (unexpected DB round-trips)

asyncio only allows I/O on specific defined places, so we can't do the lazy accessing transparently at all. This is actually a good thing as it resists in running afoul of the least-surprises principle (and hence silly bugs may occur less often).

I'm not convinced that we need .only() in the way Django implements it. The only safe way I can think of doing it is to return a sparse object with no QuerySet attached to it. (or rather disabled to modify). But then it is also a nearly useless implementation.

:thinking: a dynamic subclass of the Model with a mixin that disables save() and its ilk.

We still need convincing that this is a feature we really need though. Especially considering the limitations implied.

@rockstar

While my example could be handled by values_list, it breaks down when you want 3 of the 100+ fields

I don't quite get why it breaks down?
You can do it like this

employee_fields = await Employee.all().values_list('id', 'name', 'some_other_field')

And you will get tuples of 3 values for each entry

Same goes for values

employee_maps = await Employee.all().values('id', 'name', 'some_other_field')

That will give you list of dict of 3 elements each

Closing as I think all needs listed in question are already addressed and that we don't want to implement .only (at least for now)

I don't quite get why it breaks down?

Because using values_list gives me a list of tuples, not objects. Why am I using an ORM if I don't get to work with objects? Now my logic leaks out of the instance because now I have to operate on lists of tuples, not the objects themselves.

We still need convincing that this is a feature we really need though. Especially considering the limitations implied.

I've provided a rather detailed example in the bug report description: when given 300k records of a table, with 50+ fields, we spend a lot of time serializing data that we never actually use. Given a graphql implementation, the whole purpose of the api is to only use data that we need.

It's frustrating to find this closed...

@rockstar Would you be fine with a read-only object that is partially populated if one uses a .only()?
As in it would not even know what fields was partially populated. Where right now you are expected to be careful if you do this?

Feedback! Please lets do this!!! : ) I would be fine with the read only using .only() as I'm building a read only GQL api.

Further, I really only want to perform one query that gives me back my object(s) and sub-object(s). So, if this can be done using prefetch to avoid lazy loading issues, then I'd be super happy.

There are ways to make SQLAlchemy work Async, but not with the ORM itself. So, I could get the query being built and the resulting list of tuples easily from SQLAlchemy. What I can't get is an ORM to turn the results of one query into objects and sub-objects.

Ok, let's just do this. I could use it for the pydantic serialisation to fetch less data as well.

Huh. I must've missed the followup after the issue was closed last year (ultimately, we couldn't commit to using tortoise because of the issue). I think the _best_ API would be one where an exception is raised if you try to access properties that weren't fetched. That way, there are guarantees that those properties aren't attempted. Explicit is better than implicit and all that.

@rockstar Agreed. For the use case we're describing, it would be easy to not access unpopulated fields since we are immediately serializing and sending the fields that we requested.

Still, an exception would be best. : )

it would be easy to not access unpopulated fields

It depends on how it is used. Particularly in python, there are lots of opportunities for bugs related to meta-programming and accidental partial implementation of a protocol (i.e. it says "wack" instead of quack, but that's close enough to call it a duck). For an async implementation, just saying "this violates an I/O fault line" or something similar in an exception would help track down bugs pretty quickly. The other option is to check if things are None (or an empty string or some other "dead" marker), but that gets complex when things are _allowed_ to be None.

I will certainly defer to you on this one. You seem to have waay more experience in this realm than I. : )

That's a good idea @rockstar

Yes, I'm considering just not creating the instance attributes (so we get AttributeError), and marking the instance non-saveable.
I'll add tests to ensure that accessing non-specified fields result in error, and that .save() also fails.

I have a working implementation in #350
Just need to write docs for it as the interactions are quite complex, as I allow partial updates.

Please have a look.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

usernein picture usernein  路  4Comments

anindyamanna picture anindyamanna  路  6Comments

asyncins picture asyncins  路  4Comments

mheppner picture mheppner  路  4Comments

croshchupkin picture croshchupkin  路  5Comments