Skip to content

null to falsy value detection in syncDirtyFromProperties - #39

Merged
n0nag0n merged 1 commit into
flightphp:masterfrom
enlivenapp:master
Sep 14, 2026
Merged

n0nag0n merged 1 commit into
flightphp:masterfrom
enlivenapp:master

Conversation

@enlivenapp

Copy link
Copy Markdown
Contributor

yep... I'm back again. 🤷‍♂️

ActiveRecord.php:459


syncDirtyFromProperties() compared stored vs current property values with !=. In PHP's loose comparison, null equals 0.0, 0, '', and false. So when a property was set to 0.0 over a NULL column, 0.0 != null was false, the property was never added to $dirty, and update() silently saved nothing. The reported case was writing 0.0 over NULL.

The fix changes line 459 from != to !==.

I traced the code to make sure this does not cause false "changed" flags: on a normal find-then-update, $data and the property hold the same value and type, so !== only registers genuine changes and unchanged columns are not rewritten. The copyFrom()/dirty() path that could introduce mixed types is already skipped by the existing dirty-key guard. The loose-vs-strict operators only behave differently in the null-versus-falsy case, which is precisely the bug this fixes.

Tests:

  • testUpdatePersistsZeroFloatOverNull and testSyncDetectsFloatZeroChangeFromNull reproduce the bug: they fail with !=, pass with !==.
  • testSyncDoesNotDirtyUnchangedTypedProperties proves re-assigning identical values leaves the dirty set empty, so no needless UPDATEs.

Fixture change: TypedUser gains a nullable credits float property, and the test schema gains a credits REAL column.

Compatibility:

  • The package requires PHP 7.4+. Strict comparison operators exist in every PHP version, so this is 7.4 friendly; no newer-only syntax was introduced.
  • The package's AGENTS.md calls for strict comparisons (===/!==), so this change aligns with the project's stated standard.

Verification: full suite (230 tests) green, coverage at 100%, php -l and PHPCS clean.

@n0nag0n
n0nag0n merged commit 54c8cf7 into flightphp:master Sep 14, 2026
@n0nag0n

n0nag0n commented Sep 14, 2026

Copy link
Copy Markdown
Contributor

makes sense to me! Thanks again!

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