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

[5.7] Get always true when doing a delete cache query #25579

Merged
merged 4 commits into from
Sep 11, 2018
Merged

[5.7] Get always true when doing a delete cache query #25579

merged 4 commits into from
Sep 11, 2018

Conversation

mtawil
Copy link
Contributor

@mtawil mtawil commented Sep 11, 2018

Based on this commit, the command php artisan cache:clear will always fail when using a database cache driver and cache table is empty.

Unverified

This user has not yet uploaded their public signing key.
Based on [this commit](ad670c8), the command `php artisan cache:clear` will always fail when using a database cache driver and cache table is empty.
@staudenmeir
Copy link
Contributor

staudenmeir commented Sep 11, 2018

Doesn't this always return true?

@mtawil
Copy link
Contributor Author

mtawil commented Sep 11, 2018

Well, if the cache table name is correct, and the database driver is working well, then yes, it always returns a true.
Should we set a try/catch for issues like wrong cache table name? No, because there is a misconfiguration that needs to be debugged.

@staudenmeir
Copy link
Contributor

I think it would be more consistent to explicitly use return true. See ArrayStore and RedisStore.

@mtawil
Copy link
Contributor Author

mtawil commented Sep 11, 2018

You are right. It's not that much different, but ok, I made a new change that follows the same way on RedisStore.

Thank you.

@mtawil mtawil changed the title Get true when a delete cache query returns >= 0 Get always true when doing a delete cache query Sep 11, 2018
@mtawil
Copy link
Contributor Author

mtawil commented Sep 11, 2018

Doesn't this always return true?

@staudenmeir I thought you said "Does" :)
No, it does not always return a true since v5.7 because of this change on ClearCommand.php

The current behaviour, when deleting the cache rows, it will return the number of deleted rows.
So, it will return a zero if the table is empty (0 rows affected). By casting it to bool, that zero will be false, and the clear command will going to fail. 🙂

@mtawil mtawil changed the title Get always true when doing a delete cache query [5.7] Get always true when doing a delete cache query Sep 11, 2018
@taylorotwell taylorotwell merged commit 8a9aa85 into laravel:5.7 Sep 11, 2018
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

Successfully merging this pull request may close these issues.

None yet

3 participants