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

Cannot safely type bigint column as int when used with dbal 3.x #11721

Open
acoulton opened this issue Nov 22, 2024 · 0 comments
Open

Cannot safely type bigint column as int when used with dbal 3.x #11721

acoulton opened this issue Nov 22, 2024 · 0 comments

Comments

@acoulton
Copy link
Contributor

Bug Report

Q A
Version 2.19.4

Summary

When using any version of ORM with DBAL 3.x, an entity with a column type bigint and a property type int is incorrectly treated as changed every time it is used. This causes high volumes of unexpected database writes (and listener invocations).

Prior to 2.19.4, orm:validate-schema would report that the column type and property type were incompatible. That was changed in #11414 to fix #11377 - as a result doctrine no longer warns about this issue.

I reported this at the time, but I think as we were discussing on a closed PR (and I didn't get a chance to follow up) it has been overlooked.

Current behavior

Given an entity like:

use Doctrine\ORM\Mapping\Column;
use Doctrine\ORM\Mapping\Entity;
use Doctrine\DBAL\Types\Types;

#[Entity]
class MyEntity {

  #[Column(type: Types::INTEGER)]
  private ?int $id = null;

  #[Column(type: Types::BIGINT)]
  private int $number = 20;
}

When I run doctrine orm:validate-schema the schema validates successfully.

If I then run code like:

// assuming an entity in the database with an id=1, number=20
$entity = $entity_manager->getRepository(MyEntity::class)->find('1');

$entity_manager->flush();

And examine the queries from my database logs, I can see two queries:

  • SELECT FROM my_entity WHERE id = 1
  • UPDATE my_entity SET number = 20 WHERE id = 1.

This happens because:

  • With DBAL 3.x, bigint is hydrated as a string
  • ORM's UnitOfWork::computeChangeSet checks whether {original value returned by dbal} === {current value of entity property} - that link is for 2.x, but the logic is the same in 3.x and 4.x
  • If the entity property is typed int then PHP has automatically typecast '15' to 15 when hydrating the object.
  • This means that the UnitOfWork checks '15' === 15, this does not match, so the entity is added to the changeset and marked for update.

Expected behavior

Either:

  • The UnitOfWork should not treat the entity as changed
  • Or the schema validation tool should report that the column and property types are incompatible.

How to reproduce

I have not created a failing test because the correct test depends on how you would prefer to solve this.

I believe that the reproduction is clear from the description and links to how the implementations cause this bug, but am happy to create an official reproduction if it would help.

Proposed fix

IMHO the best solution is probably to fix this directly in the UnitOfWork, to allow people to type properties correctly and eliminate the unnecessary writes.

The === comparison appears in both computeChangeSet and recomputeSingleEntityChangeSet.

I would suggest therefore extracting a method something like:

private function isValueChanged(mixed $original, mixed $actual): bool
{
   if (is_int($actual) && is_string($original)) {
     // it is always safe to cast int to string, it's not guaranteed safe the other way round
     return  $original === (string) $actual;
   }
   // there may be scope to support other type differences here in future

   return $original === $actual;
}

If this is acceptable I can work on a PR with implementation and tests.

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

No branches or pull requests

1 participant