fix(schema): store two-state columns as tinyint(1), not enum('0','1') - #27
Merged
Conversation
fogproject ADR 0028. FOG has spelled its booleans enum('0','1') since the
beginning, and that encoding has a trap in it: an integer written to an
ENUM is a member INDEX rather than a value, so 1 selects the member '0'
-- FALSE -- and 0 is the error value STRICT_TRANS_TABLES refuses. It has
only ever worked because PDODB binds everything as PDO::PARAM_STR.
Core converts its 26 columns in schema step 368. Each plugin owns its
own schema (ADR 0009), so the ten here convert from their own schema():
LDAPServers (lsAllowAPI, lsDisplayNameEnabled, lsIsLDAPs,
lsUseGroupMatch), OIDCProviders (opAllowAPI, opAutoRedirect, opEnabled,
opJITProvision, opSingleLogout) and location (lTftpEnabled).
createSql() now declares them TINYINT(1), which is what a fresh install
gets. The conversion for an existing install is an APPENDED step, for
the same reason as every other ALTER in these lists: installdb() skips
the steps an install has already passed rather than replaying them, so
editing an earlier step is invisible to anyone past it. The historical
`ADD COLUMN ... ENUM('0','1')` steps are deliberately left alone --
applyUpdates() tolerates 1060, so a fresh install runs them harmlessly
against columns that are already tinyint.
The step calls Schema::enumToTinyint(), which is the load-bearing part:
a direct `ALTER TABLE t MODIFY c TINYINT(1)` converts an ENUM BY INDEX,
turning every '0' into 1 and every '1' into 2 -- both truthy, silently,
on every upgrading server. The helper goes through VARCHAR(1) so the
conversion is by label, and carries nullability and defaults across
(lsAllowAPI is nullable; lsUseGroupMatch has no default at all).
Not touched: lsSearchScope enum('0','1','2') and the word enums
(lsNestedGroups, lsTlsVerify, lStorageNodeProto) -- genuine
enumerations, which is what ENUM is for.
Verified against a restore of a live 1.6 database in a throwaway
container, driving each manager's own install(): all ten columns
converted, every value, default and nullability preserved, install()
idempotent on re-run. Dropping the three tables and reinstalling builds
them tinyint(1) from createSql() with lsSearchScope still an enum.
Harness: background_scripts/prove_plugin_enum_to_tinyint.{sh,php}.
Requires the core PR that adds Schema::enumToTinyint()
(FOGProject/fogproject#1364).
Co-Authored-By: Claude <noreply@anthropic.com>
osiktech
pushed a commit
to osiktech/fogproject
that referenced
this pull request
Aug 26, 2026
Carries the plugin half of ADR 0028: the ten two-state columns in
LDAPServers, OIDCProviders and location become tinyint(1), converted
through Schema::enumToTinyint() -- which this branch already ships, so
the ordering is right.
Without the bump the core columns are tinyint and the plugin ones stay
enum('0','1'), which is the two-conventions state ADR 0028 exists to end.
FOGProject/fog-plugins#27, released as v1.6.17.
Co-Authored-By: Claude <noreply@anthropic.com>
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.
Plugin half of fogproject ADR 0028. Core PR: FOGProject/fogproject#1364,
which adds the shared
Schema::enumToTinyint()this depends on. Merge that onefirst — without it these steps fatal on a missing method.
Problem
FOG spells its two-state columns
enum('0','1'). An integer written to an ENUMis a member index, not a value, so those columns are off by one:
'0'(string)'0''1'(string)'1'0(int)''STRICT_TRANS_TABLES1(int)'0'It has only ever worked because
PDODBbinds every parameter asPDO::PARAM_STR. Core converts its 26 columns in schema step 368; each pluginowns its own schema (ADR 0009), so the ten here convert from their own
schema().What changes
LDAPServerslsAllowAPI,lsDisplayNameEnabled,lsIsLDAPs,lsUseGroupMatchOIDCProvidersopAllowAPI,opAutoRedirect,opEnabled,opJITProvision,opSingleLogoutlocationlTftpEnabledcreateSql()declares themTINYINT(1)— that is what a fresh install gets.reason as every other ALTER in these lists:
installdb()skips the steps aninstall has already passed rather than replaying them, so editing an earlier
step is invisible to anyone past it.
ADD COLUMN ... ENUM('0','1')steps are deliberately leftalone. Rewriting one changes nothing for anyone who ran it, and
applyUpdates()tolerates 1060, so a fresh install runs them harmlesslyagainst columns that are already
tinyint.Why
Schema::enumToTinyint()and not a hand-written ALTERA direct
ALTER TABLE t MODIFY c TINYINT(1)converts an ENUM by index.Measured on MariaDB 11.8.8:
The helper goes
enum→VARCHAR(1)(converts by label) →UPDATEany rowstill holding the ENUM error value →
TINYINT(1), and carries nullability anddefaults across rather than assuming
NOT NULL—lsAllowAPIis nullable andlsUseGroupMatchhas no default at all, and rewriting either would be abehaviour change smuggled in by a type change.
tests/booleans-are-tinyint.test.phppins both halves: nocreateSql()declaresan
ENUM('0','1')column, and no plugin writes its ownMODIFY ... TINYINT.Mutation-verified — reverting one column to
ENUM, and swapping the helper callfor a hand-rolled ALTER, each fail the gate.
Not touched
lsSearchScope(enum('0','1','2')) and the word enums —lsNestedGroups,lsTlsVerify,lStorageNodeProto. Genuine enumerations, which is whatENUMis for.
Wire format
ATTR_EMULATE_PREPARESis off, so mysqlnd returns native types: these fieldsnow come back as the integer
1where they came back as"1", and serialisethat way in REST payloads. Deliberate, and the reason the core change lands in a
beta rather than a patches line.
dev-branch/ 1.5.x is not taking any of this.Verification
Against a restore of a live 1.6 database in a throwaway container, driving each
manager's own
install()rather than the helper directly:install()idempotent on re-run;tinyint(1)straightfrom
createSql(), withlsSearchScopestill an enum.bash tests/run-all.sh— 9 passed, 0 failed.