[TMP] IBX-12043: Upgraded to Doctrine DBAL 4 - #800
Conversation
ViniTou
changed the title
5b4f3bf to
863bf37
Compare
863bf37 to
14a4b12
Compare
cdd3c1c to
130480a
Compare
| if ($platform instanceof PostgreSQLPlatform) { | ||
| // Cast through text first: NULLIF cannot compare a native JSON column against ''. | ||
| return sprintf( | ||
| "json_extract_path_text(CAST(NULLIF(CAST(%s AS TEXT), '') AS JSON), %s)", | ||
| $value, | ||
| $key, | ||
| ); | ||
| } | ||
|
|
||
| if ($platform instanceof AbstractMySQLPlatform) { | ||
| return sprintf( | ||
| "JSON_UNQUOTE(JSON_EXTRACT(NULLIF(CAST(%s AS CHAR), ''), CONCAT('$.\"', %s, '\"')))", | ||
| $value, | ||
| $key, | ||
| ); | ||
| } |
There was a problem hiding this comment.
I'm not sure, but that may break index usage on those columns (Generalized Inverted Index on JSON are an amazing feature and I'd prefer it remains working), with all those casts and NULLIFs. Especially the MySQL CONCAT.
I'm okay with leaving it as it is now - working. But would love to work more on it so we can actually leverage the database for those queries. I know for a fact that Postgres / MySQL can index those properly, only MariaDB refuses to (because JSON is actually LONGTEXT in the engine).
There was a problem hiding this comment.
Right, they did. Fixed — the function now emits the platform's accessor and nothing else: names ->> ? on PostgreSQL, JSON_UNQUOTE(JSON_EXTRACT(names, CONCAT(…))) on MySQL/MariaDB.
With jsonb and a pg_trgm GIN index on lower(names->>'eng-GB'), 50k rows: with the casts Seq Scan, without them Bitmap Index Scan.
Unindexed they still cost real time — same plan both ways, 50k rows, 3 paired runs:
| with casts | without | |
|---|---|---|
| MySQL 8.4 | 30.4–32.5 ms | 11.3–12.4 ms |
| MariaDB 11 | 24.0–29.1 ms | 17.1–18.5 ms |
| PostgreSQL 14 | 20.8–23.1 ms | 13.0–13.7 ms |
MySQL loses most: CAST(names AS CHAR) serialises the binary JSON back to text and JSON_EXTRACT reparses it, per row.
One correction to the plan: MySQL will not use a functional index for LIKE at all. Same index, same expression — = gives ref, > gives range, LIKE 'x%' gives ALL. A stored generated column works for a prefix, but nothing indexes a leading wildcard there. PostgreSQL is the only one of the three that can.
Types::JSONB and the index belong in a follow-up. The type is portable (only PostgreSQL's DDL changes), but pg_trgm is a privileged CREATE EXTENSION and should not land in an upgrade script silently.
There was a problem hiding this comment.
pg_trgm is going a little bit too far, though it's an interesting idea.
I'd be happy to get the JSONB functionality, but currently it's not critical - and we will be able to migrate to it later on. MySQL we will need to review and check if GIN or similar indexes can be added, if this functionality even exists there.
Even sequential table scan is good as long as we don't waste CPU cycles on data casting back and forth. The improved performance you've shown is good enough - though I'd be willing to spend more time afterwards to see how far we could push supported databases (and how).
DBAL 4 turns ParameterType and ArrayParameterType into enums, makes ExpressionBuilder and join conditions string-only, drops the "platform" connection parameter, and removes QueryBuilder::execute(), Statement::execute(), Result::fetch() and getQueryPart(). Because a custom platform can no longer ride on the connection, DDL-generating call sites resolve Ibexa's platform explicitly through DbPlatformFactory. Four long-suppressed baseline entries also became runtime TypeErrors once DBAL added native parameter types. The PostgreSQL CI job passed the server version as "server_version", which DoctrineBundle never reads; it hands DBAL 4 an empty string instead and the PostgreSQL driver rejects it. The query parameter is "serverVersion", and its value is derived from the matrix image. MariaDB platforms no longer extend MySQLPlatform in DBAL 4, so the random sort clause handler stopped recognising them and every MariaDB installation lost its Random sort clause. It now matches AbstractMySQLPlatform, the common ancestor of both vendors. Tables generated from the Yaml schema now state a character set and collation, which DBAL 3 used to fill in and DBAL 4 leaves to the server. Entity-backed tables share those connections, so the prepended Doctrine configuration gives them the same options; otherwise the two halves of the schema disagree and MySQL rejects joins between their character columns. Test fixtures pass the same options through LegacySchemaImporter so suites generate the DDL an installation gets.
PHPStan 2.2.9 stopped reporting isset.property on non-nullable typed properties, so the ignores for $mainLanguageCode became unmatched and failed the build.
Points ibexa/doctrine-schema at their dbal-4-upgrade branches so this one can resolve before they are merged. Revert this commit once they are.
130480a to
e29699a
Compare
|
| run: composer run-script integration | ||
| env: | ||
| DATABASE_URL: "pgsql://postgres:postgres@localhost:${{ job.services.postgres.ports[5432] }}/testdb?server_version=10" | ||
| DATABASE_URL: "pgsql://postgres:postgres@localhost:${{ job.services.postgres.ports[5432] }}/testdb?serverVersion=${{ matrix.image == 'postgres:18' && '18' || '14' }}" |
There was a problem hiding this comment.
We could replace matrix.image with something like matrix.postgres_version to simplify/remove conditioning here
4 participants
Related PRs:
IBEXA_JSON_TEXT)dbal-4-upgradeImportant
composer.jsoncarries a[TMP]pointer"ibexa/doctrine-schema": "dev-dbal-4-upgrade as 6.0.x-dev"so this branch can resolve the unreleased schema package. It must be reverted to~6.0.x-devonce doctrine-schema#45 merges, before this PR is mergeable.Description:
Stage 3 of the staged Doctrine upgrade (stages 1 and 2 — DBAL 3 cleanup and ORM 3 — already shipped under IBX-12043).
ibexa/coreandibexa/doctrine-schemaare the two packages pinningdoctrine/dbalto 3.x, so both have to move for anything downstream to follow. The constraint goes straight to^4.4.4rather than widening.Most of the diff is the mechanical consequence of four DBAL 4 changes:
ParameterType/ArrayParameterTypebecame enums (so the remaining legacyPDO::PARAM_*constants are gone, andarray_unique()over them needsSORT_REGULAR),ExpressionBuilderand join conditions are string-only,QueryBuilder::select()/addSelect()became variadic, andQueryBuilder::execute(),Statement::execute(),Result::fetch(),getQueryPart()andupdate()'s alias parameter were removed.The part worth a reviewer's attention is platform handling. DBAL 4 removed the
platformconnection parameter, and DoctrineBundle deprecated itsplatform_serviceequivalent in 2.9 for the same reason. This is silent: the connection still accepts and echoes the parameter but returns the stock platform, soSqliteDbPlatform's schema-generation behaviour — notably keeping composite primary keys on tables SQLite cannot express withAUTOINCREMENT— was being dropped without any error. Rather than reinstating the removed behaviour behind a driver middleware, DDL-generating call sites now resolve the Ibexa platform explicitly through the existingDbPlatformFactory, which keeps the choice visible at the point of use.CoreInstallertherefore takesDbPlatformFactoryInterfaceas a required constructor argument, andDatabasePlatformResolver/DatabasePlatformNamemove down toibexa/doctrine-schema, which owns the DBAL abstraction and whichibexa/coredepends on.Two latent bugs surfaced on the way:
updateAlwaysAvailableFlag()comparedexecuteQuery()'sResultagainst0(always false — it neededexecuteStatement()), and thedata_float*columns bound with anullparameter type that DBAL 4 rejects outright.MariaDB regressed silently on top of that. DBAL 4 reparents
MariaDBPlatformfromMySQLPlatformtoAbstractMySQLPlatform, soMySqlRandom::supportsPlatform()stopped recognising it andRandomSortClauseHandlerFactorythrewNo RandomSortClauseHandler found for driver Doctrine\DBAL\Platforms\MariaDB1010Platformas soon as the legacy search handler was initialised — every MariaDB installation, not just the Random sort clause. It now matchesAbstractMySQLPlatform, the ancestor both vendors share, and a new test pins the platform-to-handler mapping across MySQL 8.0/8.4 and MariaDB 10.10/11. No repository in the organisation runs MariaDB in CI, so this only surfaced by running a downstream integration suite against MariaDB 11 by hand.One addition rather than a port:
IBEXA_JSON_TEXT(document, key), a DQL string function registered on every Ibexa entity manager.Why it exists. DBAL 4 removed the
arrayandobjecttypes, which stored a value by runningserialize()into a TEXT column andunserialize()on the way back. An entity still mapping one of them does not load at all, so every such column has to move to another encoding, and JSON is the one that keeps the column TEXT and needs noALTER TABLE. That switch has a knock-on wherever the column was also searched:serialize()writes text verbatim, soLOWER(col) LIKE '%zwierzęta%'matched the raw blob by accident, whilejson_encode()escapes non-ASCII to\uXXXXandLOWER()cannot see through an escape. Pattern-matching the stored document stops working, and the query has to read the value out of it instead.DQL cannot express that on its own — Doctrine's position is that too few supported platforms have a native JSON type, and those that do each invented their own operator syntax, so a custom function is the extension route it documents.
IBEXA_JSON_TEXTreads one top-level key out of a JSON document as text, so the result composes withLOWER()andLIKE. It works over a text column or a nativeJSONone — on PostgreSQL and MySQL the value is cast through text before the empty-string guard, because neither can compare a native JSON column against''— so a consumer converting its column does not have to change the function. Emitted asjson_extract_path_texton PostgreSQL,JSON_UNQUOTE(JSON_EXTRACT(...))on MySQL and MariaDB, andjson_extracton SQLite; NULL, an empty document and an absent key all read as NULL. The database also decodes\uXXXXduring extraction, so the stored encoding stops mattering.ibexa/taxonomyis the first and so far only consumer, for its per-language name map.ibexa/connector-payummakes the sameobject→ JSON move without needing this, because nothing ever queries its column — the contrast is what the function is for: it is needed only where a JSON column is searched.Note that DBAL 4.4 deprecates declaring an auto-increment column inside a composite primary key, which
ibexa_content_type,ibexa_content_type_field_definitionandibexa_content_fieldall do. That is a schema design question well beyond this upgrade and is left as-is.For QA:
Schema installation and the legacy storage/search gateways are the surfaces touched. Taxonomy entry search exercises the new DQL function; it is covered from ibexa/taxonomy#438. Worth verifying a fresh install and a reinstall over an existing schema on MySQL, MariaDB and PostgreSQL, since SQLite is the only backend exercised by the automated suites here. MariaDB deserves particular attention: nothing in CI covers it, and content search with a Random sort clause is the specific path that was broken.
Documentation:
N/A