fix(statement): record the default's literal kind so keyword and bit defaults converge

[aparajon]

A column DEFAULT was reduced to restored text plus a single "was it quoted" flag, which loses distinctions the characters cannot carry. TRUE, 1 and '1' all restore to a numeric-looking string, and b'1' restores to text that reads like a quoted string but has to be emitted bare.

The parser knows all of them apart — the boolean keyword carries IsBooleanFlag, a bit literal is a binary literal — and ValueExpr.Restore consults that knowledge to produce the text before it is dropped. So the fix is to record the literal form as Column.DefaultKind while the AST is still in hand, and have emission, comparison and the keyword fold read it instead of re-deriving it from the value.

Before                                  After

┌────────────────────────┐              ┌────────────────────────┐
│ AST                    │              │ AST                    │
│  KindInt64 + bool flag │              │  KindInt64 + bool flag │
│  KindString            │              │  KindString            │
│  KindBinaryLiteral     │              │  KindBinaryLiteral     │
└───────────┬────────────┘              └───────────┬────────────┘
            │ restore to text                       │ restore to text
            │ flag read, then dropped               │ + record the kind
            ▼                                       ▼
┌────────────────────────┐              ┌────────────────────────┐
│ Default    "TRUE"      │              │ Default    "TRUE"      │
│ IsString   false       │              │ Kind       KeywordBool │
└───────────┬────────────┘              └───────────┬────────────┘
            │                                       │
            ▼                                       ▼
┌────────────────────────┐              ┌────────────────────────┐
│ needsQuotes(text)      │              │ switch on the kind     │
│ guess from the chars   │              │ fold · quote · bare    │
└────────────────────────┘              └────────────────────────┘

Two things follow.

A bit literal default is now emitted as a bit literal. Quoting it produced DEFAULT 'b\'101\'', which MySQL rejects with Invalid default value, so a bit column carrying a default could not be applied at all — and the error read as the author's mistake rather than ours. MySQL reports a bit literal in its minimal form independent of the column's width (b'0101' comes back as b'101', bit(8) DEFAULT b'00000001' as b'1'), which is the form the parser already restores, so the recorded text was canonical and only needed to stop being quoted. A hex literal default has the same shape of bug (int DEFAULT 0x1A emitted 'x\'1a\'') and is deliberately left for separate work — see below.

The keyword fold now covers every type that stores the keyword as exactly 1/0, recording the literal form Spirit emits it in. Setting the kind alongside the value is what makes the two sides compare equal, since the literal form is part of column identity — folding the value alone would still diff on the form.

Folds	Stored as	MySQL reports	Spirit emits
the integer types, unscaled decimal, double, float	1 / 0	'1' / '0'	1 / 0
varchar, char, varbinary	1 / 0	'1' / '0'	'1' / '0'
bit	1 / 0	b'1' / b'0'	b'1' / b'0'

SHOW CREATE TABLE quotes the value on numeric and string columns alike — only bit reports a literal of its own. The bare numeric emission still converges, because quotedness is not part of column identity on a numeric column (columnsEqualWithContext); on a string column it is, which is what the recorded kind is there for.

Types that put the keyword through a conversion of their own are still left alone, each reading taken from a live server:

Left alone	DEFAULT TRUE stores	Why
scaled decimal	'1.00'	pads to the column's scale; canonicalizing numeric scale is separate
year	'2001'	read as a year rather than as 1
binary(4)	'1\0\0\0'	padded to the column width with NULs; varbinary has nothing to pad and does fold
enum / set	'0' before 9.7, '1' from 9.7	the keyword's meaning changed between server versions, so there is no single value to fold to (see below)

enum and set resolve the keyword differently across the supported matrix. Through 8.4 it resolves numerically, as a member index: enum('0','1') DEFAULT TRUE stores '0' (the member at index 1), enum('a','1') DEFAULT TRUE stores 'a', and DEFAULT FALSE is rejected because no member sits at index 0. From 9.7 it resolves as the string '1'/'0' and is matched against the member list, so both of those columns store '1' and DEFAULT FALSE is accepted. The two readings disagree silently. Readings taken from the official mysql:8.4.11 and mysql:9.7.2 images.

That is why neither type appears in the excluded-types integration fixture: there is no stable reading to assert. That they are skipped is pinned without a server in TestBooleanKeywordDefaultLeavesOtherTypesAlone.

