fix(db): introspect the Oracle schema in a fixed number of queries [10.16] - #41819
Conversation
…racle
Oracle cannot store empty strings, so the file cache converts them to
null before writing a row. For the storage root, whose path is the empty
string, that left md5() being called with null: PHP 8 reports that as a
deprecated implicit null to string conversion, which is noise in the log
and an error under PHPUnit's strict error handling. The stored path_hash
itself was never wrong, because md5(null) coerces to md5('').
Found by running the PHPUnit DB suite against Oracle.
Backport of #41808
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Signed-off-by: Thomas Müller <323649642+oc-tmueller@users.noreply.github.com>
To be dropped before merge. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: Thomas Müller <323649642+oc-tmueller@users.noreply.github.com>
484c394 to
5183a31
Compare
…deberg outages [10.16] (#41816) fix(composer): consume icewind deps from GitHub mirrors to survive Codeberg outages Backport of #41707 to 10.16. owncloud CI intermittently fails because Composer must git-clone icewind/streams and icewind/smb from codeberg.org (no dist archive on Packagist), and Codeberg regularly returns HTTP 503/504. Every job that installs Composer dependencies is affected, including "Make local core" in downstream app builds. Add VCS repositories pointing at the GitHub mirrors so Composer fetches a dist zipball via the GitHub API and never touches Codeberg at install time. Unlike master, 10.16 stays on icewind/smb v3.7.0: the 3.8.x line requires php >= 8.2 while this branch pins platform php 7.4. Both packages are re-locked at the exact refs already in the lock (smb e6904cb, streams cb2bd3e), which the mirrors carry, so this is a pure source repoint with no version change. No codeberg.org references remain in composer.lock. Mirrors are kept in sync weekly by DeepDiver1975/codeberg-mirrors. Signed-off-by: Thomas Müller <323649642+oc-tmueller@users.noreply.github.com> Co-authored-by: Thomas Müller <323649642+oc-tmueller@users.noreply.github.com> Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
…0.16] doctrine/dbal 2.13 describes a schema table by table: for every table it runs one query for the columns, one for the indexes, one for the foreign keys and one for the table comment. Every one of those inlines the table name as a literal, so Oracle cannot share cursors and hard parses each of them at a cost of a few hundred milliseconds. OC\DB\Migrator asks for the full schema once per applied migration, which multiplies that per-table cost by the number of migrations. Add an OracleSchemaManager that reads the whole data dictionary in a fixed number of queries and use it for Oracle connections. Reading a 48 table schema drops from 194 to 6 queries, and occ maintenance:install against Oracle on PHP 7.4 drops from 43 minutes to 30 seconds. doctrine/dbal itself works this way from 3.4 onwards, so master is unaffected; its 3.x line cannot be pulled into 10.16 because it changes public API that third-party apps rely on. The introspected schema is unchanged. The added test asserts that the batched result is identical to the stock one according to Doctrine's Comparator, and that the query count does not grow with the number of tables. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: Thomas Müller <323649642+oc-tmueller@users.noreply.github.com>
5183a31 to
3254703
Compare
|
Retargeted onto the branch of #41815, because Oracle CI does not exist on Two consequences worth noting for review:
|
75b0b29 to
3566f5d
Compare
|
CI now confirms the measurements. Same workflow, same runner class, same
The baseline job never executed a single test. This is the first time the Oracle PHPUnit suite on All checks are green: |
3eb205d
into
ci/oracle-db-in-github-actions-10.16
| run: | | ||
| make install-composer-deps | ||
|
|
||
| - name: TEMPORARY DEBUG - Oracle client |
There was a problem hiding this comment.
Is all this temp debug needed?
Back it out?
@oc-tmueller
The Oracle job carried two pieces of throwaway diagnostics that were only there to find out why maintenance:install took an hour on this branch: a "TEMPORARY DEBUG - Oracle client" step probing oci8, the instant client and a raw connect, and a v$session/v$sql sampler that wrapped the install in a background job with a 90 minute kill timeout and printed the top SQL afterwards. That question is answered - it was doctrine/dbal 2.13's per table schema introspection, fixed by #41819 - and the install is back to under a minute, so the instrumentation has no reason to stay. Installing on Oracle is a plain "php occ ${install_cmd}" again, like every other database. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: Thomas Müller <323649642+oc-tmueller@users.noreply.github.com>
…0.16] (#41819) * fix: avoid a deprecation notice when hashing the file cache path on Oracle Oracle cannot store empty strings, so the file cache converts them to null before writing a row. For the storage root, whose path is the empty string, that left md5() being called with null: PHP 8 reports that as a deprecated implicit null to string conversion, which is noise in the log and an error under PHPUnit's strict error handling. The stored path_hash itself was never wrong, because md5(null) coerces to md5(''). Found by running the PHPUnit DB suite against Oracle. Backport of #41808 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: Thomas Müller <323649642+oc-tmueller@users.noreply.github.com> * tmp: debug oracle client on php 7.4 To be dropped before merge. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: Thomas Müller <323649642+oc-tmueller@users.noreply.github.com> * fix(composer): consume icewind deps from GitHub mirrors to survive Codeberg outages [10.16] (#41816) fix(composer): consume icewind deps from GitHub mirrors to survive Codeberg outages Backport of #41707 to 10.16. owncloud CI intermittently fails because Composer must git-clone icewind/streams and icewind/smb from codeberg.org (no dist archive on Packagist), and Codeberg regularly returns HTTP 503/504. Every job that installs Composer dependencies is affected, including "Make local core" in downstream app builds. Add VCS repositories pointing at the GitHub mirrors so Composer fetches a dist zipball via the GitHub API and never touches Codeberg at install time. Unlike master, 10.16 stays on icewind/smb v3.7.0: the 3.8.x line requires php >= 8.2 while this branch pins platform php 7.4. Both packages are re-locked at the exact refs already in the lock (smb e6904cb, streams cb2bd3e), which the mirrors carry, so this is a pure source repoint with no version change. No codeberg.org references remain in composer.lock. Mirrors are kept in sync weekly by DeepDiver1975/codeberg-mirrors. Signed-off-by: Thomas Müller <323649642+oc-tmueller@users.noreply.github.com> Co-authored-by: Thomas Müller <323649642+oc-tmueller@users.noreply.github.com> Co-authored-by: Claude Opus 5 <noreply@anthropic.com> * fix(db): introspect the Oracle schema in a fixed number of queries [10.16] doctrine/dbal 2.13 describes a schema table by table: for every table it runs one query for the columns, one for the indexes, one for the foreign keys and one for the table comment. Every one of those inlines the table name as a literal, so Oracle cannot share cursors and hard parses each of them at a cost of a few hundred milliseconds. OC\DB\Migrator asks for the full schema once per applied migration, which multiplies that per-table cost by the number of migrations. Add an OracleSchemaManager that reads the whole data dictionary in a fixed number of queries and use it for Oracle connections. Reading a 48 table schema drops from 194 to 6 queries, and occ maintenance:install against Oracle on PHP 7.4 drops from 43 minutes to 30 seconds. doctrine/dbal itself works this way from 3.4 onwards, so master is unaffected; its 3.x line cannot be pulled into 10.16 because it changes public API that third-party apps rely on. The introspected schema is unchanged. The added test asserts that the batched result is identical to the stock one according to Doctrine's Comparator, and that the query count does not grow with the number of tables. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: Thomas Müller <323649642+oc-tmueller@users.noreply.github.com> --------- Signed-off-by: Thomas Müller <323649642+oc-tmueller@users.noreply.github.com> Co-authored-by: Thomas Müller <323649642+oc-tmueller@users.noreply.github.com> Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
The Oracle job carried two pieces of throwaway diagnostics that were only there to find out why maintenance:install took an hour on this branch: a "TEMPORARY DEBUG - Oracle client" step probing oci8, the instant client and a raw connect, and a v$session/v$sql sampler that wrapped the install in a background job with a 90 minute kill timeout and printed the top SQL afterwards. That question is answered - it was doctrine/dbal 2.13's per table schema introspection, fixed by #41819 - and the install is back to under a minute, so the instrumentation has no reason to stay. Installing on Oracle is a plain "php occ ${install_cmd}" again, like every other database. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: Thomas Müller <323649642+oc-tmueller@users.noreply.github.com>
…suite against Oracle again [10.16] (#41815) * ci: run the PHPUnit DB suite against Oracle again Oracle was never covered by the GitHub Actions workflows: in drone it ran only in the nightly cron matrix, and that pipeline is not part of the GitHub Actions setup. Add it as a php-unit.yml matrix entry driven by a new databases input, served by gvenzl/oracle-free:23-slim-faststart. Oracle lives in the FREEPDB1 pluggable database, so the installer is called with a connect string carrying SERVICE_NAME instead of a host. The oci8 extension is only requested for Oracle jobs, and DB_TYPE is only exported for Oracle so test-phpunit.sh narrows the run to --group DB - the other databases keep running the full suite unchanged. The Oracle job runs on every pull request and shares its display name with the existing php-unit job, so all databases appear in one group of checks. It remains a separate job so that a slow or failing Oracle run cannot cancel the other databases through fail-fast. Backport of #41808 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: Thomas Müller <323649642+oc-tmueller@users.noreply.github.com> * fix(db): introspect the Oracle schema in a fixed number of queries [10.16] (#41819) * fix: avoid a deprecation notice when hashing the file cache path on Oracle Oracle cannot store empty strings, so the file cache converts them to null before writing a row. For the storage root, whose path is the empty string, that left md5() being called with null: PHP 8 reports that as a deprecated implicit null to string conversion, which is noise in the log and an error under PHPUnit's strict error handling. The stored path_hash itself was never wrong, because md5(null) coerces to md5(''). Found by running the PHPUnit DB suite against Oracle. Backport of #41808 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: Thomas Müller <323649642+oc-tmueller@users.noreply.github.com> * tmp: debug oracle client on php 7.4 To be dropped before merge. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: Thomas Müller <323649642+oc-tmueller@users.noreply.github.com> * fix(composer): consume icewind deps from GitHub mirrors to survive Codeberg outages [10.16] (#41816) fix(composer): consume icewind deps from GitHub mirrors to survive Codeberg outages Backport of #41707 to 10.16. owncloud CI intermittently fails because Composer must git-clone icewind/streams and icewind/smb from codeberg.org (no dist archive on Packagist), and Codeberg regularly returns HTTP 503/504. Every job that installs Composer dependencies is affected, including "Make local core" in downstream app builds. Add VCS repositories pointing at the GitHub mirrors so Composer fetches a dist zipball via the GitHub API and never touches Codeberg at install time. Unlike master, 10.16 stays on icewind/smb v3.7.0: the 3.8.x line requires php >= 8.2 while this branch pins platform php 7.4. Both packages are re-locked at the exact refs already in the lock (smb e6904cb, streams cb2bd3e), which the mirrors carry, so this is a pure source repoint with no version change. No codeberg.org references remain in composer.lock. Mirrors are kept in sync weekly by DeepDiver1975/codeberg-mirrors. Signed-off-by: Thomas Müller <323649642+oc-tmueller@users.noreply.github.com> Co-authored-by: Thomas Müller <323649642+oc-tmueller@users.noreply.github.com> Co-authored-by: Claude Opus 5 <noreply@anthropic.com> * fix(db): introspect the Oracle schema in a fixed number of queries [10.16] doctrine/dbal 2.13 describes a schema table by table: for every table it runs one query for the columns, one for the indexes, one for the foreign keys and one for the table comment. Every one of those inlines the table name as a literal, so Oracle cannot share cursors and hard parses each of them at a cost of a few hundred milliseconds. OC\DB\Migrator asks for the full schema once per applied migration, which multiplies that per-table cost by the number of migrations. Add an OracleSchemaManager that reads the whole data dictionary in a fixed number of queries and use it for Oracle connections. Reading a 48 table schema drops from 194 to 6 queries, and occ maintenance:install against Oracle on PHP 7.4 drops from 43 minutes to 30 seconds. doctrine/dbal itself works this way from 3.4 onwards, so master is unaffected; its 3.x line cannot be pulled into 10.16 because it changes public API that third-party apps rely on. The introspected schema is unchanged. The added test asserts that the batched result is identical to the stock one according to Doctrine's Comparator, and that the query count does not grow with the number of tables. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: Thomas Müller <323649642+oc-tmueller@users.noreply.github.com> --------- Signed-off-by: Thomas Müller <323649642+oc-tmueller@users.noreply.github.com> Co-authored-by: Thomas Müller <323649642+oc-tmueller@users.noreply.github.com> Co-authored-by: Claude Opus 5 <noreply@anthropic.com> * ci: drop the temporary Oracle debug instrumentation The Oracle job carried two pieces of throwaway diagnostics that were only there to find out why maintenance:install took an hour on this branch: a "TEMPORARY DEBUG - Oracle client" step probing oci8, the instant client and a raw connect, and a v$session/v$sql sampler that wrapped the install in a background job with a 90 minute kill timeout and printed the top SQL afterwards. That question is answered - it was doctrine/dbal 2.13's per table schema introspection, fixed by #41819 - and the install is back to under a minute, so the instrumentation has no reason to stay. Installing on Oracle is a plain "php occ ${install_cmd}" again, like every other database. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Signed-off-by: Thomas Müller <323649642+oc-tmueller@users.noreply.github.com> --------- Signed-off-by: Thomas Müller <323649642+oc-tmueller@users.noreply.github.com> Co-authored-by: Thomas Müller <323649642+oc-tmueller@users.noreply.github.com> Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
The problem
occ maintenance:installagainst Oracle takes over 40 minutes on 10.16, while the same install on master finishes in under a minute. This is what makes the Oracle CI job on this branch look hung.Root cause
The bundled doctrine/dbal 2.13 introspects a schema table by table.
AbstractSchemaManager::listTables()loops overlistTableDetails(), which issues four queries per table — columns, indexes, foreign keys, table comment. Each of them inlines the table name as a literal, so Oracle cannot share cursors and hard parses every single one, measured at ~230–245 ms each.OC\DB\Migrator::getDiff()then asks for the full schema once per applied migration. With 68 migrations and ~48 tables that is thousands of hard parsed queries, so the cost grows with tables × migrations.doctrine/dbal batches this natively from 3.4 onwards (
selectTableColumns/selectIndexColumns/selectForeignKeyColumns), which is why master, on dbal 3.10, was never affected.Why not just upgrade doctrine/dbal
php ^7.4 || ^8.0— but the 2→3 migration on master touched 181 files and changes public API (Statement→Result,DBALException→Exception). Shipping that in a 10.16 patch release would break third-party apps.So this PR adds an ownCloud subclass instead. No vendor code is patched.
The change
OC\DB\OracleSchemaManagerextends Doctrine'sOracleSchemaManagerand overrideslistTables()to read the whole data dictionary in a fixed number of queries, grouping the rows by table in PHP and feeding them to the same unmodified_getPortableTable*List()consumers.OC\DB\Connection::getSchemaManager()returns it for Oracle platforms only; every other platform is untouched.The schema asset filter semantics are preserved, since the table list still comes from
listTableNames().Results
occ maintenance:install, Oracle + PHP 7.4 + oci86 flat queries is exactly what master's dbal 3.10 does.
Correctness evidence
Doctrine\DBAL\Schema\Comparatorreports no difference between the stock and the batched introspection in both directions, at 30 and at 62 tables, including edge cases: tables without comments, tables without foreign keys, a primary-key-only table, and quoted mixed-case identifiers.SYS_C…names, which differ between any two installs.tests/lib/DBpasses on Oracle (275 tests) and on SQLite (275 tests, the 3 new ones skipping), which also proves the non-Oracle fall-through.Tests
tests/lib/DB/OracleSchemaManagerTest.phpasserts that Oracle connections get the batched manager, that its result is Comparator-identical to the stock one, and that the query count does not grow when the table count triples.Known trade-off
When
setFilterSchemaAssetsExpressionfilters out most tables, the batched queries still scan the whole user schema and discard the extra rows. More rows scanned, but 6 queries instead of 4N — and this is what dbal 3.4+ does too.🤖 Generated with Claude Code