Repository navigation
Fix temporary id table columns for multi-table DQL UPDATE and DELETE - #12638
Open
alireza-aminzadeh wants to merge 1 commit into
Open
alireza-aminzadeh wants to merge 1 commit into
alireza-aminzadeh wants to merge 1 commit into
Conversation
Bulk DQL UPDATE and DELETE statements on entities of a Class Table Inheritance hierarchy keep the identifiers of the affected rows in a temporary table. The same happens when a one-to-many collection with orphan removal is replaced and its target entity is part of such a hierarchy. The columns of that table were declared with their name, type and nullability only, so the rest of the mapping of the identifier was lost: * The length of string identifiers. DBAL 4 refuses to build the declaration of a string column without length on MySQL, MariaDB and SQL Server (ColumnLengthRequired), and PostgreSQL silently creates an unbounded column. * The precision, the scale and the "fixed" and "unsigned" options. * The character set and the collation. MySQL and MariaDB gave the temporary table the defaults of the database, so comparing it with the real tables failed with "Illegal mix of collations" as soon as they differ, for instance because of the default table options of the connection. When the tables are case sensitive, the primary key of the temporary table also rejected identifiers that differ in case only. The statement was copied in three places. It is now built by Doctrine\ORM\Internal\Query\TemporaryIdTable from the field mapping of the identifier, like the SchemaTool does, including the default length of string columns. Identifiers derived from associations are resolved through PersisterHelper::getFieldMappingOfColumn(). The columns also carry the name of their type, which DBAL 4.5 reads instead of the deprecated type instance and older versions ignore. On MySQL and MariaDB, the temporary table also gets the character set and the collation of the root table when an identifier is a string: the options of #[Table], then the default table options and the charset of the connection. They are declared for the table rather than for the columns, so that types like guid are covered as well, and nothing is generated for the platforms that do not derive them from the table. Fixes doctrine#12634
alireza-aminzadeh
force-pushed
the
bugfix/GH-12634-multi-table-temporary-id-table-columns
branch
from
September 29, 2026 18:07
7ce753c to
f0f99eb
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
DQL
UPDATEandDELETEstatements on entities of a Class Table Inheritance (JOINED) hierarchy keep the identifiers of the affected rows in a temporary table while the tables of the hierarchy are processed one after the other (MultiTableUpdateExecutor,MultiTableDeleteExecutor).OneToManyPersister::deleteJoinedEntityCollection()does the same when the whole collection of a one-to-many association withorphanRemovalis replaced and its target entity is part of such a hierarchy.The columns of that temporary table were declared from
name,notnullandtypeonly, so the rest of the mapping of the identifier was lost. #12634 reports it for the length of string identifiers, but there is more to it:VARCHAR(NVARCHARon SQL Server) without length: MySQL, MariaDB and SQL Server fail while the executor is created (InvalidColumnDeclaration: Column "id" has invalid type, caused byColumnLengthRequired), PostgreSQL silently creates a column without length, SQLite is not affected. The issue mentions the two executors, the same code was copied a third time inOneToManyPersister.decimalcolumn, so a decimal identifier fails on every platform (DBAL 3 silently declaredNUMERIC(10, 0)).fixedandunsigned. ACHAR(36)identifier became aVARCHAR, an unsigned integer lost itsUNSIGNED.defaultTableOptions(utf8mb4_unicode_ciin the CI configuration), the statements fail with1267 Illegal mix of collations, even once the length is declared. With case sensitive tables (utf8mb4_bin), the case insensitive primary key of the temporary table also rejects identifiers that differ in case only (1062 Duplicate entry 'abc' for key '..._id_tmp.PRIMARY').Approach
The DDL of the temporary table was duplicated in three places, so it now lives in one internal class,
Doctrine\ORM\Internal\Query\TemporaryIdTable, used by all of them.FieldMappingof the identifier, like theSchemaTooldoes for the tables of the entities: type,length(with the same default forstringcolumns,Configuration::getDefaultStringTypeSchemaLength()),precision,scale, thefixedandunsignedoptions and thecharsetandcollationoptions of the column. An identifier derived from an association is resolved to the field it references; for this,PersisterHelper::getTypeOfColumn()is split into an internalgetFieldMappingOfColumn()(its own behavior is unchanged). The definitions also carry the type name undertypeName: DBAL 4.5 reads it and deprecates theTypeinstance undertype(DeprecateColumn::getType()in favor ofColumn::getTypeName()dbal#7490), older versions only knowtypeand ignore the extra key.DEFAULT CHARACTER SET … COLLATE …, taken from theoptionsof the#[Table]of the root entity first, then from thedefaultTableOptionsof the connection, with thecharsetconnection parameter as the last resort. No query is needed for this, it only relies on the mapping and the connection parameters.COLLATEto the columns. I went for the table-level options instead: a column-level collation does not reach the types that are stored as strings but do not take a collation from the column declaration (guidis aCHAR(36),ascii_string), and the table-level clause is only generated where the platform derives the collation of the columns from it, so the statements for PostgreSQL, SQL Server and SQLite do not change.columnDefinitionof the identifier is deliberately not taken over, it is platform specific and was not before either.Changes
src/Internal/Query/TemporaryIdTable.php(new): builds theCREATE TEMPORARY TABLEstatement.src/Query/Exec/MultiTableUpdateExecutor.php,src/Query/Exec/MultiTableDeleteExecutor.php,src/Persisters/Collection/OneToManyPersister.php: use it instead of the copied code.src/Utility/PersisterHelper.php: new@internalgetFieldMappingOfColumn(),getTypeOfColumn()delegates to it.phpstan-baseline.neon: the three identicalgetColumnDeclarationListSQL()entries become one. The array keyed by column name is needed by DBAL 3, DBAL 4 wants a list whose items have aname, so it is both.docs/en/reference/inheritance-mapping.rst,docs/en/reference/dql-doctrine-query-language.rst: document how bulk statements work on a Class Table Inheritance hierarchy, what the columns of the temporary table are declared from, and that the database user needs the privilege to create temporary tables.Tests
GH12634Test(functional): bulkUPDATEandDELETEwith a fixed-length string identifier, a string identifier without length and identifiers that differ in case only in a case sensitive table, plus the deletion of a replaced orphan removal collection. Without the fix it fails on MySQL and MariaDB; PostgreSQL and SQLite pass either way, as the issue says.TemporaryIdTableTest(unit, no database needed): the generated statement for MySQL, MariaDB, PostgreSQL, SQL Server and SQLite, for the different types and options, including an identifier derived from an association, and the precedence of the charset and collation sources.MultiTableExecutorTest,PersisterHelperTest(unit): the statements built by the executors, and the resolution of a column to its field mapping.The identifier derived from an association has no functional test on purpose: creating the schema of a
JOINEDhierarchy like that makes theSchemaTooldeclare the inheritance foreign key twice, which DBAL 4.5 reports as a deprecation (unrelated to this change).Checked with DBAL 4.5.0 and 3.10.0 on PHP 8.4:
GH12634Testpasses on SQLite, MySQL 8.0, MariaDB 11.4 and PostgreSQL 14, and the whole functional suite passes on MySQL, MariaDB and PostgreSQL (DBAL 4.5.0). PHPStan (both configurations) andphpcsare clean. SQL Server is only covered by the statement tests, I had no server to run it against. On my Windows machine four unrelated tests fail with and without this change (the SQLite build has noSQRT, and the console output width of two debug command tests).The
PHPUnit (fail on deprecations)job is already red on3.7.x, because DBAL 4.5 deprecates passingTypeinstances (doctrine/dbal#7490). With deduplication on, only the first occurrence is reported, and that used to be the column declaration of the temporary table (AdvancedDqlQueryTest::testUpdateAs). This change removes it, so the job now reports the next one,DatabaseDriverTest::testIssue2059. Behind it, hidden by the deduplication, are others that are not related to this change either: tests that passTypeinstances toColumn(DatabaseDriverTest,DDC2387Test,GH7684Test) andDatabaseDrivercallingColumn::getType(). I left them alone; this change adds no deprecation of its own.Thanks to @mostafasy for the detailed analysis in the issue.
Fixes #12634