search

noripyt / django-cachalot

No effort, no worry, maximum performance.
http://django-cachalot.readthedocs.io
BSD 3-Clause "New" or "Revised" License
1.25k stars 151 forks source link

Clarify and consider changing the default value of CACHALOT_FINAL_SQL_CHECK #266

Open danlamanna opened 2 months ago

danlamanna commented 2 months ago

Description

I'm struggling to understand the exact implications of CACHALOT_FINAL_SQL_CHECK introduced in https://github.com/noripyt/django-cachalot/pull/199. It seems like by default omitting this check can cause invalidations to be missed. It also appears (at least from looking at the code) that it could cache results that are part of CACHALOT_UNCACHABLE_TABLES if it misses them.

In https://github.com/noripyt/django-cachalot/pull/199#issuecomment-979440732, @Andrew-Chen-Wang said it should be a flag because of its experimental nature. Is it still considered experimental, or can it be promoted to the default? I think it's currently a bit misleading that the default behavior forgoes correctness for performance.

sdolemelipone commented 1 month ago

It appears some queries may fail with CACHALOT_FINAL_SQL_CHECK=True; see #268.

Andrew-Chen-Wang commented 1 month ago

I think it's currently a bit misleading that the default behavior forgoes correctness for performance.

Primarily, still looking for feedback as sdolemelipone has found. The error found in #268 isn't really a surprise; I've seen it before multiple times in Django cachalot when updating for the latest Django version.