Always prepare full references when querying

[alcaeus]

Q	A
Type	bug
BC Break	no
Fixed issues	Related to #1837

Summary

I noticed that the changes made in #2827 always created partial queries for references. This can lead to wrong results when using discriminated references where identifiers are reused across collections (e.g. when using ascending numeric identifiers). This PR fixes this, bringing the behaviour of Expr::equals in line with Expr::references and Expr::includesReferenceTo, essentially making those methods unnecessary.

This fix required some changes in tests where we made assertions on query shapes. I made sure that the changes don't introduce a regression with shard keys, and I'm sure that it doesn't. While looking at this, I noticed that the support for references as shard keys may actually not work as expected due to how indexes work when the index value is a document. In order to match a document, both key order and values need to be identical, so if a document contains a value of { foo: 'bar', bar: 'baz' }, using { bar: 'baz', foo: 'bar' } will not match despite the documents having the same PHP representation (and being equal for all intents and purposes). This may also mean that the typical reference queries of { "bar.$id": "..." } may not necessarily be able to use the underlying index of the shard key. I'm not particularly worried about this, but wanted to mention it in case someone trips over this.

I'll be following up this PR with another one to deprecate references and includesReferenceTo in 2.16.x for subsequent removal in 3.0.

[Copilot]

Pull request overview

This PR fixes a bug where partial reference queries could lead to incorrect results when using discriminated references with reused identifiers across collections. The fix ensures that Expr::equals always prepares full references (including $ref and discriminator fields) when querying, bringing its behavior in line with Expr::references and Expr::includesReferenceTo.

Key changes:

• Modified prepareReference method to always include full reference details in queries instead of creating partial queries
• Updated test assertions to expect full reference queries including $ref, $id, and discriminator fields where applicable

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

File	Description
src/Persisters/DocumentPersister.php	Simplified prepareReference to always use complete reference objects in queries
tests/Tests/Persisters/DocumentPersisterGetShardKeyQueryTest.php	Updated assertion to expect full reference with $ref field in shard key query
tests/Tests/DocumentRepositoryTest.php	Updated multiple test assertions to expect full reference queries with $ref and discriminator fields
tests/Tests/Functional/Ticket/GH2825Test.php	Added comprehensive tests for embedded reference queries using equals with all storage types

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[GromNaN]

I had difficulty understanding how it worked, but ultimately it is much simpler.

[GromNaN]

I'd rather keep a regular if instead of the ternary, for readability.

[ranfysvalle02]

Here’s a suggestion you might want to try:

if ($mapping['type'] === ClassMetadata::MANY) {
    return [[$fieldName, ['$elemMatch' => $reference]]];
} else {
    return array_map(static fn($key) => [$fieldName . '.' . $key, $reference[$key]], array_keys($reference));
}

Though I do see the PR has already been approved

[alcaeus]

@GromNaN updated, and I even added some explanatory comments while I was at it.

[GromNaN]

Nit: this line it too long, you should move some words to the 2nd line. Or make it a single line.

[alcaeus]

Done. Couldn't remember whether we do 80 or 120 characters and guessed wrong.

[GromNaN]

Suggested change

	// For ReferenceOne fields, we can use multiple conditions on individual fields, prefixed with the field name
	// of the reference
	// For ReferenceOne fields, we can use multiple conditions on individual fields,
	// prefixed with the field name of the reference