A hex literal stays unmodelled for the same class of reason: MySQL never reports one back, converting it to whatever the column's type stores (an integer column reports 0x1A as 26, a varbinary stores the raw byte), so converging it is a per-type conversion rather than a literal form.

Breaking: Column.DefaultIsString is replaced by Column.DefaultKind, so a caller testing quotedness compares DefaultKind != DefaultKindString instead. Nothing in this repo outside pkg/statement read the field.

Tests cover the classification directly (including that a signed default such as DEFAULT -1 is a unary operator over a literal rather than a literal, and that an unmodelled form keeps the emission it had before kinds existed), and the integration tests assert the leftover diff alongside each excluded type's live reading, so an exclusion's cost is visible in the test rather than implied. Run against MySQL 8.4.11 and 9.7.2 locally, in addition to CI.

The repository template asks that a PR be associated with an Issue; the behavior and the boundary are written up here so the discussion can happen in one place, and I am glad to open one if you would prefer it tracked separately.

🤖 Generated with Claude Code

[Copilot]

🟡 Changes recommended

Several updated comments/docs contradict the actual SHOW CREATE TABLE forms asserted in tests and should be corrected to avoid future maintenance confusion.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR improves how pkg/statement preserves and emits column DEFAULT values by recording the default literal’s AST-derived form (Column.DefaultKind) so diffing, normalization, and formatting don’t have to re-guess intent from restored text (notably for boolean keywords vs numeric literals vs bit literals).

Changes:

• Introduces DefaultKind classification during CREATE TABLE parsing and replaces Column.DefaultIsString with Column.DefaultKind.
• Updates default emission (formatColumnDefinition) and column comparison (columnsEqualWithContext) to use DefaultKind for quoting / identity decisions.
• Expands boolean-keyword default folding to cover additional types (including bit) and adds/updates unit + integration tests for convergence and round-trips.

File summaries

File	Description
pkg/statement/utils.go	Updates needsQuotes commentary to clarify it’s a fallback now that DefaultKind exists.
pkg/statement/README.md	Updates documentation for boolean keyword default folding behavior and scope.
pkg/statement/normalize_function_aliases.go	Switches the string-literal guard from DefaultIsString to DefaultKind.
pkg/statement/normalize_boolean_keyword_default.go	Extends and refactors keyword folding to set both stored value and DefaultKind.
pkg/statement/normalize_boolean_keyword_default_test.go	Expands convergence and “excluded types” coverage for boolean keyword default folding.
pkg/statement/literal_roundtrip_test.go	Adds round-trip coverage for bit-literal defaults and updates keyword-default expectations.
pkg/statement/format.go	Emits defaults based on DefaultKind (string vs bit literal vs number vs heuristic fallback).
pkg/statement/diff.go	Updates default equivalence/identity rules to use DefaultKind (incl. NULL keyword handling).
pkg/statement/diff_integration_test.go	Adds integration coverage across folding types and asserts behavior on excluded types.
pkg/statement/default_kind.go	New: defines DefaultKind and AST-based classification logic (incl. bit vs hex distinction).
pkg/statement/default_kind_test.go	New: tests DefaultKind classification and heuristic fallback emission for unknown kinds.
pkg/statement/create_table.go	Records DefaultKind at parse time while the default AST is still available.
pkg/statement/create_table_test.go	Updates parenthesized literal default tests to assert DefaultKindString.
pkg/statement/columns_equal_guard_test.go	Updates the “all fields compared” guard to include DefaultKind instead of DefaultIsString.

Review details

• Files reviewed: 14/14 changed files
• Comments generated: 5
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[morgo]

🤖 Comment from Morgan's AI agent.

Heads up on the MySQL 9.7 CI failure here — it's not a flake, and the underlying reason is worth knowing beyond this PR.

TestDiffIntegrationBooleanKeywordDefaultOnExcludedTypes fails on the 9.7 job and passes on 8.0.28 / 8.0.42 / 8.0.45 / 8.4. It has failed on both pushes to this branch (2026-09-09 and 2026-09-22), so it's deterministic.

--- FAIL: TestDiffIntegrationBooleanKeywordDefaultOnExcludedTypes (0.07s)
    diff_integration_test.go:748:
        Error: "... `choice` enum('0','1') NOT NULL DEFAULT '1' ..."
               does not contain "`choice` enum('0','1') NOT NULL DEFAULT '0'"

