Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Sorting and filtering on hybrid properties fails #562

Closed
green3g opened this issue Jul 21, 2016 · 14 comments
Closed

Sorting and filtering on hybrid properties fails #562

green3g opened this issue Jul 21, 2016 · 14 comments
Labels

Comments

@green3g
Copy link

green3g commented Jul 21, 2016

I'm using flask_restless-1.0.0b2 and it seems that if I define a hybrid property like

    @hybrid_property
    def is_complete(self):
        """
        when all features are completed, this workorder should be completed
        """
        for f in self.features:
            if f.feature_status == 'Open':
                return 'No'
        return 'Yes'

    @is_complete.expression
    def is_complete(cls):
        return select([Workorder_Feature.feature_status]).\
                where(Workorder_Feature.workorder_id==cls.id)

And I pass parameters like this:

sort:is_complete

or

filter[objects]:[{"name":"is_complete","op":"like","val":"%No%"}]

It throws errors in flask_restless\search\operators.py on line 98 https://github.com/jfinkels/flask-restless/blob/master/flask_restless/search/operators.py#L98:
'Select' has no attribute 'like'

and in flask_restless\search\drivers.py on line 121 https://github.com/jfinkels/flask-restless/blob/master/flask_restless/search/drivers.py#L121:
Select object has no attribute 'asc'

@jfinkels jfinkels added the bug label Jul 21, 2016
@jfinkels
Copy link
Owner

So getattr(MyModel, 'is_complete') returns a Select object when is_complete is a hybrid property? Okay, I guess this should be handled by another special case in the code. Something similar will probably happen in the code handling group_by.

@green3g
Copy link
Author

green3g commented Aug 2, 2016

Looking into this a bit more, if I print out what arg1 is in https://github.com/jfinkels/flask-restless/blob/master/flask_restless/search/operators.py#L173

it looks like it is the select statement returned by the @is_complete.expression function above.

When printed it looks like this:

SELECT flask.utilities_workorder_feature.feature_status
FROM flask.utilities_workorder_feature, flask.utilities_workorder
WHERE flask.utilities_workorder_feature.workorder_id = flask.utilities_workorder.id

And the type is <class 'sqlalchemy.sql.selectable.Select'>

@jfinkels
Copy link
Owner

This is two separate issues: 1. using like on hybrid properties doesn't work when filtering, 2. sorting and grouping on hybrid properties doesn't work. I will look into these each separately.

@jfinkels
Copy link
Owner

Okay, I think I understand what the issue is, but I'm not sure how to resolve this.

Your assumption was that the client will be able to make filtering queries based on the value of the hybrid attribute (the method decorated by @hybrid_property). That's reasonable. However, filters are created via SQL expressions, so the query must actually use the SQL expression (the method decorated by @is_complete.expression). In simpler cases, where the hybrid attribute and the expression would be the same this might not be a problem, because self works as either the class or an instance of the class.

In summary, you are trying to filter on instance attributes, but filtering is done at the class level (representing SQL expressions), so the hybrid property expression must be used. I'm not sure if there's a workaround for this, you may have to change your model or change the client.

In any case, if we cannot figure out a way to resolve this, there should at least be a better error message.

@green3g
Copy link
Author

green3g commented Nov 1, 2016

so the hybrid property expression must be used

I've been looking into solving this issue on my end, but I'm really confused. What do you mean by this?

I'm not sure if there's a workaround for this, you may have to change your model or change the client.

Do you have any ideas on how my model should be changed? Every example I've seen in which hybrid_properties are used with relationships uses a select(.....) statement.

@jfinkels
Copy link
Owner

jfinkels commented Nov 1, 2016

See the Hybrid Attributes section of the SQLAlchemy documentation: "In many cases, the construction of an in-Python function and a SQLAlchemy SQL expression have enough differences that two separate Python expressions should be defined." In the example there, there is a "hybrid property", which is a function on an instance of the model, and there is a separate "expression", which is a function on the model class.

In Flask-Restless, when you have an Interval class like in the linked example and a filter that looks like

{"name": "radius", "op": "<", "val": 10}

the Flask-Restless search code issues a SQL query like

