Skip to content

Removes query-level caching from QueryBuilder#655

Draft
Samuell1 wants to merge 1 commit intooctobercms:4.xfrom
Samuell1:remove-query-cache
Draft

Removes query-level caching from QueryBuilder#655
Samuell1 wants to merge 1 commit intooctobercms:4.xfrom
Samuell1:remove-query-cache

Conversation

@Samuell1
Copy link
Member

No description provided.

@daftspunk
Copy link
Member

No real benefit in removing this. It is used in Twig,

{% set records = model.where('foo', 'bar').remember(20).get() %}

@Samuell1
Copy link
Member Author

Samuell1 commented Jan 16, 2026

From my point it too much depends on query what needs to cache because just this line
md5($name.$this->toSql().serialize($this->getBindings())) makes its heavier then it looks and then key cant be controlled externally, its just makes more limitations then possibilities then i needed to use Cache::remember closure to make it work better.

Maybe move it to trait, then it can be used on model where people need it. Thats why i added is as draft, to maybe add more to it or rethink this :)

@daftspunk
Copy link
Member

Twig doesn't have closures so this is why we kept it.

Laravel removed it for the same reasons you mention, it can't serve every possible combination of query. It serves simple use cases very well, though.

I'm not sure it is possible to make a trait for the query builder.

@Samuell1
Copy link
Member Author

I will try to look into it for best possible solution to dont limit performance improvements and code quality.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

Comments