Fix false positive changes for generated columns #12018
Conversation
src/UnitOfWork.php
Outdated
|
|
||
| foreach ($actualData as $propName => $actualValue) { | ||
| if (! isset($class->associationMappings[$propName])) { | ||
| if (isset($class->fieldMappings[$propName]->notInsertable)) { |
There was a problem hiding this comment.
checking for isset does not make sense. Both true and false boolean values will be considered set.
There was a problem hiding this comment.
Sounds logic to me. Will change it.
I followed the style of the BasicEntityPersister but I think the same issue is over there:
orm/src/Persisters/Entity/BasicEntityPersister.php
Lines 631 to 633 in ef607f2
I think its working right now because the actual type seems to be null|true instead of null|bool looking at:
Its only set, if the flag is false.
orm/src/Mapping/Builder/FieldBuilder.php
Lines 98 to 100 in ef607f2
Shall I patch it in this PR or create a new one?
There was a problem hiding this comment.
joy of using confusing types like null|true instead of bool for a boolean use case...
Based on the existing type, the code with isset is correct then.
There was a problem hiding this comment.
Yeah, I completely agree. I've already changed it, but would you prefer I revert it back to isset?
Alternatively, I can create a new draft PR, since the type is defined as bool|null and this could help prevent confusion like this in the future.
| type: 'datetime_immutable', | ||
| insertable: false, | ||
| updatable: false, | ||
| columnDefinition: 'TIMESTAMP DEFAULT CURRENT_TIMESTAMP', |
There was a problem hiding this comment.
this column definition is not portable across platforms, while your test is not restricted to some platforms.
UnitOfWork was incorrectly detecting changes for database-generated columns with insertable: false and updatable: false, causing entities to be scheduled for update when only generated values changed. This adds checks to skip notInsertable fields for NEW entities and notUpdatable fields for MANAGED entities during change detection, aligning UnitOfWork behavior with BasicEntityPersister. Fixes doctrine#12017
SherinBloemendaal
force-pushed
the
fix-generated-fields-changeset-detection
branch
from
3a0fb22 to
860a64c
Compare
|
Can you please retarget on 3.5.x? This got closed because I deleted 3.4.x |
|
Yes will do that within a few days. Thanks! |
|
I apologize for the delay. I have retargeted it on |
3 participants
UnitOfWork was incorrectly detecting changes for database-generated columns with insertable: false and updatable: false, causing entities to be scheduled for update when only generated values changed.
This adds checks to skip notInsertable fields for NEW entities and notUpdatable fields for MANAGED entities during change detection, aligning UnitOfWork behavior with BasicEntityPersister.
Fixes #12017