Interval.radius < 10

It is using the "expression" form of the hybrid property, not the instance attribute form. In your example, you expected the query to execute the first is_complete method code on each instance of your model. However, what actually happens is that Flask-Restless executes the second is_complete method code at the class-level.

However, reading more carefully the Correlated Subquery Relationship Hybrid section of the documentation, I see that you are using the pattern suggested there. So Flask-Restless should be able to interpret the select query somehow, you shouldn't have to change your model.

Looking at the example in that section:

>>> print s.query(User).filter(User.balance > 400)
SELECT "user".id AS user_id, "user".name AS user_name
FROM "user"
WHERE (SELECT sum(account.balance) AS sum_1
FROM account
WHERE account.user_id = "user".id) > :param_1

makes me think that maybe this is something weird about using the "like" operator with a select statement? I'm not sure.

@green3g
Copy link
Author

green3g commented Nov 1, 2016

Thanks for the tips, seriously I really appreciate you taking the time. I guess I didn't really understand what was going on with the expression. My "correlated subquery" is incorrect I think. Instead of returning the value "Yes" or "No" if all the child objects were "completed" it was simply selecting the child objects.

I modified my expression to be this:

    @is_complete.expression
    def is_complete(cls):
        return case({True: 'Yes', False: 'No'},
            select([func.count(Workorder_Feature.id)]).\
                where(Workorder_Feature.workorder_id==cls.id).\
                where(Workorder_Feature.feature_status == 'Open') == 0
        )

And now I'm able to sort and filter. Even with like

@green3g
Copy link
Author

green3g commented Nov 4, 2016

Okay, I guess I was a bit preemptively overzealous. While I'm able to get results when sort/filter without errors, it doesn't look like the data is actually getting filtered or sorted on the hybrid property. When sorting, it returns the data in the same order regardless of ascending/descending. Filtering while throwing no errors isn't actually filtering correctly either.

filter on "Yes"

http://greggtest.cityhall.com:5000/api/workorder?filter[objects]=[{"name":"is_complete","op":"equals","val":"Yes"}]

produces the json

{
  "data": [],
  "included": [],
  "jsonapi": {
    "version": "1.0"
  },
  "links": {
    "first": "http://greggtest.cityhall.com:5000/api/workorder?filter[objects]=[{\"name\":\"is_complete\",\"op\":\"equals\",\"val\":\"Yes\"}]&page[number]=1&filter[objects]=[{\"name\": \"is_complete\", \"op\": \"equals\", \"val\": \"Yes\"}]&page[size]=10",
    "last": "http://greggtest.cityhall.com:5000/api/workorder?filter[objects]=[{\"name\":\"is_complete\",\"op\":\"equals\",\"val\":\"Yes\"}]&page[number]=0&filter[objects]=[{\"name\": \"is_complete\", \"op\": \"equals\", \"val\": \"Yes\"}]&page[size]=10",
    "next": "http://greggtest.cityhall.com:5000/api/workorder?filter[objects]=[{\"name\":\"is_complete\",\"op\":\"equals\",\"val\":\"Yes\"}]&page[number]=2&filter[objects]=[{\"name\": \"is_complete\", \"op\": \"equals\", \"val\": \"Yes\"}]&page[size]=10",
    "prev": "http://greggtest.cityhall.com:5000/api/workorder?filter[objects]=[{\"name\":\"is_complete\",\"op\":\"equals\",\"val\":\"Yes\"}]&page[number]=0&filter[objects]=[{\"name\": \"is_complete\", \"op\": \"equals\", \"val\": \"Yes\"}]&page[size]=10",
    "self": "/api/workorder"
  },
  "meta": {
    "total": 0
  }
}

filter on "No" (truencated)

http://greggtest.cityhall.com:5000/api/workorder?filter[objects]=[{"name":"is_complete","op":"equals","val":"No"}]

