johnny-cache icon indicating copy to clipboard operation
johnny-cache copied to clipboard

johnny-cache should use query.table_map.keys(), not query.tables

Open jdunck opened this issue 12 years ago • 4 comments

query.tables includes aliases, such as "T6"; these alias names are useless in terms of invalidation and also cause a lot of generational noise. They are necessarily the same base tables (aliases are only generated if required because a query refers to the same table more than once).

jdunck avatar Sep 17 '12 07:09 jdunck

As a silly demonstration of the problem, suppose we have

class Candidate(Model):
   pass
class Race(Model):
   candidates = ManyToManyField(Candidate)

See:

models.Candidate.objects.filter(races__candidates__races=1).query.table_map
models.Candidate.objects.filter(races__candidates__races=1).query.tables

jdunck avatar Sep 17 '12 07:09 jdunck

I've tried to make this change. It appears to work in Django 1.1 (ugh) in the get_tables_for_query11 function, but when I substitute the use of query.tables for query.table_map.keys() in QueryCacheBackend._monkey_write, I encounter a problem with a many to many test located here:

https://github.com/jmoiron/johnny-cache/blob/master/johnny/tests/cache.py#L649-653

Specifically, during that "clear()", a delete query is made, and the method patched onto django.db.backends.mysql.compiler.SQLDeleteCompiler finds something quite interesting;

ipdb> cls.query
<django.db.models.sql.subqueries.DeleteQuery object at 0x20cd310>
ipdb> cls.query.tables
['testapp_book_authors']
ipdb> cls.query.table_map
{}
ipdb> import django
ipdb> django.VERSION
(1, 4, 1, 'final', 0)

I'm ... not sure why that is, but this leads to an over-caching bug. Any thoughts?

jmoiron avatar Oct 05 '12 21:10 jmoiron

Hmm, can you like to a specific commit? I'm not sure which lines you're referring to, since HEAD 649 there doesn't point to a delete query.

jdunck avatar Oct 08 '12 05:10 jdunck

p1.books.clear() issues a delete query to the m2m table, which fails to invalidate properly if I replace:

https://github.com/jmoiron/johnny-cache/blob/master/johnny/cache.py#L400

With a list of the table_map instead, because the table_map does not have the m2m table "testapp_book_authors" in it.

jmoiron avatar Oct 08 '12 13:10 jmoiron