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

[6.x] Fix infinite value for RedisStore #31348

Merged
merged 2 commits into from
Feb 4, 2020
Merged

Conversation

driesvints
Copy link
Member

@driesvints driesvints commented Feb 4, 2020

This PR fixes storing infinite values for the Redis cache store. Since INF values will be converted to strings upon storing in Redis we'll need to serialize them.

Fixes #31345

@halaei
Copy link
Contributor

halaei commented Feb 4, 2020

That is fine for 6.x, except that you forget NAN.
For 7.x, type mismatches should also be handled, in a different PR.

@driesvints
Copy link
Member Author

@halaei fixed NAN as well.

For 7.x, type mismatches should also be handled, in a different PR.

I don't know what you mean by this.

@halaei
Copy link
Contributor

halaei commented Feb 4, 2020

@driesvints

Cache::put('test', 1);
Cache::get('test'); // expected: integer 1, actual: string "1"

@taylorotwell taylorotwell merged commit fbf6f6c into 6.x Feb 4, 2020
@driesvints driesvints deleted the redis-infinite-value branch February 4, 2020 13:59
@driesvints
Copy link
Member Author

@halaei not sure that's solvable tbh

@halaei
Copy link
Contributor

halaei commented Feb 4, 2020

@driesvints I think it is somehow solvable, even with no trouble for migration from 6.x to 7.x, except performance downsides for redis memory usage if someone is caching a lot of integer strings, e.g. "1" instead of 1.

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.

Redis cache store issues on numeric values
3 participants