Skip to content

Conversation

@acrobat
Copy link

@acrobat acrobat commented Nov 5, 2025

The default charset and collation are not present in the options array of the new columns and by just removing and re-adding the columns this change in the diff is lost thus generating equal alter table queries.

I've switched to the doctrine schema comparator to check for actual differences between the two table. This way it doesn't generate any sql the table stays the same.

Before change:

audit:schema:update --dump-sql
 The following SQL statements will be executed:

     CREATE TABLE brand_audit (id INT UNSIGNED AUTO_INCREMENT NOT NULL, type VARCHAR(10) NOT NULL, object_id VARCHAR(255) NOT NULL, discriminator VARCHAR(255) DEFAULT NULL, transaction_hash VARCHAR(40) DEFAULT NULL, diffs JSON DEFAULT NULL, blame_id VARCHAR(255) DEFAULT NULL, blame_user VARCHAR(255) DEFAULT NULL, blame_user_fqdn VARCHAR(255) DEFAULT NULL, blame_user_firewall VARCHAR(100) DEFAULT NULL, ip VARCHAR(45) DEFAULT NULL, created_at DATETIME NOT NULL, INDEX type_ed4e375f24635767d50412c3733f9a3c_idx (type), INDEX object_id_ed4e375f24635767d50412c3733f9a3c_idx (object_id), INDEX discriminator_ed4e375f24635767d50412c3733f9a3c_idx (discriminator), INDEX transaction_hash_ed4e375f24635767d50412c3733f9a3c_idx (transaction_hash), INDEX blame_id_ed4e375f24635767d50412c3733f9a3c_idx (blame_id), INDEX created_at_ed4e375f24635767d50412c3733f9a3c_idx (created_at), PRIMARY KEY (id)) DEFAULT CHARACTER SET utf8mb4;

audit:schema:update --force

audit:schema:update --dump-sql
 The following SQL statements will be executed:

     ALTER TABLE brand_audit CHANGE type type VARCHAR(10) NOT NULL, CHANGE object_id object_id VARCHAR(255) NOT NULL, CHANGE discriminator discriminator VARCHAR(255) DEFAULT NULL, CHANGE transaction_hash transaction_hash VARCHAR(40) DEFAULT NULL, CHANGE blame_id blame_id VARCHAR(255) DEFAULT NULL, CHANGE blame_user blame_user VARCHAR(255) DEFAULT NULL, CHANGE blame_user_fqdn blame_user_fqdn VARCHAR(255) DEFAULT NULL, CHANGE blame_user_firewall blame_user_firewall VARCHAR(100) DEFAULT NULL, CHANGE ip ip VARCHAR(45) DEFAULT NULL;

After change:

audit:schema:update --dump-sql
 The following SQL statements will be executed:

     CREATE TABLE brand_audit (id INT UNSIGNED AUTO_INCREMENT NOT NULL, type VARCHAR(10) NOT NULL, object_id VARCHAR(255) NOT NULL, discriminator VARCHAR(255) DEFAULT NULL, transaction_hash VARCHAR(40) DEFAULT NULL, diffs JSON DEFAULT NULL, blame_id VARCHAR(255) DEFAULT NULL, blame_user VARCHAR(255) DEFAULT NULL, blame_user_fqdn VARCHAR(255) DEFAULT NULL, blame_user_firewall VARCHAR(100) DEFAULT NULL, ip VARCHAR(45) DEFAULT NULL, created_at DATETIME NOT NULL, INDEX type_ed4e375f24635767d50412c3733f9a3c_idx (type), INDEX object_id_ed4e375f24635767d50412c3733f9a3c_idx (object_id), INDEX discriminator_ed4e375f24635767d50412c3733f9a3c_idx (discriminator), INDEX transaction_hash_ed4e375f24635767d50412c3733f9a3c_idx (transaction_hash), INDEX blame_id_ed4e375f24635767d50412c3733f9a3c_idx (blame_id), INDEX created_at_ed4e375f24635767d50412c3733f9a3c_idx (created_at), PRIMARY KEY (id)) DEFAULT CHARACTER SET utf8mb4;

audit:schema:update --force

audit:schema:update --dump-sql

 [OK] Nothing to update.

Fixes DamienHarper/auditor-bundle#553
Possibly fixes #241 also (to be tested as I don't use postgres in my project)

@DamienHarper
Copy link
Owner

@acrobat thanks for the contribution but most of the tests fail. I can't merge this PR as is.

@acrobat
Copy link
Author

acrobat commented Nov 10, 2025

@DamienHarper do you have any pointers on how to debug this test failure? I can't reproduce this locally, either with the in memory sqllite or mysql db

Full testsuite:

PHPUnit 11.5.43 by Sebastian Bergmann and contributors.

Runtime:       PHP 8.4.13
Configuration: /Volumes/Data/forks/auditor/phpunit.xml

...............................................................  63 / 187 ( 33%)
............................................................... 126 / 187 ( 67%)
.............................................................   187 / 187 (100%)

Time: 00:02.397, Memory: 32.00 MB

and the specific failing test file only (with testdox output)

PHPUnit 11.5.43 by Sebastian Bergmann and contributors.

Runtime:       PHP 8.4.13
Configuration: /Volumes/Data/forks/auditor/phpunit.xml

...............                                                   15 / 15 (100%)

Time: 00:00.214, Memory: 20.00 MB

Schema Manager (DH\Auditor\Tests\Provider\Doctrine\Persistence\Schema\SchemaManager)
 ✔ Storage services setup
 ✔ Create audit table
 ✔ Update audit table
 ✔ Compute audit table name with data set #2
 ✔ Compute audit table name with data set #3
 ✔ Compute audit table name with data set #4
 ✔ Compute audit table name with data set #5
 ✔ Compute audit table name with data set #6
 ✔ Compute audit table name with data set #7
 ✔ Compute audit table name with data set #8
 ✔ Compute audit table name with data set #0
 ✔ Compute audit table name with data set #1
 ✔ Compute audit table name with data set #9
 ✔ Compute audit table name with data set #11
 ✔ Compute audit table name with data set #10

@DamienHarper
Copy link
Owner

@acrobat you can run make tests locally
By running make, you'll be presented with a help screen

@acrobat
Copy link
Author

acrobat commented Nov 10, 2025

Thanks! My bad I was also running them on the wrong branch, so I'm able to reproduce now!

The test failure comes from this "funky" test code that uses reflection to calculate the diff, this bypasses the table re-assign if there are changes. But I'm not sure how to rewrite the test as it should be based on real entity changes 🤔

$reflectedMethod = $this->reflectMethod($updater, 'processColumns');
$reflectedMethod->invokeArgs($updater, [$table, $columns, self::ALTERNATE_COLUMNS, $entityManager->getConnection()]);
$reflectedMethod = $this->reflectMethod($updater, 'processIndices');
$reflectedMethod->invokeArgs($updater, [$table, $alternateIndices, $entityManager->getConnection()]);
$this->migrate($fromSchema, $toSchema, $entityManager);

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

2 participants