MySQL 9.7 changed how a boolean keyword default resolves on ENUM/SET. Reproduced locally against the official mysql:8.4.9 and mysql:9.7.0 images:

declaration	8.4.9	9.7.0
enum('0','1') NOT NULL DEFAULT TRUE	DEFAULT '0'	DEFAULT '1'
enum('0','1') NOT NULL DEFAULT FALSE	error 1067	DEFAULT '0'
enum('a','1') NOT NULL DEFAULT TRUE	error 1067	DEFAULT '1'
enum('x','y') NOT NULL DEFAULT TRUE	error 1067	error 1067
set('0','1') NOT NULL DEFAULT TRUE	DEFAULT '0'	DEFAULT '1'

• 8.4 and earlier resolve the keyword numerically: TRUE → 1 → member index 1 ('0' in enum('0','1')); FALSE → 0, not a valid index, hence 1067. SET reads it as a bitmask, so bit 0 selects the first member.
• 9.7 resolves it as the string value '1' / '0' and matches against the member list — which is why enum('a','1') DEFAULT TRUE is now accepted where 8.4 rejected it.

The test hardcodes the 8.4 reading with no version gate, so 9.7 fails on the require.Contains of the live SHOW CREATE TABLE before reaching the diff assertions.

This looks like a test-expectation defect only: the normalizer in this PR already excludes ENUM from folding, so no fold is attempted on either version, and the emitted MODIFY COLUMN re-states the declared default and converges on both. The relevance to the PR's thesis is that the comment's justification for the exclusion — "on enum the keyword names a member index rather than a value" — is now only true pre-9.7.

Two ways out, in my order of preference:

1. Drop the choice enum(...) column from the fixture. The reading it records is no longer stable across the supported version matrix, so it can't serve as the justification the test is built to document.
2. Keep it but branch the expected live default on server version, and note in the comment that 9.7 switched from index to value resolution.

Either way, SET has the same split and shouldn't be added without a gate.

The other red job here (TestMoveReverseWindowNMResumesAfterKill on the 8.0.45-with-replicas run) is unrelated to this change and is #1239 — that one is a retrigger.

[aparajon]

🤖 Comment from Armand's AI agent.

Thanks — this was a good catch and the diagnosis is right. I reproduced it against mysql:8.4.11 and mysql:9.7.2 and took your option 1: the choice enum(...) column is gone from the fixture, pushed just now along with a rebase onto main.

Your table matches what I got, with one exception worth recording:

declaration	8.4.11	9.7.2
enum('0','1') NOT NULL DEFAULT TRUE	DEFAULT '0'	DEFAULT '1'
enum('0','1') NOT NULL DEFAULT FALSE	error 1067	DEFAULT '0'
enum('a','1') NOT NULL DEFAULT TRUE	DEFAULT 'a'	DEFAULT '1'
enum('x','y') NOT NULL DEFAULT TRUE	error 1067	error 1067
set('0','1') NOT NULL DEFAULT TRUE	DEFAULT '0'	DEFAULT '1'

enum('a','1') DEFAULT TRUE is accepted on 8.4 and stores 'a' — index 1, exactly as your numeric-resolution reading predicts. So the slip is in that one row, not in the explanation, and it makes the case stronger: the two versions don't disagree by one erroring, they disagree silently, storing a different member from the same DDL. That is now the stated reason for the exclusion rather than "the keyword names a member index", which as you say only held pre-9.7.

On dropping versus gating, I went with dropping for the reason you gave, plus one more: that the fold skips enum/set is already pinned without a server in TestBooleanKeywordDefaultLeavesOtherTypesAlone, so the fixture was carrying the unstable reading and not the behaviour. The version split is documented on booleanKeywordDefaultNormalizer instead. SET stays out for the same reason — agreed it can't be added without a gate.

Confirmed the old fixture fails on 9.7 and the new one passes, and the full pkg/statement suite is green against both 8.4.11 and 9.7.2 locally.

Separately, I fixed what Copilot flagged, because it was right and I had it backwards in four places: SHOW CREATE TABLE quotes a numeric column's default just like a string column's (int NOT NULL DEFAULT '1' on 8.0.28, 8.4.11 and 9.7.2 alike). Only bit reports a literal of its own. The recorded kind decides what Spirit emits, not what the server prints — the note in columnsEqualWithContext already said so, and the other comments contradicted it.

Thanks also for pinning the replica-job red to #1239.

[morgo]

LGTM, lets ship it.