[bug] Fix current binding count when previous binding is false #409
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This bug is difficult to explain clearly so I will try my best.
I discovered it when I was trying query where a relation had a false column then using a
whereRaw
after. See this example below from the added test in this PR:Issue
Before this PR fix the cache key that is generated would be missing the 'Mixed Clause' binding see below where the key ends without the value.
Workaround
If you use zero instead as the value
$query->where('is_famous', 0)
the key is generated correctly.Cause
This is caused by false bindings being ignored in
CacheKey::getValuesFromBindings
and thecurrentBindings
count was therefore not being incremented correctly.Fix
Rather than
CacheKey::getValuesFromBindings
comparing using false it should use some specific value to compare with so it only skips if the binding is not set. Feel free to feedback on this but its a necessary evil to have some sort of unique string provided that is unlikely to be passed as a field value.And this ensures that the generated cache key contains all the correct bindings:
Potential Additional Change?
Not sure if you noticed but when the value is
false
the generated cache key inserts thefalse
as an empty string-is_famous_=_-
.I wonder if we should detect
false
values and make the key insert them as0
or'false'
string, e.g.-is_famous_=_0-
oris_famous_=_false-
?