Skip to content

fix(schema): store two-state columns as tinyint(1), not enum('0','1') - #27

Merged
mastacontrola merged 1 commit into
mainfrom
booleans-to-tinyint
Aug 25, 2026
Merged

fix(schema): store two-state columns as tinyint(1), not enum('0','1')#27
mastacontrola merged 1 commit into
mainfrom
booleans-to-tinyint

Conversation

@mastacontrola

Copy link
Copy Markdown
Member

Plugin half of fogproject ADR 0028. Core PR: FOGProject/fogproject#1364,
which adds the shared Schema::enumToTinyint() this depends on. Merge that one
first
— without it these steps fatal on a missing method.

Problem

FOG spells its two-state columns enum('0','1'). An integer written to an ENUM
is a member index, not a value, so those columns are off by one:

written stored means
'0' (string) '0' FALSE — correct
'1' (string) '1' TRUE — correct
0 (int) error value '' rejected under STRICT_TRANS_TABLES
1 (int) '0' FALSE — silently inverted

It has only ever worked because PDODB binds every parameter 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().

What changes

Table Columns
LDAPServers lsAllowAPI, lsDisplayNameEnabled, lsIsLDAPs, lsUseGroupMatch
OIDCProviders opAllowAPI, opAutoRedirect, opEnabled, opJITProvision, opSingleLogout
location lTftpEnabled
  • createSql() declares them TINYINT(1) — that 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
    . Rewriting one changes nothing for anyone who ran it, and
    applyUpdates() tolerates 1060, so a fresh install runs them harmlessly
    against columns that are already tinyint.

Why Schema::enumToTinyint() and not a hand-written ALTER

A direct ALTER TABLE t MODIFY c TINYINT(1) converts an ENUM by index.
Measured on MariaDB 11.8.8:

before        0,1,0,1
naive ALTER   1,2,1,2     <- every flag now truthy
via VARCHAR   0,1,0,1

The helper goes enumVARCHAR(1) (converts by label) → UPDATE any row
still holding the ENUM error value → TINYINT(1), and carries nullability and
defaults across rather than assuming NOT NULLlsAllowAPI is nullable and
lsUseGroupMatch has no default at all, and rewriting either would be a
behaviour change smuggled in by a type change.

tests/booleans-are-tinyint.test.php pins both halves: no createSql() declares
an ENUM('0','1') column, and no plugin writes its own MODIFY ... TINYINT.
Mutation-verified — reverting one column to ENUM, and swapping the helper call
for 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 what ENUM
is for.

Wire format

ATTR_EMULATE_PREPARES is off, so mysqlnd returns native types: these fields
now come back as the integer 1 where they came back as "1", and serialise
that 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:

  • 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) straight
    from createSql(), with lsSearchScope still an enum.

bash tests/run-all.sh9 passed, 0 failed.

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>
@mastacontrola
mastacontrola merged commit 4a99d9b into main Aug 25, 2026
2 checks passed
@mastacontrola
mastacontrola deleted the booleans-to-tinyint branch August 25, 2026 11:48
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>
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