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

Exception 'Exists' object has no attribute 'rhs' #156

Closed
shield007 opened this issue Jun 23, 2020 · 16 comments
Closed

Exception 'Exists' object has no attribute 'rhs' #156

shield007 opened this issue Jun 23, 2020 · 16 comments

Comments

@shield007
Copy link
Contributor

What happened?

We are running django-cachalot in production and keep getting exceptions "'Exists' object has no attribute 'rhs'". This is captured in sentry, so we can see some of the details.

It's comming from the following line in cachalot/utils.py:
rhs = child.rhs

Here are some of the details set at this point:

  • child - django.db.models.expressions.Exists object
  • child_class - django.db.models.expressions.Exists
  • children - [django.db.models.lookups.Exact,django.db.models.expressions.Exists]
  • rhs - uuid.UUID
  • rhs_class - uuid.UUID

What should've happened instead?

No exception

Steps to reproduce

Seems to happen on DB quieries that make use of django.db.models.expressions.Exists.

Django==3.0.6
Postgres DB (12.x)

@Andrew-Chen-Wang
Copy link
Collaborator

Andrew-Chen-Wang commented Jun 23, 2020 via email

@shield007
Copy link
Contributor Author

Yes, we should be on the latest. I have the django-cachalot==2.2.0 in the requirements file.

@Andrew-Chen-Wang
Copy link
Collaborator

@shield007 Hm. #132 (that was merged in a different commit) was supposed to fix Django's extensions. I'll look into it. Do you mind posting traceback? It's fine if you don't; you may not need to.

@shield007
Copy link
Contributor Author

shield007 commented Jun 25, 2020

@Andrew-Chen-Wang Sure, here is the traceback we got, I've striped out the lins from our code base. But if thats a problem let me know. It's probally fine to include them. Just taking them out while they are not needed:

AttributeError: 'Exists' object has no attribute 'rhs'
File "django/core/handlers/exception.py", line 34, in inner
response = get_response(request)
File "django/core/handlers/base.py", line 115, in _get_response
response = self.process_exception_by_middleware(e, request)
File "django/core/handlers/base.py", line 113, in _get_response
response = wrapped_callback(request, *callback_args, **callback_kwargs)
File "newrelic/hooks/framework_django.py", line 540, in wrapper
return wrapped(*args, **kwargs)

removed application code

File "django/views/generic/base.py", line 71, in view
return self.dispatch(request, *args, **kwargs)
File "django/utils/decorators.py", line 43, in _wrapper
return bound_method(*args, **kwargs)
File "django/views/decorators/csrf.py", line 54, in wrapped_view
return view_func(*args, **kwargs)
File "django/utils/decorators.py", line 43, in _wrapper
return bound_method(*args, **kwargs)

removed application code

File "newrelic/hooks/framework_django.py", line 931, in wrapper
return wrapped(*args, **kwargs)
File "django/views/generic/base.py", line 97, in dispatch
return handler(request, *args, **kwargs)

removed application code

File "django/db/models/query.py", line 276, in iter
self._fetch_all()
File "django/db/models/query.py", line 1261, in _fetch_all
self._result_cache = list(self._iterable_class(self))
File "django/db/models/query.py", line 57, in iter
results = compiler.execute_sql(chunked_fetch=self.chunked_fetch, chunk_size=self.chunk_size)
File "cachalot/monkey_patch.py", line 36, in inner
return original(compiler, *args, **kwargs)
File "cachalot/monkey_patch.py", line 83, in inner
table_cache_keys = _get_table_cache_keys(compiler)
File "cachalot/utils.py", line 196, in _get_table_cache_keys
for t in _get_tables(db_alias, compiler.query)]
File "cachalot/utils.py", line 172, in _get_tables
for subquery in _find_subqueries_in_where(query.where.children):
File "cachalot/utils.py", line 117, in _find_subqueries_in_where
rhs = child.rhs

Also because we have this captured in sentry, I can access any of the local varibles anywhere though that backtrace stack. So let me know if you need them to help debuging.

@shield007
Copy link
Contributor Author

shield007 commented Jun 25, 2020

Also dobule checked the running version and it is indeed 2.2.0.

/app # pip freeze | grep cachalot
django-cachalot==2.2.0

@Andrew-Chen-Wang
Copy link
Collaborator

@shield007 Thanks for adding your traceback. I understand the problem. Please look out for cachalot 2.2.2. The problem WAS fixed, but I apparently didn't put it in cachalot 2.2.0. It's actually fixed in master branch. Sorry for the inconvenience. That patch for 2.2.2 should come out today.

@Andrew-Chen-Wang
Copy link
Collaborator

@shield007 I'm going to close this issue now since I just published version 2.2.2 which should come with the fix. Sorry for the inconvenience! Please reopen this if the issue persists.

