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

#1297: Fix deserialization of doctrine entity with null identifier #1299

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

Toilal
Copy link

@Toilal Toilal commented Mar 17, 2021

Q A
Bug fix? yes
New feature? no
Doc updated no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets #1297
License MIT

@goetas
Copy link
Collaborator

goetas commented Mar 18, 2021

hmm, this sounds strange to me... the find method should be supposed to find entities using primary keys.... how is this feature going to behave if we have an entity identified by multiple columns?

Something like

$authorFetched = $constructor->construct($this->visitor, $class, ['id1' => null, 'id2' => 123 ], $type, $this->context);

@Toilal Toilal force-pushed the doctrine-constructor-null-identifier branch from d7b9767 to b8303f9 Compare March 25, 2021 09:55
@Toilal
Copy link
Author

Toilal commented Mar 25, 2021

@goetas I don't have this use case in my current projects, so I can't really test composite primary keys for now.

According to this issue doctrine/orm#6647, Doctrine doesn't support null fields in a composite primary key. So we probably want to skip find invocation when a single field is null.

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.

2 participants