pyeve / eve-sqlalchemy

SQLAlchemy data layer for Eve-powered RESTful APIs
http://eve-sqlalchemy.readthedocs.io
Other
232 stars 70 forks source link

Add group by support #173

Closed daniel-passnfly closed 4 years ago

daniel-passnfly commented 6 years ago

I would like to use "group by" feature of SQLAlchemy. My solution is based on sort feature already implemented.

Solution

  1. Parse group_by arg in find method of SQL class.
    
    def extract_group_by_arg(req):
    group_by = req.args.get('group_by')
    if group_by:
        if re.match('^[,\w]+$', group_by):
            arg = []
            for s in group_by.split(','):
                arg.append(s)
            return arg
        else:
            return ast.literal_eval(group_by)
    else:
        return None

args = {'sort': extract_sort_arg(req), 'group_by': extract_group_by_arg(req), 'resource': resource}

2. Add parsed list to args param of SQLAResultCollection.

def parse_group_by(model, query, key): """ Group by parser that works with embedded resources Moved out from the query (find) method. """ if '.' in key: # group by related mapper class rel, group_by_attr = key.split('.') relclass = getattr(model, rel).property.mapper.class query = query.outerjoin(rel_class) base = getattr(rel_class, group_by_attr) else: base = getattr(model, key)

return base

if args['group_by']: ql = [] for group_item in args['group_by']: ql.append(parse_group_by(model, query, group_item)) args['group_by'] = ql

3. Use this list in order to use group_by SQLAlchemy method in _query attribute of SQLAResultCollection class.

self._group_by = kwargs.get('group_by') if self._group_by: self._query = self._query.group_by(*self._group_by)



## Usage example

> /resource?group_by=name,age
dkellner commented 6 years ago

Thank you for bringing this up and for your PR!

Can you explain your use-case for this? I did not think it through yet, but I'd guess it will break at least features like HATEOAS as the documents you will be returning will have no IDs nor URLs. So I'm hesitating to add a feature like this without at least knowing the limitations (and then documenting them).

daniel-passnfly commented 6 years ago

Hi, thanks for your reply.

I think you are correct and is breaking HATEOAS. Use-case is for grouping by some attribute model. So one item in list '_items' really correspond to many on database based on a certain attribute.

Example

If we have a collection of People that works in a certain organization, i would like to group by organization in order to obtain the differents organizations that collection have.

We have the following database table:

id | name | org_id
-- | ---- | ------
1  | John  |   1
2  | David |   1
3  | James |   2

So doing a request to endpoint:

/people?group_by=org_id

Will produce:

{
  "_items": [
    {
      "id": 1,
      "name": "John",
      "org_id": 1,
      "_links": {
        "self": {
          "href": "people/1",
          "title": "People"
        }
      }
    },
    {
      "id": 3,
      "James": "John",
      "org_id": 2,
      "_links": {
        "self": {
          "href": "people/3",
          "title": "People"
        }
      }
    }
  ]
}

The first item with name John, could be David also because they are in the same org_id. Depends on the real query to database and is based in sort criteria.

dkellner commented 6 years ago

In your use-case, would it not be more sensible (from an API design perspective) to have an /organizations endpoint? Exposing database internals in an otherwise abstract API might not be a good idea if you want to change the underlying database at some point.

Note that you could always use regular Flask endpoints to add special endpoints behaving differently than Eve's default (which is CRUD).

I'm still not convinced this really is a feature we should add (and then basically have to support in all future versions). In the current stage it seems not too useful without additionally supporting things like COUNT, SUM etc. But doing that is really database-dependant. Even selecting columns not in the GROUP statement and without an aggregation function depends on the database implementation AFAIK.

dkellner commented 4 years ago

Closed due to inactivity. Feel free to re-open it if you want to continue the discussion!