@shield007
Copy link
Contributor Author

Thanks for looking into it. Will test 2.2.2 today.

@shield007
Copy link
Contributor Author

shield007 commented Jul 3, 2020

We still see problems with the 2.2.2 relase. Here is a stack we are getting:

AttributeError: 'Exists' object has no attribute 'rhs'
  
  File "django/db/models/query.py", line 1261, in _fetch_all
    self._result_cache = list(self._iterable_class(self))
  File "django/db/models/query.py", line 57, in __iter__
    results = compiler.execute_sql(chunked_fetch=self.chunked_fetch, chunk_size=self.chunk_size)
  File "cachalot/monkey_patch.py", line 29, in inner
    return original(compiler, *args, **kwargs)
  File "cachalot/monkey_patch.py", line 76, in inner
    table_cache_keys = _get_table_cache_keys(compiler)
  File "cachalot/utils.py", line 191, in _get_table_cache_keys
    for t in _get_tables(db_alias, compiler.query)]
  File "cachalot/utils.py", line 167, in _get_tables
    for subquery in _find_subqueries_in_where(query.where.children):
  File "cachalot/utils.py", line 109, in _find_subqueries_in_where
    rhs = child.rhs

@shield007
Copy link
Contributor Author

Can this issue be reopened ?

@Andrew-Chen-Wang
Copy link
Collaborator

Andrew-Chen-Wang commented Jul 3, 2020

I see. Same problem. Perhaps, if we moved the subquery into the try block? @shield007 Do you mind posting a sample query? I'd like to get all subqueries fixed because it seems to be a recurring issue since Django 2.2.

@shield007
Copy link
Contributor Author

I've looked into this a bit, I know have some code to reproduce the problem from the django shell. From what I can see the main issue is that on line 117, rhs = child.rhs. The problem is that the child does not have a property rhs because child.class is actually django.db.models.expressions.Exists.

@shield007
Copy link
Contributor Author

I've now created a pull request, #157. This fixes it for me. Hopefully this helps with the issue.

@Andrew-Chen-Wang
Copy link
Collaborator

@shield007 Yes, but I didn't want #157 to invalidate Exists subqueries from working with cachalot. The deleted lines in your PR was there so that your subquery could work with cachalot in Django 2.2. I'd much rather find a solution to getting it to work than for it to not work. You can use #155 whenever someone gets around to working on disabling cachalot temporarily via a context manager. Hopefully soon from my end.

Andrew-Chen-Wang added a commit that referenced this issue Jul 29, 2020
* Combine child_class passing to minimize if's

Co-authored-by: Andrew Chen Wang <60190294+Andrew-Chen-Wang@users.noreply.github.com>
@Andrew-Chen-Wang
Copy link
Collaborator

@shield007 I've merged your PR for version 2.3.0. If it's still not working, you can use the new context manager to disable cachalot. Please let me know if there are any problems.

@Andrew-Chen-Wang
Copy link
Collaborator

ping

Andrew-Chen-Wang added a commit that referenced this issue May 12, 2021
* Reverts #157 with proper fix. The initial problem was due to `django.db.models.expressions.Subquery` allowing both QuerySet and sql.Query to be used.
* Fixes Django 3.2 test_subquery and test_invalidate_subquery testing by also checking the lhs
Andrew-Chen-Wang added a commit that referenced this issue May 13, 2021
* Remove system check for Django version
* Closes #175
* Bump version and amend CHANGELOG
* Update with GitHub action CI
* Update README with Python and Django versions
* Limit Django version to 3.2, inclusively.
* Add note on Django constraints to README
* Bump minor version
* Justified by dropping support for dependency versions
* Drop support for Django 2.0-2.1, Python 3.5
* Change CI badge in README to GitHub actions
* Add support for Pymemcache for Django 3.2+
* Add Django 3.2 to test matrix
* Fix MySQL test_explain
* Allow filebased delta leniency in tests
* Allow Subquery in finding more Subqueries (Fixes #156)
* Reverts #157 with proper fix. The initial problem was due to `django.db.models.expressions.Subquery` allowing both QuerySet and sql.Query to be used.
* Fixes Django 3.2 test_subquery and test_invalidate_subquery testing by also checking the lhs
* Fix Django 2.2 Subquery having no query attr
* Fix filebased test time delta
* Fix test_invalidate_having due to new inner_query
* The new inner_query replaces subquery for Django 3.2 where subquery is now a boolean. That's why I initially used that TypeError in _get_tables_from_sql. inner_query looks to be a sql.Query
* Add PyMemcacheCache to supported cache backends

Co-authored-by: Dominik George <nik@naturalnet.de>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants