11783 failure with indexed relation #11792
Conversation
dbannik
force-pushed
the
11783-failure-with-indexed-relation
branch
from
867f8c9 to
dae1522
Compare
|
Also, please squash the commits together (since you wouldn't revert one without the others, right?), and improve the remaining commit message according to the contributing guide. Specifically, explain both the bug and the fix. |
dbannik
force-pushed
the
11783-failure-with-indexed-relation
branch
from
b0766d6 to
43d33aa
Compare
done |
|
@greg0ire The problem with phpstan checking still exists, it should be fixed in a separate commit |
|
See #11793 |
dbannik
force-pushed
the
11783-failure-with-indexed-relation
branch
from
43d33aa to
47a3015
Compare
Rebased PR |
if you are doing 2 things in one commit, that's bad. You should be doing 2 commits with clear messages, so that if one of them is wrong, it can be understood and reverted while the other stays. |
dbannik
force-pushed
the
11783-failure-with-indexed-relation
branch
from
47a3015 to
2b62d35
Compare
these 2 things can't be separated |
| @@ -611,12 +616,13 @@ public function refresh(array $id, $entity, $lockMode = null) | |||
| * | |||
| * @return CollectionCacheKey | |||
| */ | |||
| protected function buildCollectionCacheKey(array $association, $ownerId) | |||
| protected function buildCollectionCacheKey(array $association, $ownerId, string $filterHash) | |||
There was a problem hiding this comment.
@greg0ire Is introducing a new argument to a protected method okay in a version-branch that's already released? It may break projects that are implementing the Abstract class.
There was a problem hiding this comment.
I didn't spot that one. You're right, it's a breaking change for extending classes, that suddenly need to have that extra argument, and for callers, that need to provide it.
@dbannik please resort to func_get_args(). The new argument should be added in the next major release.
There was a problem hiding this comment.
@greg0ire instead of func_get_args can we use default value ?
| protected function buildCollectionCacheKey(array $association, $ownerId, string $filterHash) | |
| protected function buildCollectionCacheKey(array $association, $ownerId, string $filterHash = '') |
There was a problem hiding this comment.
That would still be a breaking change for extending classes (they would suddenly have incompatible signatures).
The bug related (doctrine#11694) and fixed mapping of sql column alias to field in entity (doctrine#11783) and invalidate cache [cache/persisted/entity|cache/persisted/collection] when sql filter changes
dbannik
force-pushed
the
11783-failure-with-indexed-relation
branch
from
2b62d35 to
6755bb0
Compare
|
Thanks @dbannik ! |
Fix issue #11783