Skip to content

Emit CHARACTER SET for columns with an explicit encoding - #1112

Open
maurobrandoni wants to merge 1 commit into
cakephp:5.xfrom
maurobrandoni:fix/mysql-column-encoding
Open

Emit CHARACTER SET for columns with an explicit encoding#1112
maurobrandoni wants to merge 1 commit into
cakephp:5.xfrom
maurobrandoni:fix/mysql-column-encoding

Conversation

@maurobrandoni

@maurobrandoni maurobrandoni commented Aug 30, 2026

Copy link
Copy Markdown

Emit CHARACTER SET for columns with an explicit encoding

Fixes #1113.

The problem

The per-column encoding option is a no-op for every type except enum/set, and has been since 5.0. All three paths that render a column definition are affected — CREATE TABLE, ALTER TABLE ... ADD and ALTER TABLE ... CHANGE — since each one goes through MysqlAdapter::columnDefinitionSql().

$this->table('requests')
    ->changeColumn('data', 'text', ['encoding' => 'ascii'])
    ->update();
-- generated
`data` TEXT NOT NULL

The column keeps the table's default character set. No error, no warning — the option is accepted and discarded.

Where it is lost

Column::getValidOptions() accepts encoding, and Table::getChangedColumnOptions() goes as far as preserving it across a changeColumn when it is not passed explicitly. From there:

  • Column::toArray() exports 'collate' and no character-set key.
  • MysqlSchemaDialect::columnDefinitionSql() in cakephp/database renders COLLATE, but has no per-column CHARACTER SET — only the table-level DEFAULT CHARSET.
  • MysqlAdapter::columnDefinitionSql() emits CHARACTER SET only inside the enum/set backwards-compatibility branch; every other type returns the dialect's output verbatim.

That branch is the surviving fragment of Phinx's MysqlAdapter::getColumnSqlDefinition(), which appended CHARACTER SET for all column types (phinx 0.16.9, line 1374). The general case was dropped in 5.0 along with the move to cakephp/database, and the option has been documented as supported throughout:

docs/en/guides/writing-migrations/columns-and-table-operations.md:87 — "MySQL also supports collation and encoding."

Why it went unnoticed

encoding combined with collation still produces the right column, because MySQL infers the character set from the collation. Only encoding on its own silently does nothing — which is also why the existing test suite never caught it.

The fix

MySQL requires CHARACTER SET to precede COLLATE, so the fragment cannot simply be appended to the definition the dialect returns. It is rendered with a placeholder collation instead, and the CHARACTER SET/COLLATE pair replaces the placeholder in that slot.

Types that render no collation never produce the placeholder, so their definition comes back unchanged and the encoding is ignored — which matches MySQL, where an integer has no character set. The set of affected types therefore stays in sync with the dialect's own list rather than being duplicated here.

Generated SQL, 5.x vs. this PR:

Column options 5.x This PR
['encoding' => 'ascii'] `data` VARCHAR(255) NOT NULL `data` VARCHAR(255) CHARACTER SET ascii NOT NULL
['encoding' => 'ascii', 'collation' => 'ascii_bin'] `data` VARCHAR(255) COLLATE ascii_bin NOT NULL `data` VARCHAR(255) CHARACTER SET ascii COLLATE ascii_bin NOT NULL
['collation' => 'ascii_bin'] `data` VARCHAR(255) COLLATE ascii_bin NOT NULL unchanged
[] `data` VARCHAR(255) NOT NULL unchanged
integer + ['encoding' => 'ascii'] `counter` INTEGER NOT NULL unchanged

Tests

Four tests in MysqlAdapterTest, one per affected path. They assert information_schema.COLUMNS.CHARACTER_SET_NAME rather than SHOW FULL COLUMNS, which exposes only the collation and so cannot distinguish an explicit character set from an inherited one:

  • testCreateTableWithCustomEncoding — the CREATE TABLE path. Fails on 5.x (utf8mb4).
  • testAddStringColumnWithCustomEncoding — the ALTER ... ADD path, string and text. Fails on 5.x (utf8mb4).
  • testChangeColumnWithCustomEncoding — the ALTER ... CHANGE path. Fails on 5.x (utf8mb4).
  • testAddStringColumnWithCustomEncodingAndCollation — pins the CHARACTER SET / COLLATE pairing and ordering. Passes on 5.x, guards the fix.

Full suite green against MySQL: 1785 tests (MariaDB 10.11). phpcs clean. No change required in cakephp/database.

The other backends are untouched. columnDefinitionSql() is defined only in MysqlAdapter, and PostgresAdapter, SqliteAdapter and SqlserverAdapter extend AbstractAdapter rather than it, so none of them can reach this code. None of them reference encoding at all, which is correct: PostgreSQL sets its character set per database, and SQL Server and SQLite have no per-column character set either — all three offer only per-column COLLATE, which this PR does not touch. The new tests live in MysqlAdapterTest and skip on other drivers.

If you would rather see per-column charset support added to MysqlSchemaDialect in cakephp/database and consumed from here, I am happy to redo it that way — this version was kept self-contained so it can ship on 5.x without a core release.

The per-column `encoding` option has been silently dropped since 5.0, when
column SQL generation moved from Phinx to cakephp/database. `Column` accepts
the option and `Table::getChangedColumnOptions()` even preserves it across a
`changeColumn`, but `Column::toArray()` exports no charset key and
`MysqlSchemaDialect::columnDefinitionSql()` renders no per-column character
set, so `CHARACTER SET` only ever reached the SQL for enum and set columns.

All three paths that render a column definition are affected, since they all
go through `MysqlAdapter::columnDefinitionSql()`: `CREATE TABLE`,
`ALTER TABLE ... ADD` and `ALTER TABLE ... CHANGE`.

Migrations that pair `encoding` with a `collation` were unaffected, since
MySQL infers the character set from the collation. Migrations that set only
`encoding` produced a column in the table's default character set, with no
error to indicate the option had been ignored.

MySQL requires `CHARACTER SET` to precede `COLLATE`, so the fragment cannot be
appended to the generated definition. Render it with a placeholder collation
instead and substitute the pair back in at that position. Types that carry no
collation never render the placeholder, so their definition is untouched and
the encoding is ignored, as MySQL would.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@maurobrandoni

Copy link
Copy Markdown
Author

Happy to be told this is the wrong fix or not worth fixing. The part that cost me time was that encoding is accepted, preserved, and documented, but silently does nothing — so I'd just like it to end up either working or documented as unsupported.

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.

Column 'encoding' option is silently ignored for all types except enum/set

1 participant