Skip to content

fix(db): only cast text and binary compare columns to char on Oracle - #41783

Draft
DeepDiver1975 wants to merge 2 commits into
10.16from
fix/oracle-upsert-clob-only-cast
Draft

fix(db): only cast text and binary compare columns to char on Oracle#41783
DeepDiver1975 wants to merge 2 commits into
10.16from
fix/oracle-upsert-clob-only-cast

Conversation

@DeepDiver1975

Copy link
Copy Markdown
Member

Description

On Oracle, Adapter::upsert() wrapped every compare column in to_char(). The cast is
required for CLOB and BLOB columns — Oracle refuses to compare those with = at all
(ORA-00932) — but to_char(column) = 'literal' is not sargable, so an index on the column
can no longer be used.

Cache::put() compares storage and path_hash, which is exactly the unique index
fs_storage_path_hash, so every upload, rename and file scan degraded from an index unique
scan into an index skip scan. On the installation where this was found the statement went
from ~0.0002 s / 8–20 buffer gets to 5.7–63.7 s / 91,000–122,000 buffer gets.

The compare column types are resolved from the schema now, and only text and binary columns
are cast:

-- before
UPDATE "oc_filecache" SETWHERE to_char("storage") = '1' AND to_char("path_hash") = ''
-- after
UPDATE "oc_filecache" SETWHERE "storage" = '1' AND "path_hash" = ''

Comparing a NUMBER column to a quoted literal is safe: Oracle converts the literal to a
number, so the column and its index stay usable.

How

Rather than hand-building the cast, the per-column type is handed to the expression builder.
OCIExpressionBuilder::eq() already emits exactly to_char("col") for
IQueryBuilder::PARAM_STR, and the base ExpressionBuilder::eq() ignores its type argument
entirely, so this is a no-op on MySQL/MariaDB/PostgreSQL/SQLite — the generated SQL there is
byte-identical to before. The CLOB path on Oracle is likewise byte-identical; only non-LOB
columns change.

The resolution itself lives in AdapterOCI8, via a new protected
Adapter::getCompareColumnTypes() seam that returns nothing on every other platform. The
schema lookup passes a quoted identifier, because ownCloud creates all tables quoted and
therefore in lower case while Oracle folds unquoted identifiers to upper case — the same
reason OracleConnection::tableExists() already quotes. The result is memoized per table.

If the types cannot be resolved (unknown table, schema manager throws), every compare column
is cast, i.e. exactly the previous behaviour: slow, but it can never raise ORA-00932. A
warning is logged once per table.

Tests

tests/lib/DB/AdapterTest.php gets six new tests that fake the platform, since there is no
Oracle job in CI. They pin both halves of the behaviour:

  • non-LOB compare columns (storage, path_hash) are not cast — the regression this fixes
  • a TextType compare column (configvalue) is still cast — the ORA-00932 case
  • a LOB column whose name is a reserved word (stored quoted) is still matched
  • an unresolvable schema (throws, or unknown table) casts everything
  • other platforms cast nothing, text columns included

The existing AdapterTest call-count expectations are unchanged, which confirms there is no
collateral change to the generated queries.

Notes for the reviewer

Two pre-existing Oracle limitations are documented in IDBConnection::upsert() but
deliberately not changed here:

  • to_char() on a CLOB longer than 4000 bytes raises ORA-22835, so comparing a long CLOB
    never worked. OC\AllConfig works around this with dbms_lob.substr(configvalue, 4000, 1);
    adopting that changes behaviour for long values and belongs in its own PR.
  • to_char() does not accept a BLOB, so a binary compare column has never worked either way.
    Keeping BlobType in the cast list preserves today's behaviour rather than changing it.

One behaviour does change on Oracle: a non-numeric string compared against a NUMBER column
now raises ORA-01722 instead of silently matching no rows. No core caller does this.

This PR targets 10.16 for a customer escalation; a forward-port to master follows
separately. The code is deliberately DBAL 2/3 agnostic (getSchemaManager()), so the
cherry-pick is clean.

Fixes #41782

🤖 Generated with Claude Code

Adapter::upsert() wrapped every compare column in to_char() on Oracle. The cast
is needed for CLOB and BLOB columns, which Oracle refuses to compare with = at
all (ORA-00932), but to_char(column) is not sargable, so an index on the column
can no longer be used. Cache::put() compares storage and path_hash - exactly the
columns of the unique index fs_storage_path_hash - so every upload, rename and
file scan degraded into an index skip scan.

The compare column types are resolved from the schema now, and only text and
binary columns are cast. If the types cannot be resolved, every column is cast,
which is the previous behaviour and can never raise ORA-00932.

#41782

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Signed-off-by: Thomas Müller <1005065+DeepDiver1975@users.noreply.github.com>
@DeepDiver1975
DeepDiver1975 force-pushed the fix/oracle-upsert-clob-only-cast branch from d8240e9 to 3babcdc Compare August 21, 2026 11:47
@DeepDiver1975 DeepDiver1975 changed the title [10.16] fix(db): only cast text and binary compare columns to char on Oracle fix(db): only cast text and binary compare columns to char on Oracle Aug 21, 2026
AdapterTest has a constructor without arguments, so PHPUnit cannot hand a data
set to an instance and the provider arguments were dropped.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Signed-off-by: Thomas Müller <1005065+DeepDiver1975@users.noreply.github.com>
@DeepDiver1975
DeepDiver1975 marked this pull request as draft August 21, 2026 12:42
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

Successfully merging this pull request may close these issues.

1 participant