eagerloading embedded resources

[dtheodor]

Hi,

I have a simple one-to-many Parent-Child relationship. I populate my database with 2 parents, each of which has 3 children.

When I request /api/child?embedded={"parent":1}, the amount of queries hitting the database range from 7 to 9 queries (excluding the count query), depending on how I configure eager loading at the relationship level. There's 1 query per child to get its parent, plus some overhead.

This is != scalable. I think SQLAlchemy's Relationship Loading Techniques can be used to great extend here to create efficient queries. The amount of queries required to retrieve the data of the simple request above, regardless of the amount of entries in the database (and assuming no pagination) should be 1.

[amleczko]

I totally agree that the current implementation is not perfect. Please remember that we have a huge dependency which is Eve itself. If you have any improvements ideas - let me know :)

[goneri]

The problem is in eve.utils.embedded_document() from Eve itself. It takes the embedded document one by one. This makes complicated to reduce the number of SQL request.

[ralphsmith80]

This might not be the most performant use of sqlalchemy depending on your situation but I use Mapping Class Inheritance. That can be really nice because the join relationship is defined in your model class with a polymorphic column and __mapper_args__.

__mapper_args__ = {
        'polymorphic_on':type,
        'polymorphic_identity':'employee',
        'with_polymorphic':'*'
    }

with_polymophic:'*' can be heavy depending on your implementation but it's perfect for doing a join on all your inheriting tables. On the SQL end this joins all children tables with one query which is exactly what I want. The issue I've ran into with Eve-SQLAlchemy is that the joined attributes that only exist in child tables get dropped when converting the sql object to a response dict .

Rather than doing a loop through the fields defined on the parent model and dropping the rest I've found that just copying all the attributes and deleting the ones that are not serializable. I've only found one that could not be serialized.

Just adding the following to the bottom of the sqla_object_to_dict is what I'm talking about.

result = copy.copy(obj.__dict__)
del result['_sa_instance_state']
return result

Yes this will break some of the existing unit tests. I've checked. The failures are related to the expected default embedding. Which obviously makes sense because we're extending that embedding via sqlalchemy and we're not telling eve-sqlalchemy.

As an alternative approach that is more EVE idiomatic would be to allow defining the embedded attributes in settings.py. Currently embedded attributes are only read from the request. I believe the default should be to read them from settings.py and allow request defined embedded attributes to override those defined in settings.py.

This alternative approach should keep the unit tests working and more importantly it enables Mapping Class Inheritance which should be working anyway. Currently it doesn't work since the response drops the joined attributes.

My only issues with this alternative approach is that it's not very DRY. The sqlalchemy models already defined this relationship with the __mapper_args__ and you're redefining it in settings.py. AFAICT this is still going to be the best option.

[ralphsmith80]

Have some more playing around it looks you can achieve the alternative approach I described about by the DOMAIN definition as follows:

DOMAIN['people']['datasource']['projection'].update({
    'status': 1,
    'filter_type': 1,
})

The datasource and projection attributes are added by eve-sqlalchemy by default. That projection can be updated in settings.py to produce the reponse the you desire. I'm just using some example attributes for my use case obviously you'll want to update with attributes you want to add. Probably any/all attributes on your inheriting child classes.

Note: You still need to use Mapping Class Inheritance to get to a single SQL query. I think that should get you where you want to be without any eve-sqlalchemy code change.