{
  "data": [
    {
      "attributes": {
        "created_by": null,
        "date_completed": null,
        "date_created": "2016-11-03T14:14:00.713000",
        "date_modified": "2016-11-03T14:14:00.713000",
        "details": "Yo",
        "is_complete": "Yes",
        "location": "Yo",
        "modified_by": null
      },
      "id": "76",
      "links": {
        "self": "http://greggtest.cityhall.com:5000/api/workorder/76"
      },
      "relationships": {
        "comments": {
          "data": [],
          "links": {
            "related": "/api/workorder/76/comments",
            "self": "/api/workorder/76/relationships/comments"
          }
        },
        "crew": {
          "data": {
            "id": "2",
            "type": "workorder_crew"
          },
          "links": {
            "related": "/api/workorder/76/crew",
            "self": "/api/workorder/76/relationships/crew"
          }
        },
        "features": {
          "data": [],
          "links": {
            "related": "/api/workorder/76/features",
            "self": "/api/workorder/76/relationships/features"
          }
        },
        "status": {
          "data": {
            "id": "2",
            "type": "workorder_status"
          },
          "links": {
            "related": "/api/workorder/76/status",
            "self": "/api/workorder/76/relationships/status"
          }
        },
        "task": {
          "data": {
            "id": "1",
            "type": "workorder_task"
          },
          "links": {
            "related": "/api/workorder/76/task",
            "self": "/api/workorder/76/relationships/task"
          }
        }
      },
      "type": "workorder"
    },

@jfinkels
Copy link
Owner

jfinkels commented Nov 4, 2016

Can you post the complete code for a minimal working example? (No extraneous attributes, for example.)

@green3g
Copy link
Author

green3g commented Nov 4, 2016

Yes, sir, here's a complete example. https://gist.github.com/roemhildtg/435fd78be14c2f0459e078a3a14fa476

@jfinkels
Copy link
Owner

I think you need to double check your hybrid property expression. The part with select().where() evaluates to a SQL SELECT expression (try printing it out), which is a SQLAlchemy Select object. This object has no __eq__ implementation, so when you do == 0, it falls back to the default Python equality check, which compares the object on the left to the object 0 on the right (which will always be False, that's why the expression always evaluates to 'No' in your example). According to the example in the Correlated Subquery Relationship Hybrid section of the SQLAlchemy documentation, you should use .label() on the result of the select().where(). You might get better help on the SQLAlchemy mailing list since I am not an expert. But I think this is not a bug with Flask-Restless.

Regardless, I'd like to add a test for filtering and sorting using a hybrid expression so that when this comes up in the future, I can point to the tests and say for sure that the problem is not in Flask-Restless.

@green3g
Copy link
Author

green3g commented Nov 30, 2016

I've finally gotten to the bottom of this... First I didn't realize that == 0 would be interpreted using the default python check. I ended up rewriting the case query again, so that it didn't use the == operator and after adding .label('count') I now have it working successfully. But wow, that is very tricky.

the final hybrid property in case it helps someone:

    @hybrid_property
    def is_complete(self):
        """
        when all features are completed, this workorder should be completed
        """
        features = Workorder_Feature.query.filter(Workorder_Feature.workorder_id == self.id)
        for f in features:
            if f.feature_status == 'Open':
                return 'No'
        return 'Yes'

    @is_complete.expression
    def is_complete(cls):
        count = select([func.count(Workorder_Feature.id)]).\
            where(Workorder_Feature.workorder_id==cls.id).\
            where(Workorder_Feature.feature_status == 'Open').label('count')
        return case({0: 'Yes'}, count, else_='No')

@jfinkels
Copy link
Owner

I have added unit tests for this situation in pull request #621. I'm going to close this as not an issue with Flask-Restless. Hopefully the discussion here helps someone with this issue in the future. Thanks for continuing to report issues @roemhildtg!

@mixmastamyk
Copy link

mixmastamyk commented Oct 24, 2017

Getting this and no idea how to fix:

AttributeError: Neither 'hybrid_property' object nor 'ExprComparator' object associated 
with PlayerGroups.count has an attribute 'property'
class PlayerGroups(db.Model):
    members = db.relationship('Players', back_populates='group')

    @hybrid_property
    def count(self):
        return len(self.members)

    @count.expression
    def count(cls):
        return db.select([db.func.count(Players.id)]).where(
                            Players.group_id == cls.id).label('count')

Happens when I try to POST / create a new group.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants